chore: Cloud compatibility fixes and performance improvements

[phernandez]

Summary

• Cloud-friendly file_service refactor - Better handling for cloud deployment scenarios
• Fix asyncpg naive datetime handling - asyncpg returns UTC timestamps as naive datetimes; now correctly interprets based on cloud_mode
• Simplify logging system - Entry-point specific initialization (CLI, MCP, API) instead of fragile auto-detection
• Fix N+1 query in to_graph_context - Batch entity lookups for relations, reducing ~30 queries to 1
• Fix module-level config bug - Removed import-time config evaluation in project_service.py that broke test isolation

Test plan

• All existing tests pass
• Memory router tests verify graph context building works correctly
• Async client tests properly mock cloud_mode config

🤖 Generated with Claude Code

[github-actions]

Claude finished @phernandez's task in 1m 47s —— View job

---

PR #454 Review: Cloud compatibility fixes and performance improvements

I've completed a comprehensive review of this PR against Basic Memory's team standards. Here are my findings:

---

✅ Code Quality & Standards

Excellent Work:

• Python 3.12+ type annotations: All new code includes proper type hints (e.g., src/basic_memory/utils.py:437-467)
• Async patterns with SQLAlchemy 2.0: Proper async/await usage throughout
• 100-character line length: All files maintain the limit
• Import order: Standard lib, third-party, local imports properly organized
• No commented-out code blocks: Clean, production-ready code

Areas of Concern:

❌ File path comparisons are not Windows-compatible (Critical issue for cross-platform support)

In src/basic_memory/utils.py:326-354, the normalize_file_path_for_comparison() function handles path separators:

# Replace path separators with forward slashes
normalized = normalized.replace("\\", "/")

However, this is incomplete for Windows compatibility. The function only normalizes for comparison, but does NOT preserve Windows paths correctly. Several issues:

1. Line 43-44: The Windows drive letter check path[1] == ":" is good, but incomplete handling
2. Line 349: Replacing backslashes with forward slashes breaks Windows absolute paths like C:\Users\...
3. Missing Path.as_posix() normalization: Should use pathlib's built-in cross-platform handling

Recommendation: Use pathlib.Path consistently for all path operations instead of string manipulation:

from pathlib import Path, PurePosix Path

def normalize_file_path_for_comparison(file_path: str) -> str:
    # Convert to Path, normalize, then to POSIX for comparison
    return Path(file_path).as_posix().lower()

This issue affects multiple files:

• src/basic_memory/file_utils.py: Path operations throughout
• src/basic_memory/services/file_service.py: Line 163, 205, 240, 278, 311, 336
• src/basic_memory/sync/sync_service.py: Line 1099 (relative_to operations)

---

✅ Testing & Documentation

Good:

• Test coverage: Existing tests updated properly (e.g., tests/api/test_async_client.py:18-19)
• README updated: New cloud features documented clearly

Concerns:

⚠️ Missing test cases for the following changes:

1. asyncpg naive datetime handling (src/basic_memory/utils.py:437-467) - No tests verify cloud_mode behavior
2. Logging initialization (src/basic_memory/config.py:530-565) - Entry-point specific logging not tested
3. File service cloud refactor (src/basic_memory/services/file_service.py) - No tests for read_file_bytes() or async Path operations

Recommendation: Add integration tests for cloud mode datetime handling and file service async operations.

---

✅ Basic Memory Architecture

Excellent:

• N+1 query fix (src/basic_memory/api/routers/utils.py:21-41): Batch entity lookup is a solid performance improvement

  # Batch fetch all entities at once
  entity_lookup: dict[int, str] = {}
  if entity_ids_needed:
      entities = await entity_repository.find_by_ids(list(entity_ids_needed))
      entity_lookup = {e.id: e.title for e in entities}

• Logging refactor (src/basic_memory/config.py:530-565): Entry-point specific initialization is clean and prevents fragile auto-detection

• Module-level config bug fix (src/basic_memory/services/project_service.py:38-54): Property-based lazy loading fixes test isolation

Concerns:

⚠️ Import-time side effects still present:

In src/basic_memory/utils.py:454-458, the ensure_timezone_aware() function imports ConfigManager inside the function but then accesses .config which loads config from disk:

from basic_memory.config import ConfigManager

if dt.tzinfo is None:
    config = ConfigManager().config  # <-- Loads config from disk
    if config.cloud_mode_enabled:

