Add MCP UI variants and TUI output #545
Conversation
Signed-off-by: phernandez <paul@basicmachines.co>
|
Claude finished @phernandez's task in 3m 8s —— View job PR Review: MCP UI variants and TUI outputI've completed a comprehensive review of this PR against the Basic Memory team checklist. This is a well-structured implementation that adds three UI presentation paths for MCP tools. ✅ Code Quality & StandardsStrengths
Issues Found1. Line Length ViolationLocation: summary = f"Results: {len(results)} | Page: {result.current_page} | Page size: {result.page_size}"Issue: 101 characters (exceeds 100-char limit) ✅ Testing & DocumentationTest Coverage
Documentation
Coverage Concerns
✅ Basic Memory ArchitectureAlignment with Principles
Architecture Decisions1. Tool Registration ( from basic_memory.mcp.tools.ui_sdk import read_note_ui, search_notes_ui✅ Proper registration of new UI SDK tools 2. Resource Registration (
3. Output Format Extension ( output_format: Literal["default", "ascii", "ansi"] = "default"✅ Clean extension of existing tools without breaking changes 4. Formatting Module (
✅ Security & PerformanceSecurity Review
Performance Considerations
✅ Windows CompatibilityPath Operations✅ Proper use of pathlib: path = Path(__file__).parent / "html" / filename
Console Output✅ No emoji/unicode in console output in NEW code:
Note: Existing code in React/UI Components ReviewBuild System (
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5d3fc78c21
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| function stripFrontmatter(text: string): string { | ||
| if (!text.startsWith("---")) return text; | ||
| const end = text.indexOf("---", 3); | ||
| if (end === -1) return text; | ||
| return text.slice(end + 3).trim(); |
There was a problem hiding this comment.
The Tool‑UI stripFrontmatter helper treats the first --- substring after position 3 as the closing delimiter. That means YAML values like title: "foo --- bar" (or any early --- in frontmatter) will be misinterpreted as the end of frontmatter, causing the preview to drop the rest of the note. This truncation only happens when frontmatter contains ---, but it yields incorrect previews in those cases. Consider matching a delimiter on its own line (e.g., \n---\n) instead of indexOf.
Useful? React with 👍 / 👎.
1 participant
Summary\n- add MCP UI resources (search/note preview) with variant switching via BASIC_MEMORY_MCP_UI_VARIANT and variant-specific URIs for the bakeoff\n- scaffold tool-ui React bundle + build script to inline JS/CSS into MCP resource HTML; keep sources under ui/tool-ui-react\n- add ASCII/ANSI output paths for search_notes/read_note plus tests; add MCP-UI SDK embedded resource tools (search_notes_ui, read_note_ui) and doc for bakeoff steps\n- register resources in CLI/tests and document bakeoff/test plan in docs/mcp-ui-bakeoff-instructions.md\n\n## Testing\n- uv run pytest test-int/mcp/test_output_format_ascii_integration.py test-int/mcp/test_ui_sdk_integration.py\n- npm run build (in ui/tool-ui-react)