refactor: address PR review feedback

- Remove `# pragma: no cover` from init_cli/mcp/api_logging since
  they are fully tested (issue 1)
- Move logfire to optional dependency `pip install basic-memory[telemetry]`
  while keeping it in dev deps for testing (issue 6)
- Collapse `telemetry.operation` into an alias for `telemetry.scope`
  with a comment explaining the convention (issue 4)
- Rename `routing.resolve_client` span to `routing.client_session`
  to accurately reflect that it covers the full tool execution
  lifetime, not just routing resolution (issue 5)
- Update PR description to clarify two-step setup:
  logfire_enabled config flag + optional LOGFIRE_TOKEN (issue 2)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: phernandez <paul@basicmachines.co>

Author: phernandez

docs/logfire-instrumentation-strategy.md

@@ -187,7 +187,7 @@ Child spans should represent real phases whose duration we care about.
 
 Examples:
 
-- `routing.resolve_client`
+- `routing.client_session`
 - `routing.resolve_project`
 - `routing.resolve_workspace`
 - `api.search.execute`

pyproject.toml

@@ -18,7 +18,6 @@ dependencies = [
     "mcp>=1.23.1",
     "pydantic-settings>=2.6.1",
     "loguru>=0.7.3",
-    "logfire>=4.19.0",
     "pyright>=1.1.390",
     "markdown-it-py>=3.0.0",
     "python-frontmatter>=1.1.0",
@@ -59,6 +58,9 @@ Documentation = "https://github.com/basicmachines-co/basic-memory#readme"
 basic-memory = "basic_memory.cli.main:app"
 bm = "basic_memory.cli.main:app"
 
+[project.optional-dependencies]
+telemetry = ["logfire>=4.19.0"]
+
 [build-system]
 requires = ["hatchling", "uv-dynamic-versioning>=0.7.0"]
 build-backend = "hatchling.build"
@@ -84,6 +86,7 @@ target-version = "py312"
 
 [dependency-groups]
 dev = [
+    "logfire>=4.19.0",
     "gevent>=24.11.1",
     "icecream>=2.1.3",
     "pytest>=8.3.4",

src/basic_memory/config.py

@@ -989,7 +989,7 @@ def _configure_logfire_for_entrypoint(entrypoint: str) -> None:
     )
 
 
-def init_cli_logging() -> None:  # pragma: no cover
+def init_cli_logging() -> None:
     """Initialize logging for CLI commands - file only.
 
     CLI commands should not log to stdout to avoid interfering with
@@ -1000,7 +1000,7 @@ def init_cli_logging() -> None:  # pragma: no cover
     setup_logging(log_level=log_level, log_to_file=True)
 
 
-def init_mcp_logging() -> None:  # pragma: no cover
+def init_mcp_logging() -> None:
     """Initialize logging for MCP server - file only.
 
     MCP server must not log to stdout as it would corrupt the
@@ -1011,7 +1011,7 @@ def init_mcp_logging() -> None:  # pragma: no cover
     setup_logging(log_level=log_level, log_to_file=True)
 
 
-def init_api_logging() -> None:  # pragma: no cover
+def init_api_logging() -> None:
     """Initialize logging for API server.
 
     Cloud mode (BASIC_MEMORY_CLOUD_MODE=1): stdout with structured context

src/basic_memory/mcp/project_context.py

@@ -550,7 +550,7 @@ async def get_project_client(
     if is_factory_mode():
         route_mode = "factory"
         with telemetry.scope(
-            "routing.resolve_client",
+            "routing.client_session",
             project_name=resolved_project,
             route_mode=route_mode,
             workspace_id=workspace,
@@ -568,7 +568,7 @@ async def get_project_client(
     if _explicit_routing() and _force_local_mode():
         route_mode = "explicit_local"
         with telemetry.scope(
-            "routing.resolve_client",
+            "routing.client_session",
             project_name=resolved_project,
             route_mode=route_mode,
         ):
@@ -611,7 +611,7 @@ async def get_project_client(
         if effective_workspace is not None:
             # Config-resolved workspace — pass directly to get_client, skip network lookup
             with telemetry.scope(
-                "routing.resolve_client",
+                "routing.client_session",
                 project_name=resolved_project,
                 route_mode=route_mode,
                 workspace_id=effective_workspace,
@@ -627,7 +627,7 @@ async def get_project_client(
             # No config-based workspace — use resolve_workspace_parameter for discovery
             active_ws = await resolve_workspace_parameter(workspace=None, context=context)
             with telemetry.scope(
-                "routing.resolve_client",
+                "routing.client_session",
                 project_name=resolved_project,
                 route_mode=route_mode,
                 workspace_id=active_ws.tenant_id,
@@ -644,7 +644,7 @@ async def get_project_client(
     # Step 4: Local routing (default)
     route_mode = "local_asgi"
     with telemetry.scope(
-        "routing.resolve_client",
+        "routing.client_session",
         project_name=resolved_project,
         route_mode=route_mode,
     ):

src/basic_memory/telemetry.py

@@ -174,11 +174,9 @@ def scope(name: str, **attrs: Any) -> Iterator[None]:
             yield
 
 
-@contextmanager
-def operation(name: str, **attrs: Any) -> Iterator[None]:
-    """Create a root operation span and mirror its metadata into Loguru context."""
-    with scope(name, **attrs):
-        yield
+# Alias: `operation` signals a root-level boundary (entrypoint, tool invocation),
+# while `scope` signals a nested phase. The distinction is convention only.
+operation = scope
 
 
 @contextmanager

tests/mcp/test_project_context_telemetry.py

@@ -89,6 +89,6 @@ async def test_get_project_client_contextualizes_route_mode(config_manager, monk
 
     span_names = [name for name, _ in spans]
     assert "routing.resolve_project" in span_names
-    assert "routing.resolve_client" in span_names
+    assert "routing.client_session" in span_names
     assert "routing.validate_project" in span_names
     assert {"project_name": "main", "route_mode": "local_asgi"} in contexts

tests/test_telemetry.py

@@ -204,7 +204,7 @@ def test_scope_creates_span_and_nested_log_context(monkeypatch) -> None:
     try:
         with telemetry.contextualize(project_name="main"):
             with telemetry.scope(
-                "routing.resolve_client",
+                "routing.client_session",
                 route_mode="local_asgi",
                 workspace_id=None,
             ):
@@ -213,7 +213,7 @@ def test_scope_creates_span_and_nested_log_context(monkeypatch) -> None:
         logger.remove(sink_id)
 
     assert fake_logfire.span_calls == [
-        ("routing.resolve_client", {"route_mode": "local_asgi"})
+        ("routing.client_session", {"route_mode": "local_asgi"})
     ]
     assert records == [{"project_name": "main", "route_mode": "local_asgi"}]