This creates hidden I/O during what appears to be a pure utility function. Recommendation: Pass cloud_mode as a parameter instead of loading config implicitly.

---

⚠️ Security & Performance

Good:

• No hardcoded secrets: Clean throughout
• N+1 query optimization: Reduces ~30 queries to 1 (excellent!)
• Proper async I/O: Uses aiofiles throughout

Concerns:

❌ Unicode and emoji in logging (violates "Compatibility" checklist)

From CLAUDE.md requirements:

"Avoid using emojis and unicode characters in console and log output"

But I found these issues:

1. README.md:10: Contains emoji "🚀 Basic Memory Cloud is Live!" (acceptable in docs)
2. No violations in actual log/console output (✅ compliant)

Actually, no violations found - the code correctly avoids emojis in console/log output!

---

✅ Compatibility

Windows Path Handling (Critical):

As detailed above in "Code Quality & Standards", the path comparison logic needs Windows-compatible improvements.

Specific files to update:

• src/basic_memory/utils.py:326-354 - normalize_file_path_for_comparison()
• src/basic_memory/utils.py:16-56 - normalize_project_path()
• All file_service path operations should use Path.as_posix() consistently

---

📋 Team Checklist Results

Code Quality & Standards

• Follows Basic Memory's coding conventions in CLAUDE.md
• Python 3.12+ type annotations and async patterns
• SQLAlchemy 2.0 best practices
• FastAPI and Typer conventions followed
• 100-character line length limit maintained
• No commented-out code blocks

Testing & Documentation

• Unit tests updated (test_async_client.py)
• Missing: Integration tests for cloud mode datetime handling
• Missing: Tests for file_service async operations
• Documentation updated (README, CLAUDE.md unchanged as expected)
• CLAUDE.md unchanged (no convention changes)

Basic Memory Architecture

• Preserves local-first architecture principles
• Knowledge graph operations maintain consistency
• File handling preserves integrity
• N+1 query fix follows performance best practices
• Concern: Import-time I/O in ensure_timezone_aware() (minor)

Security & Performance

• No hardcoded secrets or credentials
• Proper error handling and logging
• Performance optimization (N+1 fix) addressed
• No sensitive data in logs or commits

Compatibility

• Critical: File path comparisons need Windows compatibility improvements
• No emojis/unicode in console/log output (compliant!)

---

🎯 Summary

This PR delivers solid cloud compatibility improvements and an excellent N+1 query fix. The logging refactor is clean and the asyncpg datetime handling is properly architected.

Must Fix Before Merge:

1. Windows path compatibility - Update normalize_file_path_for_comparison() and related functions to use pathlib.Path.as_posix() consistently

Should Fix:

2. Test coverage - Add integration tests for cloud mode datetime handling and file service async operations
3. Import-time I/O - Refactor ensure_timezone_aware() to accept cloud_mode as a parameter instead of loading config

Nice to Have:

4. Documentation - Add docstring examples for ensure_timezone_aware() showing cloud_mode vs local_mode behavior

The code is well-structured and the fixes address real issues. With the Windows path compatibility fixes, this will be ready to merge.

---

[github-actions]

Claude finished @phernandez's task in 1m 57s —— View job

---

PR #454 Review: Cloud compatibility fixes and performance improvements

I've completed a comprehensive review of this PR against Basic Memory's team standards. Great work addressing the previous review feedback! Here are my findings:

---

✅ Code Quality & Standards

Excellent Work:

• Python 3.12+ type annotations: All code properly typed (e.g., src/basic_memory/utils.py:433)
• Async patterns with SQLAlchemy 2.0: Proper async/await usage throughout
• 100-character line length: Maintained across all files
• Import order: Standard lib, third-party, local imports properly organized
• No commented-out code blocks: Clean, production-ready code
• FastAPI and Typer conventions: Followed correctly in src/basic_memory/api/app.py:4 and src/basic_memory/cli/app.py:4

Key Improvements from Previous Review:

✅ Windows path compatibility FIXED: The normalize_file_path_for_comparison() function now uses PurePath.as_posix() (src/basic_memory/utils.py:345) - this is the correct cross-platform approach!

---

✅ Testing & Documentation

Excellent Additions:

• New test file: tests/utils/test_timezone_utils.py - Comprehensive coverage of ensure_timezone_aware() with 8 test cases covering:

  • Already timezone-aware datetimes
  • Naive datetimes in cloud mode (UTC interpretation)
  • Naive datetimes in local mode (local time interpretation)
  • Config loading vs explicit parameter
  • asyncpg naive UTC scenario simulation

