fix: Add explicit type annotations to MCP tool parameters

[phernandez]

Description

Fixes #393

This PR adds explicit type annotations to MCP tool parameters that were causing tools to be skipped by MCP clients like qwen code.

Changes

• Add TagType annotation to write_note.tags parameter
• Replace StringOrInt type alias with inline union type in build_context.depth
• Ensures MCP protocol compatibility with clients that require explicit types

Root Cause

MCP clients require all tool parameters to have explicit, JSON-serializable type annotations. The issues were:

1. write_note: The tags parameter had no type annotation (tags=None)
2. build_context: Used Python 3.12+ type alias (type StringOrInt = str | int) which doesn't serialize to JSON schema

Testing

• Existing tests should pass
• Issue reporter should verify with qwen code CLI

---

Generated with Claude Code

[github-actions]

Claude finished @phernandez's task —— View job

---

PR Review Complete ✅

I've reviewed this PR against the Basic Memory team checklist. Here's my analysis:

Tasks

• Read CLAUDE.md for project context
• Review changed files
• Check test coverage
• Verify against team checklist
• Provide detailed feedback

---

Review Summary

This PR successfully addresses issue #393 by adding explicit type annotations to MCP tool parameters. The changes are minimal, focused, and follow the project's coding standards.

✅ Approved Changes

1. build_context.py - Type Alias Replacement

Change: Removed type StringOrInt = str | int type alias and replaced with inline union type str | int | None

Analysis:

• ✅ Correct approach - Python 3.12+ type aliases don't serialize to JSON schema for MCP
• ✅ Preserves exact same type semantics
• ✅ Line 40: depth: str | int | None = 1 is properly typed
• ✅ Existing logic at lines 93-99 handles string-to-int conversion

2. write_note.py - Tags Parameter Type

Change: Added explicit TagType annotation to tags parameter

Analysis:

• ✅ Fixes missing type annotation (tags=None → tags: TagType = None)
• ✅ TagType = Union[List[str], str, None] is properly defined
• ⚠️ Minor Issue: Duplicate TagType definition (lines 17 and 20)

---

Team Checklist Review

✅ Code Quality & Standards

• Follows Basic Memory's coding conventions in CLAUDE.md
• Python 3.12+ type annotations and async patterns
• 100-character line length limit maintained
• No commented-out code blocks
• Proper import organization

✅ Testing & Documentation

• Existing tests cover the changes:
  • test_tool_write_note.py: 32 tests covering write_note with various tag formats (lines 20-26, 277-300)
  • test_tool_build_context.py: Test for string depth parameter (lines 128-144)
• No new tests needed: Changes are type-level only, existing tests verify runtime behavior
• Documentation: PR description clearly explains root cause and fix

✅ Basic Memory Architecture

• No database schema changes required
• MCP tools maintain atomic, composable design
• Preserves MCP protocol compatibility (primary goal)
• No changes to tool behavior, only type annotations

✅ Security & Performance

• No security concerns
• No performance impact
• Input validation unchanged
• Error handling preserved (build_context lines 93-99)

---

Issues Found

⚠️ Minor: Duplicate Type Definition

File: src/basic_memory/mcp/tools/write_note.py:17-20

# Define TagType as a Union that can accept either a string or a list of strings or None
TagType = Union[List[str], str, None]

# Define TagType as a Union that can accept either a string or a list of strings or None
TagType = Union[List[str], str, None]

Impact: Low - Python will use the second definition, no runtime error
Recommendation: Remove one of the duplicate definitions (lines 17-18 or 19-20)

---

Recommendations

1. Fix Duplicate Definition (Required)

Remove the duplicate TagType definition in write_note.py. Keep lines 16-17:

# Define TagType as a Union that can accept either a string or a list of strings or None
TagType = Union[List[str], str, None]

And remove lines 19-20.

2. Consider Test Enhancement (Optional)

While existing tests cover the functionality, you could add a specific test to verify MCP schema generation includes proper type annotations. This would catch similar issues in the future.

3. Verify with qwen code (Recommended)

The issue reporter should test with qwen code CLI to confirm the fix resolves the original problem.

---

Conclusion

Status: ✅ Approve with minor fix required

The PR correctly addresses the root cause and follows project conventions. The only issue is a duplicate type definition that should be removed before merging. Once fixed, this PR is ready to merge.

Excellent work on identifying the MCP protocol compatibility issue and providing a clean, focused fix! 🎯

---

Branch: claude/issue-393-20251022-1244