feat: enable default_project_mode by default

[phernandez]

Summary

• Flips default_project_mode from False to True so MCP tools automatically use the configured default project when no project parameter is specified
• Eliminates the "which project?" back-and-forth for the common single-project case (reported by Drew)
• Updates tool docstrings and AI assistant guide to reflect automatic project resolution
• Discovery mode tests now explicitly set default_project_mode=False to continue testing that path

Test plan

• just fast-check — lint, format, typecheck, impacted tests all pass
• just test — full suite (1572 unit + 226 integration), 0 failures
• Manual: start MCP server without BASIC_MEMORY_MCP_PROJECT, call write_note without project — should use default project automatically

🤖 Generated with Claude Code

[github-actions]

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

---

Review Complete: Enable default_project_mode by default ✅

I've thoroughly reviewed this PR against the Basic Memory team checklist. This is a clean, well-executed change that improves the UX for single-project workflows.

---

Code Quality & Standards ✅

• Follows Basic Memory coding conventions - All changes align with CLAUDE.md standards
• Python 3.12+ type annotations - No type changes needed (simple boolean flip + docstring updates)
• 100-character line length - All lines within limit
• No commented-out code - Clean implementation

Notes:

• The changes are minimal and focused on flipping the default + updating documentation
• Code follows the literate programming style with clear docstrings explaining project resolution

---

Testing & Documentation ✅

• Unit tests updated - tests/test_project_resolver.py:246 now sets default_project_mode=False explicitly
• Integration tests updated - test-int/conftest.py:246 explicitly sets default_project_mode=False for test isolation
• Discovery mode tests preserved - tests/mcp/test_tool_recent_activity.py now explicitly sets default_project_mode=False to test the discovery path
• 100% test coverage maintained - All existing tests pass with the new default
• Documentation updated - Tool docstrings and AI assistant guide reflect automatic project resolution

Outstanding:

• Manual testing checkbox incomplete - The PR description has one unchecked item:

  "Manual: start MCP server without BASIC_MEMORY_MCP_PROJECT, call write_note without project — should use default project automatically"

Observations:

• The test changes are excellent - they explicitly disable default_project_mode=False where needed rather than relying on the old default
• This makes tests more explicit and prevents them from breaking if the default changes again
• The discovery mode tests at tests/mcp/test_tool_recent_activity.py:131-194 now properly set up the test environment

---

Basic Memory Architecture ✅

• Preserves local-first architecture - No structural changes
• Knowledge graph operations unchanged - Only affects project resolution
• MCP tools follow atomic, composable design - No changes to tool behavior beyond project parameter defaults
• ProjectResolver logic intact - The resolution hierarchy remains correct:
  1. Cloud mode check (project required unless discovery allowed)
  2. Constrained project from env var (highest priority)
  3. Explicit project parameter
  4. Default project if default_project_mode=true ← Now enabled by default

Impact Analysis:

• User Experience: Single-project users no longer need to specify project parameter repeatedly
• Multi-project users: Can still pass explicit project parameter or use BASIC_MEMORY_MCP_PROJECT env var
• Backward compatibility: Existing code that passes project explicitly continues to work unchanged

---

Security & Performance ✅

• No security implications - This is a UX change, not a security change
• No performance impact - Same resolution logic, just different default value
• Input validation unchanged - Project validation still happens in the same places

---

Compatibility ✅

• Cross-platform compatible - Boolean config flag, no path operations
• No emoji/unicode in logs - Only in docstrings for readability
• File path handling unchanged - No changes to path comparison logic

---

Summary

This PR successfully addresses the UX friction reported by Drew for single-project workflows. The implementation is clean, well-tested, and maintains backward compatibility.

Strengths

1. Focused change - Only touches what's necessary (config default + docs)
2. Test hygiene - Makes tests explicit about their expectations rather than relying on defaults
3. Documentation - Updates all relevant docstrings to explain the new behavior
4. Backward compatible - Explicit project parameters still work

Recommendations

1. Complete manual testing - Verify the unchecked manual test case before merging
2. Consider release notes - This is a UX-impacting change that should be highlighted in the changelog

Overall Assessment

APPROVE - This is a well-crafted change that improves the default UX while preserving power-user workflows. The test updates demonstrate good engineering discipline by making expectations explicit.

---

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f5ef114cb4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Scope default-project guidance to local mode

This text now tells callers that the default project is used automatically when project is omitted, but that is only valid in local mode with default_project_mode=true. ProjectResolver.resolve still requires an explicit project in cloud mode (it raises when cloud_mode is set and no project is passed), and users can also disable default-project mode; in both cases this guidance leads assistants to issue calls that fail at runtime. Please qualify this instruction so it only applies when that mode is actually active.

Useful? React with 👍 / 👎.

[github-actions]

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

---

Review Complete: Enable default_project_mode by default

I've thoroughly reviewed this PR against the Basic Memory team checklist. This is a well-executed UX improvement with one important issue that needs addressing.

---

Code Quality & Standards ✅