• Enhanced file service tests: tests/services/test_file_service.py added tests for:

  • read_file_bytes() method (lines 191-227)
  • Binary file handling
  • Image file handling
  • Error handling for missing files

• README updated: Cloud features documented (line 10)

Coverage Assessment:

✅ Cloud mode datetime handling: Fully tested with explicit test cases
✅ File service async operations: Good coverage for read_file_bytes() and error handling
✅ N+1 query fix: Covered by existing memory router tests

---

✅ Basic Memory Architecture

Excellent Changes:

1. N+1 Query Fix (src/basic_memory/api/routers/utils.py:21-41):

   # Batch fetch all entities at once
   entity_lookup: dict[int, str] = {}
   if entity_ids_needed:
       entities = await entity_repository.find_by_ids(list(entity_ids_needed))
       entity_lookup = {e.id: e.title for e in entities}

   This is a major performance improvement - reduces ~30 queries to 1. Well done!

2. Logging Refactor (src/basic_memory/config.py:530-565):

   • Entry-point specific initialization: init_cli_logging(), init_mcp_logging(), init_api_logging()
   • Clean separation of concerns
   • Prevents fragile auto-detection
   • Cloud mode properly logs to stdout with structured context

3. Module-level Config Bug Fix (src/basic_memory/services/project_service.py:38-54):

   • Property-based lazy loading using @property
   • Fixes test isolation issues
   • No more import-time config evaluation

4. Timezone Handling (src/basic_memory/utils.py:433-466):

   • Properly handles asyncpg's naive UTC datetimes
   • Clean parameter design: explicit cloud_mode parameter with fallback to config
   • Well-documented with clear docstring

Architecture Compliance:

✅ Local-first architecture: Preserved
✅ Repository pattern: Followed in N+1 fix
✅ Async patterns: Used correctly throughout
✅ File service: Cloud-friendly refactor with read_file_bytes() for S3 compatibility

---

✅ Security & Performance

Good:

• No hardcoded secrets: Clean throughout
• N+1 query optimization: Major win! ~30 queries → 1 query
• Proper async I/O: Uses aiofiles for file operations (src/basic_memory/services/file_service.py:245)
• Error handling: Comprehensive with FileOperationError wrapping
• Atomic writes: Maintained with write_file_atomic()

Minor Note:

The ensure_timezone_aware() function loads config when cloud_mode=None (src/basic_memory/utils.py:454-456), but the new tests show this is intentional with an explicit parameter option to skip config loading. The test at line 53-64 (test_explicit_cloud_mode_skips_config_loading) validates this design. ✅

---

✅ Compatibility

Windows Path Handling:

✅ FIXED: normalize_file_path_for_comparison() now uses PurePath(file_path).as_posix() for cross-platform consistency (src/basic_memory/utils.py:345)

Console/Log Output:

✅ No emojis in logging: Verified no emoji usage in logger statements or console output

• Grep search found emojis only in:
  • README.md (documentation - acceptable)
  • Test files (tests/test_production_cascade_delete.py - test output only)
  • Spec files (documentation)
• No violations in production code ✅

---

📋 Team Checklist Results

Code Quality & Standards

• Follows Basic Memory's coding conventions in CLAUDE.md
• Python 3.12+ type annotations and async patterns
• SQLAlchemy 2.0 best practices
• FastAPI and Typer conventions followed
• 100-character line length limit maintained
• No commented-out code blocks

Testing & Documentation

• Unit tests for new functions/methods (test_timezone_utils.py)
• Test coverage for edge cases (asyncpg scenarios, binary files)
• Documentation updated (README)
• CLAUDE.md unchanged (no convention changes needed)

Basic Memory Architecture

• Database changes follow async patterns
• Preserves local-first architecture principles
• Knowledge graph operations maintain consistency (N+1 fix)
• File handling preserves integrity (atomic writes maintained)
• AI-human collaboration patterns followed

Security & Performance

• No hardcoded secrets or credentials
• Proper error handling and logging
• Performance optimization addressed (N+1 query fix)
• No sensitive data in logs or commits

Compatibility

• File path comparisons are Windows compatible (PurePath.as_posix())
• No emojis/unicode in console and log output

---

🎯 Summary

This PR successfully addresses all critical issues from the previous review and delivers solid improvements:

Key Achievements:

