Conversation
Implements comprehensive MCP-UI support for visual search results and content discovery, providing interactive HTML components alongside existing JSON responses for backward compatibility.
## Features
### UI Components
- Sticky header with NVIDIA branding and logo
- Interactive search results with relevance badges and content type icons
- HTMX-powered dynamic filtering (sort, relevance slider)
- Content discovery UI with content type tabs
- Citations panel with numbered references
- Empty states and error handling
### Technical Implementation
- New UI module: components, renderer, templates, styles
- HTMX integration for dynamic updates without page reloads
- 3 new HTTP endpoints: /ui/filter, /ui/content, /ui/citation/{index}
- Fallback to JSON-only if mcp-ui-server not installed
- Backward compatible: returns both UI resource and JSON
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds optional MCP-UI integration: an optional Changes
Sequence Diagram(s)sequenceDiagram
participant Tool as MCP Tool
participant Server as Server.call_tool
participant Builder as build_tool_result_with_ui
participant UI as UI Renderer/Templates
participant Client as HTTP Client
Tool->>Server: return response dict
Server->>Builder: build_tool_result_with_ui(response, tool_name)
Builder->>UI: request HTML render for tool_name
UI-->>Builder: HTML string or ImportError
Builder-->>Server: CallToolResult (text + optional UI resource)
Server-->>Client: return tool result including optional UI
sequenceDiagram
participant Client as HTTP Client
participant HTTPServer as http_server handler
participant QueryEngine as Search/Discovery Engine
participant Renderer as render_filter_ui
participant Templates as template fragment builder
Client->>HTTPServer: GET /ui/filter?query=...
HTTPServer->>QueryEngine: execute query
QueryEngine-->>HTTPServer: response JSON
HTTPServer->>Renderer: render_filter_ui(response, ...)
Renderer->>Templates: build filter fragment (panel + results)
Templates-->>Renderer: HTML fragment
Renderer-->>HTTPServer: HTMLResponse
HTTPServer-->>Client: 200 OK + HTML fragment
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 Fix all issues with AI agents
In @pyproject.toml:
- Line 45: The mcp-ui-server entry is listed as a required dependency but the
code (function build_tool_result_with_ui in response_builders.py) expects it to
be optional via try/except ImportError; remove the "mcp-ui-server>=0.1.0" line
from the main dependencies array and add it to the project's optional
dependencies (e.g., under [project.optional-dependencies] or the equivalent
extras section) preserving the version spec so callers can opt into UI support
when needed.
In @src/mcp_nvidia/http_server.py:
- Around line 90-92: The except block in the filter endpoint currently returns
the raw exception string to users; keep the existing logger.exception(...) call
to capture full details but change the HTMLResponse to return a generic,
non-revealing message (e.g., "An internal error occurred") and an appropriate
HTTP status (500) instead of embedding {e!s}; locate the try/except handling the
"Error in filter endpoint" log and update the return to a generic error response
while leaving logger.exception(...) unchanged.
- Around line 124-126: The content endpoint currently returns the raw exception
string in the HTMLResponse (using {e!s}), which can leak sensitive details; keep
the logger.exception(e) call but change the HTMLResponse to a generic
user-facing message (e.g., "<div class='mcp-nvidia-error'>An internal error
occurred</div>") instead of including {e!s}; modify the return in the content
endpoint handler that uses logger.exception and HTMLResponse so it returns a
non-leaking generic error message.
- Around line 129-140: The handler handle_citation currently calls
int(request.path_params.get("index", 1)) which raises ValueError for non-numeric
path params; wrap parsing in a validation step (e.g., read index_str =
request.path_params.get("index"), try to convert to int in a try/except) and on
ValueError or missing/invalid value set a safe default (e.g., citation_num = 1)
or return a controlled HTMLResponse/400; ensure the rest of the function uses
the validated citation_num when building the citation HTML and always returns an
HTMLResponse instead of letting the exception propagate.
In @src/mcp_nvidia/ui/__init__.py:
- Around line 3-5: The package __init__ currently imports render_content_ui and
render_search_ui from templates.py which bypasses the error-handling wrapper in
renderer.py; update the import to pull these symbols from mcp_nvidia.ui.renderer
(i.e., replace the import line to "from mcp_nvidia.ui.renderer import
render_content_ui, render_search_ui") so the response.get("success") checks and
error UI rendering in renderer.render_search_ui / renderer.render_content_ui are
used, leaving the __all__ export list unchanged.
In @src/mcp_nvidia/ui/components.py:
- Around line 164-184: render_content_type_tabs currently interpolates the raw
topic into the hx-vals attribute, allowing quotes or control chars in topic to
break the JSON/HTML and enable XSS; fix by building the hx-vals value with
json.dumps({"content_type": ct, "topic": topic}) and then HTML-escaping that
JSON string (e.g., html.escape) before embedding it into the attribute in
render_content_type_tabs so the attribute always contains a safely-encoded JSON
value.
- Around line 80-120: In render_result_card, all user-provided fields (title,
snippet, domain, url, content_type, matched_keywords, and published_date) must
be HTML-escaped before insertion into the template and the href must be
validated to disallow javascript: (or other unsafe) schemes; fix by applying a
standard HTML escape (e.g., html.escape) to title, snippet, domain, content_type
(and each kw in matched_keywords) and only use the escaped values in the
returned string, and sanitize/validate url (allow only safe schemes like http,
https, mailto or percent-encode it) before inserting into href and hx-get
attributes; update the generation of keywords_html, date_html, and all
interpolations in render_result_card to use these escaped/sanitized variables.
- Around line 21-31: The render_search_header function (and any other renderers
in this file that interpolate user-controlled values like title, snippet, url,
domain, topic, content_type, and keywords) currently injects raw strings into
HTML and must HTML-escape them to prevent XSS; import Python's html module and
wrap each user-provided value with html.escape(..., quote=True) before
interpolating (e.g., escape query in render_search_header and apply the same
change to all templates that use title, snippet, url, domain, topic,
content_type, keywords), ensuring you escape values consistently and keep
readonly/placeholder text unchanged.
In @src/mcp_nvidia/ui/templates.py:
- Line 44: The <title> lines that interpolate user input ("<title>NVIDIA MCP
Search - {query}</title>" and the similar occurrence for {topic}) must
HTML-escape the values before insertion; update the template rendering to call
an escape function (e.g., html.escape(query) or markupsafe.escape(query)) for
both query and topic, import the chosen escape utility, and use the escaped
value where the title is built so user-controlled input cannot inject markup.
- Around line 71-80: The header string construction directly interpolates
unescaped values (topic, total_found, search_time_ms) into HTML; fix by
HTML-escaping topic and the numeric values before interpolation (e.g., use
html.escape or your project's escape utility to produce escaped_topic =
escape(topic) and escaped_total = escape(str(total_found)), escaped_time =
escape(str(search_time_ms))) and then use those escaped variables when building
the header string in the header assignment block so no raw user-controlled
content is injected into the input value or results count.
- Around line 107-129: render_error_ui currently injects raw error["code"] and
error["message"] into the HTML, which allows HTML/JS to be rendered; import and
use html.escape to sanitize both code and message before formatting (e.g.,
escape the values stored in the local variables code and message inside
render_error_ui) so the returned string contains escaped content; add the
necessary import (from html import escape) at top and replace occurrences of
code and message in the template with their escaped counterparts.
🧹 Nitpick comments (2)
src/mcp_nvidia/lib/response_builders.py (1)
296-325: Broaden exception handling to catch rendering failures.The
try/exceptonly catchesImportError, but if the UI rendering functions (render_search_ui,render_content_ui) orcreate_ui_resourceraise other exceptions (e.g.,KeyError,TypeErrorfrom malformed response data), the error will propagate and break the tool call instead of gracefully falling back.Suggested improvement
try: from mcp_ui_server import create_ui_resource from mcp_nvidia.ui import render_content_ui, render_search_ui if tool_name == "search_nvidia": html = render_search_ui(response) elif tool_name == "discover_nvidia_content": html = render_content_ui(response) else: html = None ui_content = [] if html: ui_resource = create_ui_resource( { "uri": f"ui://{tool_name}/results", "content": {"type": "rawHtml", "htmlString": html}, "encoding": "text", } ) ui_content.append(ui_resource) return CallToolResult( content=[TextContent(type="text", text=json.dumps(response, indent=2)), *ui_content], structuredContent=response, isError=not response.get("success", False), ) - except ImportError: + except ImportError: + return build_tool_result(response) + except Exception: + # Log the error for debugging but don't break the response + import logging + logging.getLogger(__name__).warning("UI rendering failed, falling back to JSON-only response", exc_info=True) return build_tool_result(response)tests/test_ui_rendering.py (1)
71-90: Consider adding XSS test cases for user-provided content.The result card tests verify content rendering, but there are no tests verifying that special characters in user-provided fields (title, snippet, query) are properly escaped to prevent XSS when rendered in HTML.
Suggested additional test
def test_render_result_card_escapes_html(self): """Test result card escapes HTML special characters.""" result = { "title": "<script>alert('xss')</script>", "url": "https://example.com", "snippet": "Test <b>snippet</b> with HTML", "domain": "example.com", "content_type": "article", "relevance_score": 50, } html = render_result_card(result, index=1) assert "<script>" not in html assert "<script>" in html or "alert" not in html
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
pyproject.tomlsrc/mcp_nvidia/http_server.pysrc/mcp_nvidia/lib/__init__.pysrc/mcp_nvidia/lib/response_builders.pysrc/mcp_nvidia/server.pysrc/mcp_nvidia/ui/__init__.pysrc/mcp_nvidia/ui/components.pysrc/mcp_nvidia/ui/renderer.pysrc/mcp_nvidia/ui/styles.pysrc/mcp_nvidia/ui/templates.pytests/test_ui_rendering.py
🧰 Additional context used
🧬 Code graph analysis (8)
src/mcp_nvidia/ui/templates.py (2)
src/mcp_nvidia/ui/components.py (7)
render_citations(138-161)render_content_container(218-230)render_content_type_tabs(164-184)render_filter_panel(34-77)render_results_container(123-135)render_search_header(21-31)render_warnings(210-215)src/mcp_nvidia/ui/renderer.py (2)
render_search_ui(17-30)render_content_ui(33-46)
src/mcp_nvidia/lib/__init__.py (1)
src/mcp_nvidia/lib/response_builders.py (1)
build_tool_result_with_ui(282-325)
src/mcp_nvidia/ui/renderer.py (1)
src/mcp_nvidia/ui/templates.py (4)
render_content_ui(61-104)render_error_ui(107-129)render_filter_fragment(132-150)render_search_ui(17-58)
src/mcp_nvidia/http_server.py (5)
src/mcp_nvidia/lib/response_builders.py (2)
build_search_response_json(12-107)build_content_response_json(110-207)src/mcp_nvidia/lib/search.py (1)
search_all_domains(211-496)src/mcp_nvidia/ui/renderer.py (2)
render_filter_ui(49-78)render_content_ui(33-46)src/mcp_nvidia/lib/content_discovery.py (1)
discover_content(62-287)src/mcp_nvidia/ui/templates.py (1)
render_content_ui(61-104)
src/mcp_nvidia/ui/components.py (1)
bin/mcp-nvidia.js (1)
result(34-34)
src/mcp_nvidia/ui/__init__.py (2)
src/mcp_nvidia/ui/renderer.py (2)
render_content_ui(33-46)render_search_ui(17-30)src/mcp_nvidia/ui/templates.py (2)
render_content_ui(61-104)render_search_ui(17-58)
src/mcp_nvidia/server.py (1)
src/mcp_nvidia/lib/response_builders.py (1)
build_tool_result_with_ui(282-325)
src/mcp_nvidia/lib/response_builders.py (2)
src/mcp_nvidia/ui/renderer.py (2)
render_content_ui(33-46)render_search_ui(17-30)src/mcp_nvidia/ui/templates.py (2)
render_content_ui(61-104)render_search_ui(17-58)
⏰ 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). (4)
- GitHub Check: test-with-coverage
- GitHub Check: test (3.11)
- GitHub Check: test (3.10)
- GitHub Check: test (3.12)
🔇 Additional comments (5)
src/mcp_nvidia/ui/styles.py (1)
1-462: LGTM - Well-structured CSS with consistent naming.The styles are comprehensive and follow a clear naming convention (
mcp-nvidia-*). The NVIDIA branding is consistent throughout.Minor formatting note: Line 314 has a slight indentation inconsistency (extra leading space), but this doesn't affect functionality.
src/mcp_nvidia/lib/__init__.py (1)
36-36: LGTM - Export correctly added.The new
build_tool_result_with_uifunction is properly imported and exported alongside related response builders.Also applies to: 64-64
tests/test_ui_rendering.py (1)
1-17: LGTM - Comprehensive test coverage for UI components.The test suite provides good coverage for all UI components, styles, templates, and HTMX integration. The tests verify both content presence and HTMX attributes.
src/mcp_nvidia/server.py (1)
27-27: LGTM - Clean integration of UI-enabled responses.The
build_tool_result_with_uiis correctly used only for successful search results while error responses continue to usebuild_tool_result. The tool names passed match the schema definitions.Also applies to: 492-492, 543-543
src/mcp_nvidia/ui/renderer.py (1)
17-78: LGTM!Clean wrapper module that provides consistent error handling and delegates to templates. The separation of concerns is appropriate.
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/mcp_nvidia/http_server.py (1)
26-35: Remove the dummyResponse()return—connect_sse()already sends the full ASGI response viaEventSourceResponse.The
SseServerTransport.connect_sse()context manager runsEventSourceResponseas an ASGI application in a background task, which sendshttp.response.startandhttp.response.body. ReturningResponse()after the context exits attempts to send the response again, violating the ASGI protocol. Convert this endpoint to a raw ASGI callable to eliminate the problematic return.Proposed refactor (switch to ASGI callable)
-async def handle_sse(request: Request) -> Response: - """Handle SSE connections for MCP.""" - logger.info("New SSE connection established") - - async with sse.connect_sse(request.scope, request.receive, request.scope["send"]) as (read_stream, write_stream): - await mcp_app.run(read_stream, write_stream, mcp_app.create_initialization_options()) - - # Return a dummy response to satisfy type checker, though SSE session keeps connection open - return Response() +async def handle_sse(scope, receive, send): + """Handle SSE connections for MCP.""" + logger.info("New SSE connection established") + async with sse.connect_sse(scope, receive, send) as (read_stream, write_stream): + await mcp_app.run(read_stream, write_stream, mcp_app.create_initialization_options())Then update the route registration to use this ASGI callable directly.
🤖 Fix all issues with AI agents
In @src/mcp_nvidia/http_server.py:
- Around line 134-151: The handler handle_citation currently defaults invalid or
missing index to 1 which can hide client errors and allow out-of-range IDs;
change it to validate index_str by attempting int conversion and if conversion
fails return a 400 response (Bad Request) with a clear error message, then clamp
the parsed citation_num to a sane range (e.g., min 1, max some MAX_CITATION
constant) before using it in the citation id string so requests like
/ui/citation/999999 are either rejected or normalized; update any references to
citation_num and introduce/mention MAX_CITATION in the same module.
- Around line 50-98: The handle_ui_filter function needs tighter input
validation and graceful handling when UI deps are missing: validate sort_by
against an allowlist (e.g., {"relevance","date","domain"}), default to
"relevance" if invalid; clamp min_relevance_score to the 0–100 range after
parsing; and split the broad try/except so ImportError (or ModuleNotFoundError)
from importing render_filter_ui is caught and returns a small "UI not available"
HTML fragment (or 501 response) rather than a 500, while other exceptions still
log via logger.exception; update references in this logic to the existing
symbols DEFAULT_DOMAINS, build_search_response_json, search_all_domains, and
render_filter_ui so the changes are localized to handle_ui_filter.
In @src/mcp_nvidia/ui/components.py:
- Around line 7-20: CONTENT_TYPE_ICONS currently uses "blog_post" while UI tabs
emit content_type="blog", causing blog cards to fall back to the default icon;
update CONTENT_TYPE_ICONS (and the other similar mapping around the mentioned
block) to include the "blog" key (or normalize keys by mapping "blog" ->
"blog_post") so both "blog" and "blog_post" resolve to the same emoji, and
ensure any lookup code that reads CONTENT_TYPE_ICONS continues to use the same
key names.
- Around line 160-193: In render_citations(), avoid KeyError and unescaped
output by replacing c['number'] with an escaped value from c.get("number", "")
(e.g., num = html.escape(str(c.get("number", "")), quote=True)) and use that in
the citation-number span; ensure the anchor only uses safe_url when validated
and add rel="noopener noreferrer" to the anchor tag alongside target="_blank" to
mitigate security issues; keep using html.escape for title and domain as done
and handle missing/empty url by rendering either a non-link span or an anchor
with empty href safely.
- Around line 37-82: render_filter_panel currently uses
hx-target="#mcp-nvidia-results" with default innerHTML swap and hx-include that
omits query; update the sort select and min_relevance_score range inputs in
render_filter_panel to include hx-swap="outerHTML" and add "[name='query']" to
their hx-include attributes so the endpoint receives query and replaces the
whole results container. Also ensure render_results_container and
render_content_container always wrap their empty-state HTML with the appropriate
wrapper IDs (e.g., <div id="mcp-nvidia-results">...</div> and <div
id="mcp-nvidia-content-results">...</div>) so responses have a consistent
structure; apply the same hx-swap/hx-include fixes to the content discovery tab
controls referenced in the UI code.
🧹 Nitpick comments (2)
src/mcp_nvidia/ui/components.py (1)
85-106: Repeated URL allowlist logic: extract a helper to keep behavior consistent
Right now URL sanitization is duplicated 3 times; easy to drift (e.g., if you later addtel:or decide to.strip()first).Sketch
+def _safe_href(url: str) -> str: + if not url: + return "" + u = url.strip() + if u.lower().startswith(("http://", "https://", "mailto:")): + return html.escape(u, quote=True) + return ""Then replace each block with
safe_url = _safe_href(url).Also applies to: 160-176, 225-243
src/mcp_nvidia/http_server.py (1)
161-163: Route registration should reflect “optional UI” objective (conditionally register UI routes).
Right now the UI endpoints are always exposed and will return 500 if optional UI imports fail at runtime; recommend detecting UI availability at startup and only adding these routes when enabled.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
pyproject.tomlsrc/mcp_nvidia/http_server.pysrc/mcp_nvidia/ui/__init__.pysrc/mcp_nvidia/ui/components.pysrc/mcp_nvidia/ui/templates.py
🚧 Files skipped from review as they are similar to previous changes (3)
- src/mcp_nvidia/ui/templates.py
- src/mcp_nvidia/ui/init.py
- pyproject.toml
🧰 Additional context used
🧬 Code graph analysis (1)
src/mcp_nvidia/http_server.py (5)
src/mcp_nvidia/lib/response_builders.py (2)
build_search_response_json(12-107)build_content_response_json(110-207)src/mcp_nvidia/lib/search.py (1)
search_all_domains(211-496)src/mcp_nvidia/ui/renderer.py (2)
render_filter_ui(49-78)render_content_ui(33-46)src/mcp_nvidia/lib/content_discovery.py (1)
discover_content(62-287)src/mcp_nvidia/ui/templates.py (1)
render_content_ui(64-112)
⏰ 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). (4)
- GitHub Check: test (3.12)
- GitHub Check: test-with-coverage
- GitHub Check: test (3.10)
- GitHub Check: test (3.11)
🔇 Additional comments (1)
src/mcp_nvidia/http_server.py (1)
13-13: Import change looks fine (HTMLResponse added).
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @src/mcp_nvidia/http_server.py:
- Around line 113-144: handle_ui_content currently catches all exceptions and
returns a generic 500, but it should mirror handle_ui_filter by gracefully
handling missing UI deps; modify handle_ui_content to catch ImportError (from
attempting to import mcp_nvidia.ui and related modules) separately and return a
501 HTMLResponse with the same installation/informational message used by
handle_ui_filter (and log the ImportError with logger.warning or logger.info),
while letting other exceptions continue to be caught by the existing general
Exception handler.
🧹 Nitpick comments (1)
src/mcp_nvidia/http_server.py (1)
49-75: Consider adding query length validation.The
queryparameter is used directly without length validation. While downstream functions may handle this, adding an explicit check at the HTTP layer would provide defense-in-depth against potential DoS via extremely long query strings.💡 Suggested validation
async def handle_ui_filter(request: Request) -> HTMLResponse: """Handle HTMX filter requests for search results.""" query = request.query_params.get("query", "") + + # Limit query length to prevent abuse + MAX_QUERY_LENGTH = 500 + if len(query) > MAX_QUERY_LENGTH: + query = query[:MAX_QUERY_LENGTH] + sort_by = request.query_params.get("sort_by", "relevance")
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/mcp_nvidia/http_server.pysrc/mcp_nvidia/ui/components.py
🚧 Files skipped from review as they are similar to previous changes (1)
- src/mcp_nvidia/ui/components.py
🧰 Additional context used
🧬 Code graph analysis (1)
src/mcp_nvidia/http_server.py (5)
src/mcp_nvidia/lib/response_builders.py (2)
build_search_response_json(12-107)build_content_response_json(110-207)src/mcp_nvidia/lib/search.py (1)
search_all_domains(211-496)src/mcp_nvidia/ui/renderer.py (2)
render_filter_ui(49-78)render_content_ui(33-46)src/mcp_nvidia/lib/content_discovery.py (1)
discover_content(62-287)src/mcp_nvidia/ui/templates.py (1)
render_content_ui(64-112)
⏰ 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). (4)
- GitHub Check: test-with-coverage
- GitHub Check: test (3.12)
- GitHub Check: test (3.10)
- GitHub Check: test (3.11)
🔇 Additional comments (3)
src/mcp_nvidia/http_server.py (3)
28-33: LGTM! Correct ASGI protocol fix.The switch from Starlette's
Requestwrapper to raw ASGI parameters (scope,receive,send) is the correct approach for SSE transport. This aligns with howSseServerTransport.connect_sseexpects to be called and resolves the protocol violation.
147-176: LGTM! Proper input validation for citation endpoint.Good defensive coding:
- Validates presence of
indexparameter- Validates it's a valid integer
- Clamps to valid range
[1, MAX_CITATION]- No XSS risk since only the validated integer is interpolated
187-189: LGTM! Routes properly registered.The new UI routes follow a clean
/ui/*namespace pattern and are correctly wired to their async handlers.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @src/mcp_nvidia/ui/renderer.py:
- Around line 82-93: The function render_content_ui_fragment is reading the
wrong key from the response; replace the call to response.get("results", [])
with response.get("content", []) so it uses the array returned by
build_content_response_json, and keep a safe default (empty list) if the key is
missing; update the reference in render_content_ui_fragment (and any nearby
tests or callers if present) to use "content" so render_content_fragment
receives the actual structured content.
🧹 Nitpick comments (5)
src/mcp_nvidia/ui/templates.py (2)
31-35: Extract defaultmin_relevance_scoreto a shared constant.The magic number
17formin_relevance_scoreappears in multiple places (templates.py, renderer.py, http_server.py). Centralizing it as a named constant improves maintainability and prevents drift.♻️ Suggested approach
Define a constant in a shared module (e.g.,
constants.pyorui/__init__.py):DEFAULT_MIN_RELEVANCE_SCORE = 17Then import and use it across all files.
Also applies to: 147-150
48-48: Add Subresource Integrity (SRI) hash for HTMX CDN script.Both lines 48 and 100 load HTMX from unpkg without an integrity hash, exposing the application to supply chain attacks. Add the SRI integrity attribute to ensure the browser rejects any tampered scripts.
♻️ Suggested fix with SRI hash
- <script src="https://unpkg.com/htmx.org@1.9.10"></script> + <script src="https://unpkg.com/htmx.org@1.9.10" integrity="sha384-D1Kt99CQMDuVetoL1lrYwg5t+9QdHe7NLX/SoJYkXDFfX37iInKRy5xLSi8nO7UC" crossorigin="anonymous"></script>src/mcp_nvidia/ui/renderer.py (1)
5-15: Consider consolidating imports from the same module.The three separate import blocks from
mcp_nvidia.ui.templatescan be merged into a single statement for cleaner organization.♻️ Suggested consolidation
-from mcp_nvidia.ui.templates import ( - render_content_fragment, - render_error_ui, - render_filter_fragment, -) -from mcp_nvidia.ui.templates import ( - render_content_ui as render_content_ui_template, -) -from mcp_nvidia.ui.templates import ( - render_search_ui as render_search_ui_template, -) +from mcp_nvidia.ui.templates import ( + render_content_fragment, + render_content_ui as render_content_ui_template, + render_error_ui, + render_filter_fragment, + render_search_ui as render_search_ui_template, +)src/mcp_nvidia/http_server.py (2)
108-110: Avoid including exception details in f-string for logging.Using
logger.exception()already logs the full traceback. Including{e}in the message is redundant and, in some edge cases, could expose sensitive information in aggregated logs.♻️ Suggested simplification
except Exception as e: - logger.exception(f"Error in filter endpoint: {e}") + logger.exception("Error in filter endpoint") return HTMLResponse("<div class='mcp-nvidia-error'>An internal error occurred</div>", status_code=500)Apply the same pattern to
handle_ui_content.Also applies to: 154-156
178-179: Silent clamping may mask invalid requests.If a client requests
/ui/citation/5000, silently clamping to 1000 returns misleading feedback ("Citation [1000] copied") instead of the requested citation. Consider returning a 400 error for out-of-range values, similar to the handling for non-numeric indices.♻️ Suggested change
- # Clamp citation_num to valid range - citation_num = max(1, min(citation_num, MAX_CITATION)) + # Validate citation_num range + if citation_num < 1 or citation_num > MAX_CITATION: + return HTMLResponse( + f"<div class='mcp-nvidia-error'>Citation index must be between 1 and {MAX_CITATION}</div>", + status_code=400, + )
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/mcp_nvidia/http_server.pysrc/mcp_nvidia/ui/renderer.pysrc/mcp_nvidia/ui/templates.py
🧰 Additional context used
🧬 Code graph analysis (3)
src/mcp_nvidia/http_server.py (4)
src/mcp_nvidia/lib/response_builders.py (2)
build_search_response_json(12-107)build_content_response_json(110-207)src/mcp_nvidia/lib/search.py (1)
search_all_domains(211-496)src/mcp_nvidia/ui/renderer.py (2)
render_filter_ui(50-79)render_content_ui_fragment(82-93)src/mcp_nvidia/lib/content_discovery.py (1)
discover_content(62-287)
src/mcp_nvidia/ui/renderer.py (1)
src/mcp_nvidia/ui/templates.py (5)
render_content_fragment(165-167)render_error_ui(115-141)render_filter_fragment(144-162)render_content_ui(64-112)render_search_ui(18-61)
src/mcp_nvidia/ui/templates.py (2)
src/mcp_nvidia/ui/components.py (6)
render_citations(165-204)render_content_container(279-293)render_content_type_tabs(207-234)render_filter_panel(38-85)render_search_header(24-35)render_warnings(270-276)src/mcp_nvidia/ui/renderer.py (2)
render_search_ui(18-31)render_content_ui(34-47)
⏰ 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). (4)
- GitHub Check: test-with-coverage
- GitHub Check: test (3.10)
- GitHub Check: test (3.11)
- GitHub Check: test (3.12)
🔇 Additional comments (10)
src/mcp_nvidia/ui/templates.py (4)
18-61: LGTM! Proper XSS protection and clean template composition.The
render_search_uifunction properly escapes user input (query) before embedding in HTML attributes and title. The delegation to component functions maintains good separation of concerns.
64-112: LGTM! Content UI rendering is well-structured.User-provided values (
topic,total_found,search_time_ms) are properly escaped. The composition pattern with component functions is consistent with the search UI.
115-141: LGTM! Error UI handles escaping correctly.Both
codeandmessageare escaped before rendering, preventing XSS from malicious error payloads.
144-167: LGTM! Fragment renderers are concise delegations.Both
render_filter_fragmentandrender_content_fragmentcleanly delegate to their respective component functions without introducing additional logic or escaping gaps.src/mcp_nvidia/ui/renderer.py (2)
18-31: LGTM! Error-aware wrapper with clean delegation.The pattern of checking for errors before delegating to templates is consistent and provides graceful error handling.
50-79: LGTM! Filter UI renderer correctly extracts and passes parameters.The function properly extracts all necessary fields from the response and passes them to
render_filter_fragment.src/mcp_nvidia/http_server.py (4)
28-33: LGTM! Correct ASGI raw callable signature for SSE.The signature change to
(scope, receive, send)properly implements the ASGI interface for direct transport handling.
49-75: LGTM! Robust input validation with graceful fallback.Good security practices:
- Allowlist validation for
sort_by- Clamping
min_relevance_scoreto 0-100- Graceful 501 response when UI deps are unavailable
113-156: LGTM! Content endpoint follows the same robust pattern.The validation and error handling mirrors
handle_ui_filter, maintaining consistency across UI endpoints.
191-203: LGTM! Route configuration is clean and well-organized.The new UI routes are logically grouped and follow RESTful naming conventions.
1 participant
Summary
Adds comprehensive MCP-UI support to mcp-nvidia, enabling interactive visual search results and content discovery interfaces. The implementation uses HTMX for dynamic interactivity and maintains full backward compatibility.
Features
Interactive UI Components
Technical Implementation
src/mcp_nvidia/ui/module (5 files, 1000+ lines)Testing & Quality
The UI renders interactive HTML in MCP-UI compatible clients with:
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.