fix: normalize underscores in memory:// URLs for build_context

[phernandez]

Summary

Fixes #304 by normalizing underscores to hyphens in memory:// URLs before searching. This allows users to use either part_of or part-of format in their memory:// URLs and get consistent results.

Changes

• Updated context_service.py to normalize paths using generate_permalink() before searching
• Added comprehensive integration tests for underscore normalization
• Tests cover both wildcard patterns and exact paths with underscores

Implementation

The fix applies the "be generous with what you accept" principle:

• Users write part_of [[Target]] in markdown
• System stores permalink as part-of (normalized)
• Users can query with either memory://source/part_of/* or memory://source/part-of/*
• Both formats work correctly through automatic normalization

Testing

New integration tests verify:

• Underscore and hyphen formats both work for wildcard searches
• Exact path lookups work with underscores
• Complex paths with underscores in multiple parts
• Relations with underscores are found correctly

🤖 Generated with Claude Code

Co-Authored-By: Claude noreply@anthropic.com

[github-actions]

Claude finished @phernandez's task —— View job

---

PR Review: Underscore Normalization in memory:// URLs

Review Summary

This PR successfully fixes issue #304 by normalizing underscores to hyphens in memory:// URLs. The implementation is clean, well-tested, and follows Basic Memory conventions. I recommend approval with one minor suggestion below.

---

Code Quality & Standards ✅

• Follows Basic Memory's coding conventions - Code adheres to CLAUDE.md guidelines
• Python 3.12+ type annotations - All type hints present and correct
• SQLAlchemy 2.0 best practices - Repository pattern usage is appropriate
• 100-character line length - All lines within limit
• No commented-out code - Clean implementation

File: src/basic_memory/services/context_service.py

• Clean use of generate_permalink() for normalization at lines 105-106
• Proper variable naming (normalized_path) clearly indicates intent
• Consistent application across both wildcard and exact path lookups
• Metadata correctly updated to use normalized path (line 156)

---

Testing & Documentation ✅

• Comprehensive integration tests - Excellent test coverage in test_build_context_underscore.py
• Edge cases covered - Tests verify:
  • Underscore format: memory://testing/parent-entity/part_of/*
  • Hyphen format: memory://testing/parent-entity/part-of/*
  • Exact paths with underscores
  • Complex paths with underscores in multiple segments (entity names + relation types)
  • Multiple relation types (part_of, related_to, implements_for)
• Clear test documentation - Good docstrings and inline comments

Test Quality Highlights:

• test_build_context_underscore_normalization() - Validates core use case with 4 test scenarios
• test_build_context_complex_underscore_paths() - Tests all permutations of underscore/hyphen combinations
• Tests verify behavior, not just absence of errors

---

Basic Memory Architecture ✅

• Atomic, composable design - Fix is localized and doesn't break existing functionality
• No database changes needed - Works with existing schema
• Local-first principles preserved - File integrity maintained
• Knowledge graph consistency - Normalization ensures consistent graph traversal
• "Be generous with what you accept" - Excellent UX principle application

Architecture Notes:

• The fix leverages existing generate_permalink() utility (src/basic_memory/utils.py:30-144)
• generate_permalink() already handles underscore-to-hyphen conversion (lines 102, 123)
• Smart use of split_extension=False parameter preserves wildcards in pattern matching
• No changes to storage layer needed - normalization happens at query time

---

Security & Performance ✅

• No security concerns - Input normalization before database query is good practice
• Proper error handling - Existing error handling sufficient
• Performance - Negligible overhead from generate_permalink() call
• Input validation - Normalization prevents ambiguity in queries

---

Detailed Code Review

context_service.py:103-119

What changed:

# Before: Used raw path from memory_url_path()
path = memory_url_path(memory_url)
if "*" in path:
    primary = await self.search_repository.search(permalink_match=path, ...)

# After: Normalize path before searching
path = memory_url_path(memory_url)
normalized_path = generate_permalink(path, split_extension=False)
if "*" in normalized_path:
    primary = await self.search_repository.search(permalink_match=normalized_path, ...)

Why this works:

1. memory_url_path() extracts the path portion from memory://folder/entity/relation_type/target
2. generate_permalink(path, split_extension=False) converts part_of → part-of while preserving wildcards
3. Database permalinks are already stored in normalized form (with hyphens)
4. Query now matches regardless of whether user typed underscore or hyphen

Correctness: ✅ This is the right approach - normalize at query time rather than requiring users to remember the storage format.

---

Minor Suggestion (Non-blocking)

Add a comment in the test file explaining the normalization behavior

The tests are excellent, but future developers might benefit from a module-level docstring explaining why both formats should work:

"""Integration test for build_context with underscore in memory:// URLs.

Users naturally write relation types with underscores (part_of, related_to) in markdown,
but the system normalizes these to hyphens (part-of, related-to) for permalinks.
This test ensures that memory:// URLs accept both formats through automatic normalization.
"""

This would make it clearer that underscore support is intentional UX design, not a quirk.

---

Final Checklist

Code Quality & Standards ✅