1. ✅ Windows path compatibility - Fixed using PurePath.as_posix()
2. ✅ Comprehensive test coverage - New timezone and file service tests
3. ✅ N+1 query optimization - Major performance improvement
4. ✅ Clean logging architecture - Entry-point specific initialization
5. ✅ Cloud-ready datetime handling - Proper asyncpg naive UTC handling
6. ✅ Test isolation fix - Property-based config loading

Code Quality:

• Well-structured with clear separation of concerns
• Excellent test coverage for new functionality
• Proper error handling throughout
• Cross-platform compatible

Recommendation: ✅ Approved - Ready to merge

This PR demonstrates excellent collaborative development with thorough responses to review feedback. The code is production-ready and follows all Basic Memory team standards.

---

[github-actions]

Claude finished @phernandez's task in 2m 29s —— View job

---

PR #454 Review: Cloud compatibility fixes and performance improvements

I've completed a thorough review of this PR against Basic Memory's team standards. This PR addresses critical cloud deployment issues and includes an excellent performance optimization.

---

✅ Code Quality & Standards

Excellent Work:

• Python 3.12+ type annotations: Properly typed throughout (src/basic_memory/utils.py:433, src/basic_memory/api/routers/utils.py:21-41)
• Async patterns with SQLAlchemy 2.0: Correct async/await usage in all services
• 100-character line length: Maintained across all changed files
• Import order: Standard lib, third-party, local imports properly organized
• No commented-out code blocks: Clean, production-ready code
• FastAPI and Typer conventions: Followed correctly (src/basic_memory/api/app.py:33, src/basic_memory/cli/app.py:5)

Key Improvements:

✅ Windows path compatibility VERIFIED:

• normalize_file_path_for_comparison() now uses PureWindowsPath.as_posix() (src/basic_memory/utils.py:345)
• This is the correct cross-platform approach for path normalization
• Handles both Windows and Unix paths properly

✅ Entry-point logging initialization (src/basic_memory/config.py:530-565):

• Clean separation: init_cli_logging(), init_mcp_logging(), init_api_logging()
• Cloud mode properly logs to stdout with structured context
• Local mode logs to file only
• Prevents stdout corruption in MCP and CLI contexts

---

✅ Testing & Documentation

Comprehensive Test Coverage:

✅ New test file: tests/utils/test_timezone_utils.py (97 lines)

• 8 test cases covering timezone-aware datetime handling
• Tests asyncpg naive UTC scenario (test_asyncpg_naive_utc_scenario)
• Tests explicit cloud_mode parameter vs config loading
• Validates both cloud mode (UTC interpretation) and local mode (local time)

✅ Enhanced file service tests: tests/services/test_file_service.py

• New test_read_file_bytes() - binary file handling
• New test_read_file_bytes_image() - image file simulation
• New test_read_file_bytes_missing_file() - error handling
• Validates cloud compatibility with S3FileService interface

✅ Async client tests updated: tests/api/test_async_client.py

• Properly mocks cloud_mode config to prevent test pollution
• Tests both ASGI transport (local) and HTTP transport (cloud) modes

✅ Documentation:

• README.md updated with cloud features (line 10+)
• Docstrings comprehensive with examples (src/basic_memory/utils.py:433-450)

---

✅ Basic Memory Architecture

Major Performance Win:

🎯 N+1 Query Fix (src/basic_memory/api/routers/utils.py:21-41):

# First pass: collect all entity IDs needed for relations
entity_ids_needed: set[int] = set()
for context_item in context_result.results:
    for item in [context_item.primary_result] + context_item.observations + context_item.related_results:
        if item.type == SearchItemType.RELATION:
            if item.from_id:
                entity_ids_needed.add(item.from_id)
            if item.to_id:
                entity_ids_needed.add(item.to_id)

# Batch fetch all entities at once
entity_lookup: dict[int, str] = {}
if entity_ids_needed:
    entities = await entity_repository.find_by_ids(list(entity_ids_needed))
    entity_lookup = {e.id: e.title for e in entities}

Impact: Reduces ~30 individual entity queries to 1 batch query. This is a significant performance improvement for graph context building.

Architecture Compliance:

✅ Cloud-friendly file_service refactor (src/basic_memory/services/file_service.py:224-258):

• New read_file_bytes() method for binary file handling
• Maintains interface compatibility with S3FileService
• Uses aiofiles for true async I/O
• Proper path handling with Path.as_posix() throughout

