fix: resolve MCP prompt rendering errors#524
Conversation
- Change TimeFrame to str in prompt signatures to ensure FastMCP can properly generate JSON schemas (affects continue_conversation, search, and recent_activity prompts) - Fix recent_activity_prompt architecture: the prompt was calling recent_activity.fn() which returns a formatted string, but then checking isinstance(result, ProjectActivitySummary) which always failed. Now the prompt uses the tool's string output directly and adds helpful guidance for next steps. - Update PromptContext dataclass to use str instead of TimeFrame - Pass type parameter as string "entity" instead of enum to match the tool's expected input format - Update tests to match new prompt output format Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Claude finished @groksrc's task in 2m 12s —— View job PR Review: MCP Prompt Type Annotations FixI've completed a comprehensive review of this PR against the Basic Memory team checklist. Here's my detailed analysis: ✅ Code Quality & StandardsFollows Basic Memory coding conventions: ✅
Type Annotations: ✅
No commented-out code: ✅ ✅ Testing & DocumentationTest Coverage: ✅ EXCELLENT
Test Quality: ✅
Coverage Maintenance: ✅
Documentation: ✅
✅ Basic Memory ArchitectureMCP Prompt Design: ✅ SIGNIFICANT IMPROVEMENT
Atomic, Composable Operations: ✅
Type Parameter Handling: ✅
✅ Security & PerformanceNo Security Issues: ✅
Performance: ✅
✅ CompatibilityWindows Compatibility: ✅ N/A (no file path operations in changes) Console Output: ✅
📋 Review Checklist ResultsCode Quality & Standards
Testing & Documentation
Basic Memory Architecture
Security & Performance
Compatibility
🎯 SummaryThis is a high-quality fix that addresses two important issues:
Key Improvements:
Outstanding Item:
Recommendation: ✅ APPROVE - This PR is ready to merge pending the manual test validation. |
Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
…1-100, 5-consec-skip abort at basicmachines-co#10; process v2 with --theirs on doc/test/CI conflicts) Lot 2 picks (oldest-first from /tmp/bm_safe_v2_reverse.txt): - 4ff1e4a fix: resolve MCP prompt rendering errors (basicmachines-co#524) [via --theirs on tests/mcp/test_recent_activity_prompt_modes.py] - dfb948a run ubuntu tests on depot [via --theirs on .github/workflows/test.yml] - 8de13ee fix ci runner i Skipped (deferred to Phase C, real code conflicts): - 8838571 metadata filter tests + fast write fix (router/entity_service) - 009e849 fix stabilize metadata filters postgres (metadata_filters/postgres_search) - 24ca5f6 fix recent_activity prompt defaults (mcp/prompts/recent_activity.py) - a0e754b fix restore legacy /projects/projects endpoint (api/app.py) - c46d7a6 fix add POST legacy compat routes (api/app.py) - a47c9c0 feat add --format json to CLI tool commands (cli/commands/tool.py) - 8489a3d fix add X-Tigris-Consistent headers (cli/commands/cloud/rclone_commands.py) Validation: ruff 23 errors (baseline-equal), tests 1882 PASS / 2 FAIL (baseline 1881/2, +1 test gained). Protected dataview files: untouched. Real success rate: 3/10 = 30% (below 84-94% prediction).
Summary
TimeFrametostrin prompt type annotations for FastMCP JSON schema compatibilityrecent_activity_promptarchitecture - it was checkingisinstance(result, ProjectActivitySummary)on a stringProblem
The
recent_activityMCP prompt was failing with "Error rendering prompt recent_activity" because:Type annotation issue:
TimeFrame = Annotated[str, BeforeValidator(...)]could cause issues with FastMCP's JSON schema generation for prompt parametersArchitecture bug: The prompt called
recent_activity.fn()which returns a formatted string, but then checkedisinstance(recent, ProjectActivitySummary)which was alwaysFalseSolution
timeframeparameters fromTimeFrametostrrecent_activity_promptto use the tool's string output directly and add helpful guidancePromptContextdataclass to usestrinstead ofTimeFrametype="entity"as string instead of enum[SearchItemType.ENTITY]Test plan
/basic-memory-cloud:recent_activity (MCP)in Claude Code🤖 Generated with Claude Code