fix: correct get_default_project() query to check for True instead of not NULL

[phernandez]

Summary

• Fixed get_default_project() in project_repository.py to use is_(True) instead of is_not(None)
• The incorrect query matched any project with a non-NULL is_default value (both True and False), causing MultipleResultsFound when multiple projects exist

Test plan

• Existing unit tests pass (tests/repository/test_project_repository.py)
• Manual test: Create tenant with 2+ projects, set different defaults via PUT /v2/projects/{id}/default

Unblocks basic-memory-cloud PR #398.

🤖 Generated with Claude Code

[github-actions]

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

---

Code Review: PR #521

Summary

This is a critical bug fix that correctly queries for the default project. The change is minimal, correct, and well-explained in the commit message.

The Bug

The original query:

query = self.select().where(Project.is_default.is_not(None))

This matched any project where is_default was not NULL, including both True and False. When multiple projects existed with different boolean values, SQLAlchemy's find_one() would raise MultipleResultsFound.

The fix:

query = self.select().where(Project.is_default.is_(True))

Now explicitly matches only projects where is_default=True.

---

Review Checklist

✅ 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 (correct use of .is_(True))
• 100-character line length maintained
• No commented-out code blocks

⚠️ Testing & Documentation

• Existing unit tests pass
• Documentation updated (commit message is excellent)
• Test coverage for the specific bug scenario is missing

Issue identified: The existing test test_get_default_project() only tests the happy path with one default project. It doesn't test the bug scenario that caused MultipleResultsFound.

Recommendation: Add a test case that verifies the fix:

@pytest.mark.asyncio
async def test_get_default_project_with_false_values(project_repository: ProjectRepository):
    """Test that get_default_project ignores projects with is_default=False.
    
    Regression test for bug where is_not(None) matched both True and False,
    causing MultipleResultsFound when multiple projects had different boolean values.
    """
    # Create projects with explicit is_default values
    project_true = await project_repository.create({
        "name": "Default Project",
        "path": "/default/path",
        "is_default": True,
    })
    
    project_false = await project_repository.create({
        "name": "Not Default Project",
        "path": "/not-default/path",
        "is_default": False,
    })
    
    project_none = await project_repository.create({
        "name": "Null Default Project",
        "path": "/null/path",
        "is_default": None,
    })
    
    # Should return only the project with is_default=True
    default = await project_repository.get_default_project()
    assert default is not None
    assert default.id == project_true.id
    assert default.name == "Default Project"

This test would have caught the original bug and prevents regression.

✅ Basic Memory Architecture

• Preserves repository pattern for data access
• Maintains consistency with other query methods
• No database migration needed (logic change only)

✅ Security & Performance

• No security implications
• Performance unchanged (still uses indexed column)
• Proper error handling maintained

✅ Compatibility

• No path comparison issues (not applicable)
• No emoji/unicode issues (not applicable)

---

Additional Notes

1. Schema Context: The Project.is_default field is defined as Mapped[Optional[bool]] with nullable=True. This means it can be True, False, or NULL. The fix correctly handles this three-state logic.

2. Used by: This method is called from project_service.py line 413 in initialize_projects(), so this bug would manifest during project initialization when multiple projects exist.

3. Test coverage: While existing tests pass, they don't cover the specific edge case this PR fixes. The test file has is_default: False in some fixtures (lines 23, 38, 48, 64) but never creates multiple projects with mixed True/False/None values in a single test.

4. Cloud blocker: As mentioned, this unblocks basic-memory-cloud PR Optimize TigrisFS sync performance using find command #398, likely where the bug was discovered in a multi-tenant scenario.

---

Recommendation

Approve with minor suggestion: The code fix is correct and necessary. Consider adding the regression test above to prevent this bug from reoccurring. Since Basic Memory maintains 100% test coverage, this test would strengthen the test suite.

The manual test plan mentioned in the PR description is good, but an automated test would be better for long-term maintenance.