✅ Module-level config bug FIXED (src/basic_memory/services/project_service.py:38-54):

• Removed import-time config evaluation
• Now uses @property for lazy loading
• Fixes test isolation issues

✅ Timezone handling (src/basic_memory/utils.py:433-466):

• Properly handles asyncpg's naive UTC datetimes
• Explicit cloud_mode parameter with config fallback
• Well-documented behavior differences between cloud and local modes

✅ Repository pattern: Followed correctly in N+1 fix
✅ Local-first architecture: Preserved throughout
✅ Async patterns: Used consistently

---

✅ Security & Performance

Security:

• ✅ No hardcoded secrets or credentials
• ✅ Proper error handling with FileOperationError wrapping
• ✅ Input validation maintained

Performance:

• 🎯 N+1 query optimization: Major win (~30 queries → 1 query)
• ✅ Proper async I/O: Uses aiofiles for non-blocking file operations
• ✅ Semaphore controls: Maintains concurrency limits to prevent OOM
• ✅ Atomic writes: Preserved with write_file_atomic()

Minor Note:

The ensure_timezone_aware() function loads config when cloud_mode=None (src/basic_memory/utils.py:454-456), but this is intentional design with an explicit parameter option to skip config loading. Test coverage validates this at tests/utils/test_timezone_utils.py:53-64. ✅

---

✅ Compatibility

Cross-Platform:

✅ Windows path handling FIXED:

• normalize_file_path_for_comparison() uses PureWindowsPath.as_posix()
• Proper handling in file_service.py path operations (lines 163, 205, 240, 278, 311, 336)
• All paths use Path.as_posix() for cross-platform consistency

Console/Log Output:

✅ No emojis in production logs: Verified clean

• Emojis only in documentation (README.md) and test output - acceptable
• No violations in production code

---

📋 Team Checklist Results

Code Quality & Standards

• Follows Basic Memory's coding conventions in CLAUDE.md
• Python 3.12+ type annotations and async patterns
• SQLAlchemy 2.0 best practices
• FastAPI and Typer conventions followed
• 100-character line length limit maintained
• No commented-out code blocks

Testing & Documentation

• Unit tests for new functions/methods
• Test coverage for edge cases (asyncpg, binary files, cloud mode)
• Documentation updated (README)
• CLAUDE.md unchanged (no convention changes needed)

Basic Memory Architecture

• Database changes follow async patterns
• Preserves local-first architecture principles
• Knowledge graph operations maintain consistency
• File handling preserves integrity (atomic writes)
• AI-human collaboration patterns followed
• Repository pattern used correctly

Security & Performance

• No hardcoded secrets or credentials
• Proper error handling and logging
• Performance optimization addressed (N+1 query fix - major improvement)
• No sensitive data in logs or commits

Compatibility

• File path comparisons are Windows compatible (PureWindowsPath.as_posix())
• No emojis/unicode in console and log output

---

🎯 Summary

This PR successfully addresses critical cloud deployment compatibility issues while delivering a significant performance optimization.

Key Achievements:

1. ✅ N+1 Query Optimization - Reduces ~30 queries to 1 (major performance win)
2. ✅ asyncpg Naive Datetime Handling - Proper UTC interpretation in cloud mode
3. ✅ Logging Refactor - Entry-point specific initialization prevents fragile auto-detection
4. ✅ Windows Path Compatibility - Fixed using PureWindowsPath.as_posix()
5. ✅ Comprehensive Test Coverage - New tests for timezone utils and file service
6. ✅ Module-level Config Bug Fix - Property-based lazy loading fixes test isolation

Code Quality:

• Well-structured with clear separation of concerns
• Excellent test coverage for new functionality (timezone utils, file service bytes)
• Proper error handling throughout
• Cross-platform compatible
• Follows all Basic Memory team standards

CI Status:

• Tests are currently running (2 test runs in progress)
• PR title check: ✅ Passed
• No failed runs

---

✅ Recommendation: Ready to Merge

This PR demonstrates excellent engineering practices with thorough responses to previous review feedback. The N+1 query fix alone is a substantial improvement that will benefit all graph context operations. All team checklist items are satisfied, and the code is production-ready.

Previous review concerns addressed:

• ✅ Windows path compatibility fixed
• ✅ Test coverage gaps filled (timezone utils, file service)
• ✅ Logging architecture cleaned up
• ✅ Module-level config bug resolved

Great work! 🎉