• Follows Basic Memory coding conventions - Changes align with CLAUDE.md standards
• Python 3.12+ type annotations - No type changes needed
• 100-character line length - All lines within limit
• No commented-out code - Clean implementation

Notes: The changes are minimal and focused. Code follows literate programming style with clear docstrings.

---

Testing & Documentation ⚠️

• Unit tests updated - tests/test_project_resolver.py has comprehensive coverage of new default behavior
• Integration tests updated - test-int/conftest.py:246 explicitly sets default_project_mode=False
• Discovery mode tests preserved - tests/mcp/test_tool_recent_activity.py now explicitly sets default_project_mode=False
• 100% test coverage maintained - All 1798 tests passing
• Documentation updated - Tool docstrings and AI assistant guide updated

Outstanding:

• Manual testing checkbox incomplete - PR description has unchecked manual test
• Docstring guidance issue - See "Critical Issue" section below

Observations:

• Excellent test hygiene - tests explicitly set default_project_mode=False where needed rather than relying on defaults
• Discovery mode tests at tests/mcp/test_tool_recent_activity.py:131-194 properly configure test environment

---

Basic Memory Architecture ✅

• Preserves local-first architecture - No structural changes
• Knowledge graph operations unchanged - Only affects project resolution
• MCP tools follow atomic, composable design - No behavioral changes beyond project parameter defaults
• ProjectResolver logic intact - Resolution hierarchy remains correct:
  1. Cloud mode check (project required unless discovery allowed)
  2. Constrained project from env var (BASIC_MEMORY_MCP_PROJECT)
  3. Explicit project parameter
  4. Default project if default_project_mode=true ← Now enabled by default

Impact Analysis:

• Single-project users: No longer need to specify project parameter repeatedly ✅
• Multi-project users: Can still pass explicit project or use env var ✅
• Backward compatibility: Explicit project parameters continue to work ✅

---

Security & Performance ✅

• No security implications - UX change only, no security surface changes
• No performance impact - Same resolution logic, different default value
• Input validation unchanged - Project validation happens in same places

---

Compatibility ✅

• Cross-platform compatible - Boolean config flag, no path operations
• No emoji/unicode in logs - Only in docstrings for readability
• File path handling unchanged - No changes to path comparison logic

---

Critical Issue ⚠️

Problem: The codex review comment at src/basic_memory/mcp/tools/write_note.py:36 is valid and important.

The docstring currently states:

"Server resolves projects in this order: Single Project Mode → project parameter → default project.
Uses default project automatically. Specify project parameter to target a different project."

This guidance is only accurate in local mode with default_project_mode=true. In cloud mode, ProjectResolver.resolve() requires an explicit project (raises ValueError unless allow_discovery=True). See project_resolver.py:171:

if self.cloud_mode:
    if allow_discovery:
        # ... discovery mode ...
    raise ValueError("No project specified. Project is required for cloud mode.")

Impact: AI assistants reading these docstrings will incorrectly believe they can omit the project parameter in cloud mode, leading to runtime errors.

Affected files:

• src/basic_memory/mcp/tools/write_note.py:34-36
• src/basic_memory/mcp/tools/read_note.py:32-34
• src/basic_memory/mcp/tools/build_context.py:52-54

Recommended fix:

# Before:
"""
Project Resolution:
Server resolves projects in this order: Single Project Mode → project parameter → default project.
Uses default project automatically. Specify `project` parameter to target a different project.
"""

# After:
"""
Project Resolution (Local Mode):
When default_project_mode=true (default), the server uses the default project automatically.
Specify `project` parameter to target a different project.

Note: In cloud mode, `project` parameter is always required.
"""

---

Summary

Strengths

1. Clean implementation - Only touches necessary code (config default + docs)
2. Excellent test hygiene - Makes expectations explicit, prevents future breakage
3. Well-documented - Updates docstrings consistently across tools
4. Backward compatible - Explicit parameters still work
5. Addresses real UX friction - Drew's feedback about repetitive project params

Issues to Address

1. Fix docstring guidance - Scope "automatic project resolution" to local mode only
2. Complete manual testing - Verify unchecked manual test case
3. Consider release notes - This is a UX-impacting change worth highlighting

Recommendation

REQUEST CHANGES - The docstring issue could cause runtime errors for AI assistants in cloud mode. This should be fixed before merge.

The fix is straightforward: qualify the "Uses default project automatically" guidance with "In local mode when default_project_mode=true" and note that cloud mode always requires explicit project.

Fix the docstring issue →

---

[github-actions]

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

---

Review Complete: Enable default_project_mode by default ✅

I've thoroughly reviewed this PR against the Basic Memory team checklist. This is an excellent, well-executed UX improvement that went beyond the original scope to fix a critical architectural issue.

---

Code Quality & Standards ✅

• Follows Basic Memory coding conventions - All changes align with CLAUDE.md standards
• Python 3.12+ type annotations - Type annotations are correct and complete
• 100-character line length - All lines within limit
• No commented-out code - Clean implementation
• Literate programming style - ProjectResolver has excellent section comments explaining the unified priority chain

Notes:

• The code in project_resolver.py:134-179 follows the literate programming style perfectly with clear section headers for each priority level
• Commit messages are well-structured with clear context and co-authorship

---

Testing & Documentation ✅

• Unit tests updated - tests/test_project_resolver.py adds 69 new lines of comprehensive test coverage
• Integration tests updated - test-int/conftest.py and discovery mode tests properly configured
• Test coverage maintained at 100% - All 1798 tests passing (1572 unit + 226 integration)
• Documentation updated - Tool docstrings, AI assistant guide, and project_resolver.py all updated
• Edge cases covered - New tests for cloud+default scenarios address the architectural issue

Test Updates:

• tests/test_project_resolver.py:246-310: 7 new tests for cloud mode + default_project_mode combinations
• tests/mcp/test_project_context.py:131-154: 1 new integration test for cloud + default scenario
• tests/mcp/test_tool_recent_activity.py:131-194: Discovery mode tests now explicitly set default_project_mode=False
• test-int/conftest.py:246: Integration fixture explicitly disables default_project_mode for test isolation

Outstanding Manual Testing:

• Manual: start MCP server without BASIC_MEMORY_MCP_PROJECT, call write_note without project — should use default project automatically

---

Basic Memory Architecture ✅

• Preserves local-first architecture - No structural changes to core architecture
• Knowledge graph operations unchanged - Only affects project resolution, not graph operations
• MCP tools follow atomic, composable design - Tool behavior unchanged beyond parameter defaults
• ProjectResolver logic improved - ⭐ Major architectural improvement

Architectural Excellence:

This PR discovered and fixed a critical architectural inconsistency in ProjectResolver.resolve(). The original implementation had cloud mode short-circuiting before checking default_project, making default_project_mode ineffective for cloud users.

The fix unified the resolution into a single linear priority chain that works identically in both modes:

# Unified Priority Chain (project_resolver.py:109-179)
1. ENV_CONSTRAINT — BASIC_MEMORY_MCP_PROJECT env var (highest priority)
2. EXPLICIT — project parameter passed directly
3. DEFAULT — default project when default_project_mode=true
4. Fallback — cloud: CLOUD_DISCOVERY or ValueError; local: NONE

Impact:

• Before: Cloud users couldn't use default_project_mode at all
• After: Both local and cloud users get consistent project resolution behavior
• Single-project users: No longer need to specify project parameter repeatedly
• Multi-project users: Can still pass explicit project or use BASIC_MEMORY_MCP_PROJECT env var
• Backward compatibility: All existing code continues to work unchanged

---

Security & Performance ✅

• No security implications - UX change with architectural fix, no security surface changes
• No performance impact - Same resolution logic, just reordered for consistency
• Input validation unchanged - Project validation still happens in same places

---

Compatibility ✅

• Cross-platform compatible - Boolean config flag, no path operations
• No emoji/unicode in logs - Only in docstrings for readability
• File path handling unchanged - No changes to path comparison logic

---

Docstring Issue Resolution ✅

Previous review concern: Docstrings claimed "Uses default project automatically" without qualifying for cloud mode.

Resolution: Commit ed644b8 updated all affected tool docstrings to state:

"Server resolves projects using a unified priority chain (same in local and cloud modes)"

This is more accurate than the previous review's suggestion because:

1. The architectural fix in commit 5359187 truly unified the resolution logic
2. Both modes now follow the exact same priority chain
3. The only difference is the final fallback behavior (cloud raises error, local returns None)

Affected files verified:

• src/basic_memory/mcp/tools/write_note.py:34-36 ✅
• src/basic_memory/mcp/tools/read_note.py:32-34 ✅
• src/basic_memory/mcp/tools/build_context.py:52-54 ✅

---

Summary

Strengths

1. Scope expansion done right - Discovered architectural inconsistency during implementation and fixed it properly
2. Excellent test hygiene - Tests explicitly set expectations rather than relying on defaults
3. Unified architecture - Eliminated cloud vs local divergence in project resolution
4. Well-documented - Clear docstrings, literate programming style in core logic
5. Backward compatible - All existing code paths preserved
6. Addresses real user friction - Drew's feedback about repetitive project parameters

Commit Quality

• f5ef114: Core feature - flip default_project_mode to True
• 5359187: Architectural fix - unify cloud/local resolution logic
• ed644b8: Documentation fix - clarify unified behavior in docstrings

All three commits have clear messages, proper co-authorship, and signed-off commits.

Recommendations

1. ✅ Docstring issue resolved - No changes needed
2. ⚠️ Complete manual testing - Verify the unchecked manual test case before merging
3. 📝 Highlight in release notes - Both the UX improvement AND the architectural fix deserve mention

Overall Assessment

APPROVE ⭐

This PR exemplifies excellent engineering:

• Started with a simple UX improvement (flip a default)
• Discovered a deeper architectural issue (cloud mode inconsistency)
• Fixed the root cause (unified resolution logic)
• Updated tests and documentation comprehensively
• Maintained 100% test coverage and backward compatibility

The architectural unification is arguably more valuable than the original UX improvement. Both local and cloud users now benefit from consistent, predictable project resolution behavior.

---