All items satisfied

Testing & Documentation ✅

All items satisfied - tests are comprehensive

Basic Memory Architecture ✅

All items satisfied - excellent use of existing utilities

Security & Performance ✅

All items satisfied - no concerns

---

Recommendation

✅ APPROVE - This PR is ready to merge.

The implementation is clean, well-tested, and solves a real usability issue. The "be generous with what you accept" principle is perfectly applied here - users can use natural underscore syntax in their memory:// URLs without needing to remember the internal hyphen convention.

Great work on the comprehensive test coverage with multiple edge cases!

---

[github-actions]

Claude finished @phernandez's task —— View job

---

PR Review Complete ✅

I've completed a comprehensive review of this PR against the Basic Memory team checklist. Here's my assessment:

---

✅ Code Quality & Standards

Excellent adherence to Basic Memory conventions:

• ✅ Python 3.12+ type annotations present throughout
• ✅ Async patterns with SQLAlchemy 2.0 properly used
• ✅ 100-character line length maintained
• ✅ No commented-out code blocks
• ✅ Proper import ordering (standard lib, third-party, local)
• ✅ snake_case naming conventions followed

Code quality notes:

• The normalization approach in context_service.py:106 is clean and well-placed
• The simplified upsert_entity logic (entity_repository.py:103-201) is a significant improvement - cleaner error handling with database-level conflict resolution
• Type annotations are complete and accurate

---

✅ Testing & Documentation

Strong test coverage:

• ✅ New integration test file (test_build_context_underscore.py) with comprehensive scenarios
• ✅ Tests cover wildcard patterns, exact paths, and complex underscore combinations
• ✅ Edge cases well-tested (multiple underscores, mixed hyphen/underscore formats)
• ✅ Unit tests updated to reflect simplified upsert logic

Test quality highlights:

• test_build_context_underscore.py:8-111 - Covers all 4 critical scenarios
• test_build_context_underscore.py:114-166 - Tests complex paths with underscores in multiple parts
• Tests follow pytest asyncio patterns correctly

Documentation:

• ✅ PR description clearly explains the fix and implementation
• ✅ Docstrings maintained in updated methods
• ⚠️ Minor suggestion: Consider adding a code comment at context_service.py:105-106 explaining the "be generous" principle for future maintainers

---

✅ Basic Memory Architecture

Excellent architectural alignment:

• ✅ Preserves local-first architecture - files remain source of truth
• ✅ Normalization uses existing generate_permalink() utility (utils.py:30-144)
• ✅ Knowledge graph operations maintain consistency
• ✅ Repository pattern properly followed
• ✅ Database-level conflict resolution (entity_repository.py:138-177) is more robust than previous application-level checks

Architecture notes:

• The fix correctly applies normalization at the query boundary (context_service.py:106)
• This allows both part_of and part-of formats to work seamlessly
• The approach is consistent with how permalinks are generated from markdown files (underscores → hyphens)

---

✅ Security & Performance

No security concerns:

• ✅ No hardcoded secrets or credentials
• ✅ Proper input validation via MemoryUrl schema
• ✅ Project-level security filtering maintained (context_service.py:254, 269-270)
• ✅ No sensitive data in logs

Performance considerations:

• ✅ Normalization is lightweight (single function call)
• ✅ Database-level conflict resolution reduces roundtrips
• ✅ No N+1 query issues introduced
• The simplified upsert logic actually improves performance by removing unnecessary pre-checks

---

📊 Summary

Overall Assessment: APPROVED ✅

This is a well-crafted fix that addresses issue #304 cleanly and comprehensively. The PR demonstrates:

1. Correct problem identification: Users need flexibility in URL format (underscores vs hyphens)
2. Clean solution: Normalize at the query boundary using existing utilities
3. Comprehensive testing: Both integration and unit tests cover edge cases
4. Architecture alignment: Follows Basic Memory's "be generous" principle and local-first design

What I particularly like:

• The entity_repository.py simplification (separate commit 937909f) makes the codebase more maintainable
• Integration tests use realistic scenarios (parent-child relations with different formats)
• The fix is minimally invasive - only 2 lines of actual normalization code

Minor suggestions (non-blocking):

1. Add inline comment at context_service.py:105-106 explaining the normalization rationale
2. Consider documenting this URL flexibility in user-facing docs/README

Commit quality:

• ✅ Clear, descriptive commit messages
• ✅ Proper attribution and sign-off
• ✅ Fixes reference ([BUG] build_context does not support underscore format in memory:// URLs #304) included

---

🔍 Detailed Code References

Key changes:

• context_service.py:105-106 - Normalization logic
• entity_repository.py:103-201 - Simplified upsert with better conflict handling
• test_build_context_underscore.py:1-166 - Comprehensive integration tests

Tests are currently running - waiting for CI results, but code review is complete.

---

[github-actions]

Claude finished @phernandez's task —— View job

---

PR Review Against Team Checklist

I've completed a thorough review of PR #329 against the Basic Memory team standards. Here are my findings:

Tasks

• Review Code Quality & Standards
• Review Testing & Documentation
• Review Basic Memory Architecture
• Review Security & Performance
• Provide final summary and recommendations

---

✅ Code Quality & Standards

Excellent adherence to Basic Memory conventions:

• ✅ Python 3.12+ type annotations - All type hints present and correct throughout
• ✅ Async patterns with SQLAlchemy 2.0 - Proper async/await usage in repository and service layers
• ✅ 100-character line length - All lines comply with the limit
• ✅ No commented-out code - Clean implementation
• ✅ Import order - Follows standard lib → third-party → local pattern
• ✅ Naming conventions - snake_case for functions/variables, PascalCase for classes
• ✅ Repository pattern - Proper data access layer usage

Key implementation highlights:

• context_service.py:106-116 - Clean normalization logic with proper wildcard handling
• context_service.py:122-123 - Exact path normalization for non-wildcard URLs
• Type annotations complete across all modified files
• No linting issues detected

---

✅ Testing & Documentation

Comprehensive test coverage:

• ✅ Integration tests - test_build_context_underscore.py provides excellent coverage
• ✅ Edge cases tested - Multiple scenarios covered:
  • Underscore format: memory://testing/*/part_of/* (line 62)
  • Hyphen format: memory://testing/*/part-of/* (line 80)
  • Exact paths with underscores (line 107)
  • Complex paths with underscores in multiple segments (lines 155-159)
  • Multiple relation types verified (part_of, related_to, implements_for)
• ✅ Unit tests updated - test_entity_repository_upsert.py reflects simplified upsert logic
• ✅ Test quality - Clear docstrings and assertion messages

Documentation:

• ✅ PR description clearly explains the fix and "be generous" principle
• ✅ Docstrings maintained in updated methods
• ✅ Test comments explain expected behavior

Suggestion: Consider adding an inline comment at context_service.py:105-106 explaining the normalization rationale for future maintainers:

# Normalize path to match stored permalinks (underscores → hyphens)
# This allows users to query with either part_of or part-of format

---

✅ Basic Memory Architecture

Excellent architectural alignment:

• ✅ MCP tools remain atomic - build_context maintains composability
• ✅ No database migrations needed - Works with existing schema
• ✅ Local-first principles preserved - Files remain source of truth
• ✅ Knowledge graph consistency - Normalization ensures consistent graph traversal
• ✅ "Be generous with what you accept" - Perfect application of UX principle

Architecture highlights:

• Leverages existing generate_permalink() utility (utils.py:30-144)
• Normalization at query boundary (context_service.py:106) - correct design choice
• The split_extension=False parameter (line 113) cleverly preserves wildcards
• Wildcard handling (lines 109-116) properly splits and normalizes each segment

Bonus improvement: The entity_repository.py simplification (commit 937909f) enhances maintainability:

• Removed 82 lines of complex pre-check logic
• Database-level conflict resolution is more robust
• Cleaner error handling (lines 138-177)

---

✅ Security & Performance

No security concerns identified:

• ✅ No hardcoded secrets - Code is clean
• ✅ Input validation - MemoryUrl schema validates inputs
• ✅ Project-level security - Security filtering maintained (context_service.py:258, 262, 277-278)
• ✅ No sensitive data exposure - Logs contain only debug info

Performance:

• ✅ Minimal overhead - generate_permalink() is a lightweight string operation
• ✅ No N+1 queries - Query patterns unchanged
• ✅ Database efficiency - The simplified upsert actually improves performance by removing pre-checks

Security note: Normalization before database query is good practice - prevents ambiguity in permalink matching.

---

📊 Final Summary

Overall Assessment: APPROVED ✅

This PR successfully fixes issue #304 with a clean, well-tested implementation. The changes demonstrate:

1. Correct problem identification - Users need flexibility in memory:// URL format
2. Clean solution - Normalize at query boundary using existing utilities
3. Comprehensive testing - Integration tests cover all edge cases
4. Architecture alignment - Follows Basic Memory's principles and patterns

What I particularly like:

• Minimal code change - Only 2 lines for normalization (context_service.py:106, 123)
• Wildcard preservation - Smart handling splits paths to preserve * characters
• Test scenarios - Realistic parent-child relations with different formats
• Bonus refactor - The entity_repository.py simplification makes the codebase more maintainable

Commit Quality:

• ✅ Clear, descriptive commit messages
• ✅ Proper issue reference ([BUG] build_context does not support underscore format in memory:// URLs #304)
• ✅ Co-authorship attribution included

CI Status:

Tests are currently running. Code review is complete and ready for merge once CI passes.

---

🔍 Detailed Code References

Key changes:

• context_service.py:106-116 - Wildcard-aware normalization logic
• context_service.py:122-123 - Exact path normalization
• entity_repository.py:103-201 - Simplified upsert with database-level conflict resolution
• test_build_context_underscore.py:1-172 - Comprehensive integration tests covering all scenarios

Test coverage:

• Lines 8-114: Basic underscore/hyphen normalization tests
• Lines 117-172: Complex paths with underscores in multiple segments

---