fix: Add proper datetime JSON schema format annotations for MCP validation#312
fix: Add proper datetime JSON schema format annotations for MCP validation#312phernandez merged 3 commits intomainfrom
Conversation
…ation - Added Annotated[datetime, Field(json_schema_extra)] to all datetime fields - This ensures JSON schemas include 'format: date-time' for MCP client validation - Kept @field_serializer decorators for actual JSON serialization - Fixed memory URL validation to allow empty paths for root memory:// URLs - Resolves schema validation failures in strict MCP clients like ChatWise Fixes #263 Co-authored-by: Paul Hernandez <phernandez@users.noreply.github.com>
|
Claude finished @phernandez's task —— View job Review of PR #312: Fix datetime JSON schema format annotationsI've completed a thorough review of this PR against the Basic Memory team checklist. Here's my analysis: ✅ Code Quality & Standards
|
|
@claude do we need to fix the tests? |
|
Claude finished @phernandez's task —— View job Analysis of test requirementsTodo list:
Current Test CoverageAfter reviewing the test suite, I found:
Answer: No test fixes neededThe existing tests should continue to pass because:
RecommendationWhile not required for this PR to work, I recommend adding a test that validates the JSON schema itself includes the datetime format annotations. This would catch regressions if someone accidentally removes the annotations in the future. Here's a simple test that could be added to def test_json_schema_has_datetime_format_annotations(self):
"""Test that JSON schemas include format: date-time for datetime fields."""
# Check EntitySummary schema
schema = EntitySummary.model_json_schema()
assert schema["properties"]["created_at"]["format"] == "date-time"
# Check other models...
assert RelationSummary.model_json_schema()["properties"]["created_at"]["format"] == "date-time"
assert ObservationSummary.model_json_schema()["properties"]["created_at"]["format"] == "date-time"
assert MemoryMetadata.model_json_schema()["properties"]["generated_at"]["format"] == "date-time"But this is optional - your PR correctly fixes the MCP validation issue and no existing tests need updates. |
- Revert memory URL validation to disallow empty paths (empty URLs don't make semantic sense for build_context) - Fix format_prompt_context to handle entities without permalinks (e.g., binary files) - Update all validation tests to match correct behavior - Add whitespace stripping in normalize_memory_url for consistency All tests now pass. The main PR changes (datetime format annotations) remain intact. Signed-off-by: Claude Code <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
Test UpdatesUpdated tests after merging main branch. Key changes:
All tests passing ✅. The core fix (datetime JSON schema format annotations) remains unchanged and addresses issue #263. |
|
Claude finished @phernandez's task —— View job PR Review: Datetime JSON Schema Format AnnotationsComprehensive review against Basic Memory team checklist completed. Code Quality & Standards ✅Follows Basic Memory's coding conventions: ✅
Line length (100 chars max): ✅
No commented-out code: ✅
Async patterns: ✅
Testing & Documentation ✅Test Coverage: ✅ Excellent
Edge Cases Covered: ✅
Documentation: ✅
Basic Memory Architecture ✅MCP Tool Compatibility: ✅ This is the core fix
Schema Changes: ✅
Local-First Architecture: ✅
Knowledge Graph Consistency: ✅
Additional Changes (Memory URL Validation) ✅The PR also includes validation improvements that were refined after the main fix: Semantic Correctness: ✅
Permalink Handling: ✅
Security & Performance ✅Security: ✅
Performance: ✅
Error Handling: ✅
Minor ObservationsFormatting Consistency (very minor):
Type Annotations:
Summary✅ LGTM - Ready to MergeThis PR correctly solves issue #263 by adding proper datetime format annotations to JSON schemas for MCP validation. The implementation is:
Key Strengths:
No blocking issues found.The comment about reverting empty URL support makes semantic sense - |
Comprehensive changelog for v0.15.0 release covering: - Critical permalink collision data loss fix - 10+ bug fixes including #330, #329, #328, #312 - 9 new features including cloud sync and subscription validation - Platform improvements (Python 3.13, Windows, Docker) - Enhanced testing and documentation 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
Fixes schema validation failures in MCP clients by adding proper datetime format annotations to JSON schemas.
The issue was that
@field_serializeronly affects JSON serialization, not schema generation. MCP clients validate against the JSON schema which needed explicitformat: "date-time"annotations.Changes:
Annotated[datetime, Field(json_schema_extra)]to all datetime fields@field_serializerfor actual JSON serializationmemory://URLsResolves #263
Generated with Claude Code