---

[github-actions]

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

---

PR Review Complete ✓

I've reviewed PR #521 against the Basic Memory team checklist. This PR addresses a critical bug and adds appropriate test coverage.

---

Summary

The Fix: Changed Project.is_default.is_not(None) to Project.is_default.is_(True) in get_default_project() (line 91 of project_repository.py)

The Bug: The original query matched ANY project with a non-NULL is_default value (both True AND False), causing MultipleResultsFound when multiple projects existed with different boolean values.

The Test: Added test_get_default_project_with_false_values() which creates projects with is_default=True, is_default=False, and is_default=None, then verifies only the True project is returned.

---

Code Quality & Standards ✅

• Follows Basic Memory's coding conventions in CLAUDE.md
• Python 3.12+ type annotations and async patterns maintained
• SQLAlchemy 2.0 best practices - correct use of .is_(True) for boolean comparison
• FastAPI and Typer conventions (not applicable - repository layer only)
• 100-character line length limit maintained
• No commented-out code blocks

Notes:

• The fix uses SQLAlchemy's .is_(True) which is the correct way to match boolean True in SQLAlchemy 2.0
• Consistent with other boolean comparisons in the codebase (see line 96: Project.is_active == True)

---

Testing & Documentation ✅

• Unit tests for new functions/methods (not new function, but bug fix tested)
• Integration tests for new MCP tools (not applicable)
• Test coverage for edge cases - excellent regression test added
• 100% test coverage maintained
• Documentation updated (excellent commit messages)
• CLAUDE.md updated (not needed - no convention changes)

Test Quality: The regression test test_get_default_project_with_false_values() is exactly what was needed:

• Creates three projects with is_default=True, False, and None
• Verifies only the True project is returned
• Would have caught the original bug
• Prevents future regressions
• Clear docstring explaining the bug it tests for

The existing test test_get_default_project() only tested the happy path, so this addition fills a critical gap.

---

Basic Memory Architecture ✅

• MCP tools follow atomic, composable design (not applicable)
• Database changes include Alembic migrations (not needed - logic-only change)
• Preserves local-first architecture principles
• Knowledge graph operations maintain consistency (not applicable)
• Markdown file handling preserves integrity (not applicable)
• AI-human collaboration patterns followed (excellent co-authorship attribution)

Notes:

• Repository pattern maintained correctly
• Method signature unchanged - backward compatible
• Used by project_service.py:413 in initialize_projects()

---

Security & Performance ✅

• No hardcoded secrets or credentials
• Input validation for MCP tools (not applicable)
• Proper error handling and logging maintained
• Performance considerations addressed
• No sensitive data in logs or commits

Performance: Query performance unchanged - still uses indexed is_default column. The .is_(True) comparison may actually be slightly more efficient than .is_not(None) since it's more specific.

---

Compatibility ✅

• File path comparisons must be Windows compatible (not applicable)
• Avoid using emojis and unicode characters in console/log output (not applicable)

---

Review Checklist Assessment

Category	Status	Notes
Code Quality	✅ Pass	Minimal, correct fix with excellent commit message
Testing	✅ Pass	Regression test added covering all edge cases
Architecture	✅ Pass	Repository pattern preserved, no breaking changes
Security	✅ Pass	No security implications
Performance	✅ Pass	No performance impact
Compatibility	✅ Pass	No compatibility concerns

---

Recommendation: Approve and Merge ✅

This is a critical bug fix with excellent test coverage. The PR:

1. Fixes a real bug that causes MultipleResultsFound exceptions
2. Uses the correct SQLAlchemy pattern for boolean comparison
3. Includes a comprehensive regression test
4. Has excellent commit messages explaining the bug and fix
5. Maintains 100% test coverage
6. No breaking changes

The previous Claude review requested this exact test, and the author delivered. Well done!

Unblocks: basic-memory-cloud PR #398

---

| Branch: fix/get-default-project-query