feat(core): add default_search_type config setting

[phernandez]

Summary

• Add default_search_type config option (text, vector, or hybrid) so users can set their preferred search mode globally
• When unset (null), preserves existing auto-detection behavior: hybrid if semantic search is enabled, text otherwise
• Users can still override per-query via the search_type parameter on search_notes

Example config:

{
  "default_search_type": "vector"
}

Closes #671

Test plan

• Config tests: default is None, accepts valid values, rejects invalid values
• Search tests: respects config value, falls back to hybrid/text when unset
• Full CI passes

🤖 Generated with Claude Code

[github-actions]

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

---

PR Review: feat(config): add default_search_type setting

• Gather context and read changed files
• Review config.py changes
• Review search.py changes
• Review test files
• Check against team checklist

---

Summary

The feature design is clean and well-scoped — adding a default_search_type config option with None-as-auto-detect is the right approach. However, there is one breaking bug introduced by the refactor, and a few minor style issues.

---

Bug: Existing stub configs break after refactor

Severity: High — this will cause existing tests to fail with AttributeError.

The refactored _default_search_type() now accesses config.default_search_type before config.semantic_search_enabled:

# search.py:38
if config.default_search_type:
    return config.default_search_type

return "hybrid" if config.semantic_search_enabled else "text"

But the pre-existing stub config objects in four existing tests don't have a default_search_type attribute:

• test_search_notes_defaults_to_hybrid_when_semantic_enabled (line 789) — StubConfig dataclass with only semantic_search_enabled
• test_search_notes_defaults_to_fts_when_semantic_disabled (line 813) — same pattern
• test_search_notes_defaults_to_hybrid_when_container_not_initialized (line 978) — type("Cfg", (), {"semantic_search_enabled": True})()
• test_search_notes_defaults_to_fts_when_container_not_initialized_and_semantic_disabled (line 1040) — same pattern

Accessing .default_search_type on these stubs will raise AttributeError. These tests need to be updated to include default_search_type=None in the stub configs. Fix this →

---

Missing literate-programming comment (CLAUDE.md requirement)

The old _semantic_search_enabled_for_text_search() had Trigger/Why/Outcome comments on the except RuntimeError branch explaining the CLI fallback. The refactor removed them:

# Before (old code had this):
except RuntimeError:
    # Trigger: MCP container is not initialized (e.g., `bm tool search-notes` direct call).
    # Why: CLI path still needs the same semantic-default behavior as MCP server path.
    # Outcome: load config directly and keep text-mode retrieval behavior consistent.
    return ConfigManager().config.semantic_search_enabled

The new code has the same branch with no comment. Per CLAUDE.md's literate programming style, non-obvious branches need Trigger/Why/Outcome comments.

---

Minor: Broad exception type in test

# tests/test_config.py — overly broad
def test_default_search_type_rejects_invalid_values(self):
    with pytest.raises(Exception):
        BasicMemoryConfig(default_search_type="invalid")

Should use pytest.raises(ValidationError) (from pydantic). The Exception catch is too broad — it would pass even if the wrong error is raised.

---

Minor: Imports inside test function bodies

The three new tests in test_tool_search.py each import sys and unittest.mock inside the function body. Project convention (and standard pytest practice) is module-level imports. These could be moved to the top of the file.

---

Team Checklist

Item	Status
Follows Basic Memory coding conventions (CLAUDE.md)	⚠️ Missing Trigger/Why/Outcome comment on except RuntimeError
Python 3.12+ type annotations (Literal[...] | None)	✅
100-character line length	✅
No commented-out code	✅
Unit tests for new functions	✅
100% test coverage maintained	⚠️ Existing tests may fail (see bug above); RuntimeError fallback needs default_search_type=None in stubs
No database changes / no migration needed	✅
Preserves auto-detect fallback behavior	✅
No hardcoded secrets	✅
Input validation (Pydantic Literal)	✅
No emojis/unicode in console/log output	✅

[github-actions]

Claude finished @phernandez's task in 4m 20s —— View job

---

PR Review: feat(config): add default_search_type setting

• Read changed files
• Add literate-programming Trigger/Why/Outcome comments to except RuntimeError branch in search.py
• Fix pytest.raises(Exception) → pytest.raises(ValidationError) in test_config.py
• Move import importlib, import sys, from dataclasses import dataclass, and from unittest.mock import MagicMock, patch to module level in test_tool_search.py
• Committed fixes (commit 9d76a40)
• Push to remote (requires approval — please approve the git push tool call)

Changes made

src/basic_memory/mcp/tools/search.py — Added Trigger/Why/Outcome literate-programming comment to the except RuntimeError fallback branch in _default_search_type(), per CLAUDE.md conventions:

except RuntimeError:
    # Trigger: MCP container is not initialized (e.g., CLI `bm tool search-notes` direct call).
    # Why: CLI invocation bypasses the MCP server startup, so get_container() is unavailable.
    #      We still need the same default-type resolution as the MCP server path.
    # Outcome: Load config directly from disk; downstream logic proceeds identically.
    config = ConfigManager().config

tests/test_config.py — Added from pydantic import ValidationError import and tightened the assertion:

def test_default_search_type_rejects_invalid_values(self):
    with pytest.raises(ValidationError):   # was: Exception
        BasicMemoryConfig(default_search_type="invalid")

tests/mcp/test_tool_search.py — Moved import importlib, import sys, from dataclasses import dataclass, and from unittest.mock import MagicMock, patch to module level; removed the six inline imports from the new test function bodies.