Enhancement/shipping form example#135
Conversation
…into enhancement/shipping-form-example
There was a problem hiding this comment.
Pull request overview
Adds a new “Build a Shipping Form” documentation example and updates vector DB path handling/logging to use the configured vector database location.
Changes:
- Store the vector DB path on
DocumentationIndexerand use it in log output. - Add a new MkDocs example page for building a shipping form with Param/Panel.
- Add accompanying example image asset and link the page into
mkdocs.ymlnavigation.
Reviewed changes
Copilot reviewed 3 out of 5 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
src/holoviz_mcp/holoviz_mcp/data.py |
Persist vector DB path on the indexer and update log messages accordingly. |
mkdocs.yml |
Adds the new shipping form example page to the Examples nav. |
docs/examples/build-a-shipping-form.md |
New example write-up including prompt and resulting code snippet. |
docs/assets/images/examples/shipping-form-claude.png |
New image asset referenced by the example page. |
| logger.info(f"🏠 User config: {indexer._holoviz_mcp_config.config_file_path(location='user')}") | ||
| logger.info(f"📁 Repository directory: {indexer.repos_dir}") | ||
| logger.info(f"💾 Vector database: {indexer.data_dir / 'chroma'}") | ||
| logger.info(f"💾 Vector database: {self._vector_db_path}") |
There was a problem hiding this comment.
In run_indexer, the log line uses self._vector_db_path instead of indexer._vector_db_path. This will be wrong if run_indexer is ever invoked with a different indexer instance than the outer self (and it’s inconsistent with the other log lines in this function). Use indexer._vector_db_path here.
| logger.info(f"💾 Vector database: {self._vector_db_path}") | |
| logger.info(f"💾 Vector database: {indexer._vector_db_path}") |
| - Include a method to validate the complete form and return any errors | ||
| - Add proper docstrings and type hints | ||
| - Create a simple Panel UI form to fill in the shipping form, submit and display the formatted address | ||
| - Catch ValueError during validation and shown them to the user in the UI |
There was a problem hiding this comment.
The requirement text has a grammar issue: "shown them" should be "show them" (or "display them").
| - Catch ValueError during validation and shown them to the user in the UI | |
| - Catch ValueError during validation and show them to the user in the UI |
| - **Environment Defaults**: Reads `DEFAULT_COUNTRY` from environment variables at class definition | ||
| - **Custom Validation**: Country-specific postal code patterns (US: 5 digits, Canada: A1A 1A1, Germany: 5 digits) | ||
| - **Custom Parameter Types**: `MinLengthString` for recipient name validation | ||
| - **Form Methods**: `get_formatted_address()` and `validate()` for complete form handling |
There was a problem hiding this comment.
The “Key features demonstrated” list mentions a get_formatted_address() method, but the example code defines and uses formatted_address(). Please update the text to match the code (or rename the method in the code snippet for consistency).
| - **Form Methods**: `get_formatted_address()` and `validate()` for complete form handling | |
| - **Form Methods**: `formatted_address()` and `validate()` for complete form handling |
| - Plot from Picture: examples/generate-plot-from-picture.md | ||
| - Dashboard from Picture: examples/generate-dashboard-from-picture.md | ||
| - Create ML Experiment Tracker: examples/create-ml-experiment-tracker.md | ||
| - Build A Shipping Form: examples/build-a-shipping-form.md |
There was a problem hiding this comment.
Nav entry capitalization is inconsistent with nearby items (e.g., "Plot from Picture", "Dashboard from Picture" use a lowercase preposition/article). Consider changing "Build A Shipping Form" to "Build a Shipping Form" to match the established style in this nav section.
| address_pane.object = ( | ||
| f"**Formatted Address:**\n```\n{form.formatted_address()}\n```" |
There was a problem hiding this comment.
User-controlled shipping address fields (recipient_name, street_address, city, etc.) are concatenated into a Markdown string and assigned directly to address_pane.object, which can be rendered as HTML in the browser. An attacker can craft input containing Markdown/HTML (e.g., closing the fenced code block with ``` and adding <script> tags) to break out of the intended code block and execute arbitrary JavaScript in the user's browser. To prevent this, ensure that all user-supplied values are properly escaped before inserting into Markdown/HTML, or render the formatted address in a widget/pane that treats the content as plain text rather than rich Markdown/HTML.
No description provided.