Skip to content

Commit 912df8a

Browse files
phernandezclaude
andcommitted
fix(cli): show embedding progress when bm project add indexes
bm project add indexed through one foreground API request that embeds inline and prints nothing until it returns; a 1000-note project sat silent for two minutes. The add now runs the same pass as bm project index: the search pass, then the embedding pass under the existing Rich progress bar from bm reindex. Embeddings follow semantic_search_enabled without the warning an explicit reindex prints. Reindex now matches --project by permalink. Config reconciliation stores normalized names, so `new_default` missed the `new-default` row it had just written. The reconciliation also normalizes the config key at add time, as initialization already does elsewhere; the integration test asserts that. Part of #1635. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01APFUk2bjEwMptMQqpRhjea Signed-off-by: phernandez <paul@basicmachines.co>
1 parent c15c040 commit 912df8a

7 files changed

Lines changed: 180 additions & 24 deletions

File tree

‎src/basic_memory/cli/commands/command_utils.py‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -132,17 +132,6 @@ async def report_project_readiness(project: str) -> None:
132132
console.print(f"[dim]{escape(project_item.name)}: {summary}[/dim]")
133133

134134

135-
async def index_project_and_report_readiness(project: str) -> None:
136-
"""Index a project, then say what state that left it in.
137-
138-
One coroutine so the caller opens the database once for both steps:
139-
`run_with_cleanup` shuts the engine down on exit, so a second call would pay
140-
the reconnect and the migration check over again.
141-
"""
142-
await run_project_index(project, force_full=True, run_in_background=False)
143-
await report_project_readiness(project)
144-
145-
146135
async def get_project_info(project: str):
147136
"""Get project information via API endpoint."""
148137
# Deferred: ToolError lives in FastMCP's runtime, which must not load at CLI startup (#886).

‎src/basic_memory/cli/commands/db.py‎

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,9 @@
1717
from rich.progress import Progress, SpinnerColumn, TextColumn, BarColumn, TaskProgressColumn
1818

1919
from basic_memory.cli.app import app
20-
from basic_memory.cli.commands.command_utils import run_with_cleanup
20+
from basic_memory.cli.commands.command_utils import report_project_readiness, run_with_cleanup
2121
from basic_memory.config import ConfigManager, ProjectMode
22+
from basic_memory.utils import generate_permalink
2223

2324
console = Console()
2425
REINDEX_ERROR_SUMMARY_MAX_LENGTH = 240
@@ -304,6 +305,33 @@ def run_reindex_command(
304305
)
305306

306307

308+
async def index_project_and_report_readiness(project: str) -> None:
309+
"""Index a just-added project the way `bm project index` does, then report readiness.
310+
311+
`bm project add` used to index through the API in one foreground request,
312+
which embeds inline and prints nothing until it returns: a 1000-note project
313+
sat silent for two minutes between "added successfully" and the final count
314+
(#1635). `_reindex` runs the search pass and then the embedding pass under the
315+
progress bar `bm reindex` shows, so the add reports progress and leaves the
316+
project in the state its own remedy, `bm project index`, would.
317+
318+
It is incremental (`full=False`) for the same reason the remedy is: change
319+
detection sees every file of a never-indexed project as new, and an adopted,
320+
already-indexed project only redoes what changed.
321+
"""
322+
app_config = ConfigManager().config
323+
# Semantic search off is a supported configuration, not a failure, so the
324+
# embedding pass is skipped without the warning an explicit reindex prints.
325+
await _reindex(
326+
app_config,
327+
search=True,
328+
embeddings=app_config.semantic_search_enabled,
329+
full=False,
330+
project=project,
331+
)
332+
await report_project_readiness(project)
333+
334+
307335
@app.command()
308336
def reindex(
309337
embeddings: bool = typer.Option(
@@ -377,7 +405,14 @@ async def _reindex(
377405
projects = await project_repository.get_active_projects(session)
378406

379407
if project:
380-
projects = [p for p in projects if p.name == project]
408+
# Trigger: the caller names the project as typed, e.g. `new_default`.
409+
# Why: config reconciliation above stores normalized names
410+
# (`new-default`), so exact name equality can miss the project
411+
# the caller just registered; the API resolves by permalink too.
412+
# Outcome: `bm project add new_default` and `bm project index
413+
# new_default` index the project they name.
414+
project_permalink = generate_permalink(project)
415+
projects = [p for p in projects if p.permalink == project_permalink]
381416
if not projects:
382417
# Check if it's a cloud-only project — those can't be reindexed locally
383418
project_mode = app_config.get_project_mode(project)

‎src/basic_memory/cli/commands/project.py‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,9 @@
2828
)
2929
from basic_memory.cli.commands.command_utils import (
3030
get_project_info,
31-
index_project_and_report_readiness,
3231
run_with_cleanup,
3332
)
34-
from basic_memory.cli.commands.db import run_reindex_command
33+
from basic_memory.cli.commands.db import index_project_and_report_readiness, run_reindex_command
3534
from basic_memory.cli.commands.routing import force_routing, validate_routing_flags
3635
from basic_memory.config import BasicMemoryConfig, ConfigManager, ProjectEntry, ProjectMode
3736
from basic_memory.mcp.async_client import get_client, resolve_configured_workspace
@@ -820,7 +819,7 @@ def _abort_after_project_created(
820819
deleting it to tidy up an error message would discard what they asked for.
821820
"""
822821
# A typer.Exit carries no message of its own -- the failing step already
823-
# printed one (run_project_index does) -- so only the state and remedy are
822+
# printed one (the reindex pass does) -- so only the state and remedy are
824823
# missing. Anything else still needs its message shown.
825824
detail = "" if isinstance(error, typer.Exit) else f": {error}"
826825
console.print(f"[yellow]Project '{name}' was created, but {step} failed{detail}[/yellow]")

‎test-int/cli/test_project_commands_integration.py‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,9 @@ def test_remove_main_project(app, app_config, config_manager):
170170
assert result.exit_code == 0
171171
config_after_list = config_manager.load_config()
172172
assert "main" not in config_after_list.projects
173-
assert "new_default" in config_after_list.projects
173+
# `project add` indexes through the same pass as `bm project index`
174+
# (#1635), whose config reconciliation stores the normalized key.
175+
assert "new-default" in config_after_list.projects
174176

175177

176178
def test_local_project_remove_keeps_files_without_delete_notes(app, app_config, config_manager):

‎tests/cli/test_command_utils.py‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,13 @@
11
"""Tests for CLI command utilities."""
22

3+
from collections.abc import AsyncIterator
4+
from contextlib import asynccontextmanager
5+
from types import SimpleNamespace
6+
from unittest.mock import AsyncMock
7+
8+
import pytest
9+
10+
import basic_memory.cli.commands.command_utils as command_utils
311
import basic_memory.index.note_content_materialization as note_content_materialization
412
import basic_memory.db as db
513
import basic_memory.index.local_schedulers as local_schedulers
@@ -38,3 +46,49 @@ async def work() -> int:
3846

3947
assert result == 42
4048
assert calls == ["work", "drain-materializations", "drain-background", "shutdown"]
49+
50+
51+
@pytest.mark.asyncio
52+
@pytest.mark.parametrize(
53+
"response,expected",
54+
[
55+
({"message": "Indexing started in background"}, "Indexing started in background"),
56+
(
57+
{"total_files": 3, "enqueued_files": 2, "enqueued_batches": 1, "deleted_files": 0},
58+
"Indexed 2/3 files (batches: 1, deleted orphans: 0)",
59+
),
60+
],
61+
)
62+
async def test_run_project_index_reports_background_and_foreground_responses(
63+
monkeypatch, response: dict[str, object], expected: str
64+
):
65+
"""`bm project add` no longer calls this; cloud project indexing still does."""
66+
printed: list[str] = []
67+
68+
@asynccontextmanager
69+
async def fake_get_client(project_name: str | None = None) -> AsyncIterator[object]:
70+
yield object()
71+
72+
class FakeProjectClient:
73+
def __init__(self, client: object) -> None:
74+
pass
75+
76+
async def index(self, external_id: str, **kwargs: bool) -> dict[str, object]:
77+
assert external_id == "project-ext"
78+
return response
79+
80+
monkeypatch.setattr(command_utils, "get_client", fake_get_client)
81+
monkeypatch.setattr(
82+
command_utils,
83+
"get_active_project",
84+
AsyncMock(return_value=SimpleNamespace(external_id="project-ext")),
85+
)
86+
monkeypatch.setattr(command_utils, "ProjectClient", FakeProjectClient)
87+
monkeypatch.setattr(
88+
command_utils.console, "print", lambda message="", *a, **k: printed.append(str(message))
89+
)
90+
91+
await command_utils.run_project_index("research")
92+
93+
[line] = printed
94+
assert expected in line.replace("[green]", "").replace("[/green]", "")

‎tests/cli/test_db_reindex.py‎

Lines changed: 81 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ def _configure_embedding_runtime(
7373
) -> tuple[SimpleNamespace, AsyncMock, list[str]]:
7474
"""Install the runtime boundaries needed to exercise the real reindex command."""
7575
app_config = _stub_app_config()
76-
project = SimpleNamespace(id=1, name="foo", path="/tmp/foo")
76+
project = SimpleNamespace(id=1, name="foo", permalink="foo", path="/tmp/foo")
7777
printed_lines: list[str] = []
7878
project_index = AsyncMock(
7979
return_value=SimpleNamespace(
@@ -282,7 +282,7 @@ async def test_reindex_project_full_uses_core_project_index_and_reports_summary(
282282
session_maker,
283283
):
284284
app_config = _stub_app_config()
285-
project = SimpleNamespace(id=1, name="foo", path="/tmp/foo")
285+
project = SimpleNamespace(id=1, name="foo", permalink="foo", path="/tmp/foo")
286286
project_index = AsyncMock(
287287
return_value=SimpleNamespace(
288288
total_files=3,
@@ -343,7 +343,7 @@ async def test_reindex_embeddings_only_full_passes_force_full_to_vector_reindex(
343343
session_maker,
344344
):
345345
app_config = _stub_app_config()
346-
project = SimpleNamespace(id=1, name="foo", path="/tmp/foo")
346+
project = SimpleNamespace(id=1, name="foo", permalink="foo", path="/tmp/foo")
347347
printed_lines: list[str] = []
348348
vector_reindex_calls: list[dict[str, object]] = []
349349

@@ -443,7 +443,7 @@ async def test_reindex_embeddings_only_warns_when_project_has_no_indexed_entitie
443443
):
444444
"""Embeddings-only mode explains that it cannot discover project files."""
445445
app_config = _stub_app_config()
446-
project = SimpleNamespace(id=1, name="foo", path="/tmp/foo")
446+
project = SimpleNamespace(id=1, name="foo", permalink="foo", path="/tmp/foo")
447447
printed_lines: list[str] = []
448448

449449
class StubProjectRepository:
@@ -530,8 +530,8 @@ async def test_reindex_recovers_stuck_materializations_before_scan(monkeypatch,
530530
as a missing file. Recovery must re-drive stuck rows before each project scan."""
531531
app_config = _stub_app_config()
532532
projects = [
533-
SimpleNamespace(id=1, name="foo", path="/tmp/foo"),
534-
SimpleNamespace(id=2, name="bar", path="/tmp/bar"),
533+
SimpleNamespace(id=1, name="foo", permalink="foo", path="/tmp/foo"),
534+
SimpleNamespace(id=2, name="bar", permalink="bar", path="/tmp/bar"),
535535
]
536536
call_order: list[str] = []
537537

@@ -721,7 +721,7 @@ async def test_reindex_full_does_not_double_embed(monkeypatch, session_maker):
721721
"""A full reindex (search + embeddings) must embed once: the FTS rebuild runs
722722
with embeddings=False so only the explicit vector phase calls the provider."""
723723
app_config = _stub_app_config()
724-
project = SimpleNamespace(id=1, name="foo", path="/tmp/foo")
724+
project = SimpleNamespace(id=1, name="foo", permalink="foo", path="/tmp/foo")
725725
vector_reindex_calls: list[dict[str, object]] = []
726726
project_index = AsyncMock(
727727
return_value=SimpleNamespace(
@@ -878,3 +878,77 @@ def test_reindex_embedding_success_reports_index_and_model_and_exits_zero(
878878
) in output
879879
assert "Representative error:" not in output
880880
assert "Reindex complete!" in output
881+
882+
883+
# --- `bm project add` indexing (#1635) ---
884+
885+
886+
@pytest.mark.asyncio
887+
@pytest.mark.parametrize("semantic_search_enabled", [True, False])
888+
async def test_project_add_indexing_runs_the_reindex_pass_then_reports_readiness(
889+
monkeypatch, semantic_search_enabled: bool
890+
):
891+
"""`project add` reuses the reindex pass, so its embedding phase shows the progress bar.
892+
893+
The old path made one foreground API request that embedded inline and printed
894+
nothing until it returned. Embeddings follow the config here, without the
895+
"Semantic search is not enabled" warning an explicit reindex prints.
896+
"""
897+
app_config = _stub_app_config(semantic_search_enabled=semantic_search_enabled)
898+
monkeypatch.setattr(db_cmd, "ConfigManager", lambda: SimpleNamespace(config=app_config))
899+
steps: list[tuple[str, object]] = []
900+
901+
async def fake_reindex(config, **kwargs):
902+
assert config is app_config
903+
steps.append(("reindex", kwargs))
904+
905+
async def fake_report(project: str) -> None:
906+
steps.append(("readiness", project))
907+
908+
monkeypatch.setattr(db_cmd, "_reindex", fake_reindex)
909+
monkeypatch.setattr(db_cmd, "report_project_readiness", fake_report)
910+
911+
await db_cmd.index_project_and_report_readiness("research")
912+
913+
assert steps == [
914+
(
915+
"reindex",
916+
{
917+
"search": True,
918+
"embeddings": semantic_search_enabled,
919+
"full": False,
920+
"project": "research",
921+
},
922+
),
923+
("readiness", "research"),
924+
]
925+
926+
927+
@pytest.mark.asyncio
928+
async def test_reindex_matches_the_project_by_permalink(monkeypatch, session_maker):
929+
"""`new_default` names the project reconciliation stored as `new-default`.
930+
931+
Exact name equality missed it, so `bm project add new_default` reported its
932+
own index pass as "Project 'new_default' not found".
933+
"""
934+
stats = _vector_stats(total_entities=1, embedded=1, skipped=0, errors=0)
935+
_, project_index, _ = _configure_embedding_runtime(monkeypatch, session_maker, stats)
936+
stored = SimpleNamespace(id=1, name="new-default", permalink="new-default", path="/tmp/n")
937+
938+
class NormalizedProjectRepository:
939+
async def get_active_projects(self, session):
940+
return [stored]
941+
942+
monkeypatch.setattr("basic_memory.repository.ProjectRepository", NormalizedProjectRepository)
943+
944+
await db_cmd._reindex(
945+
_stub_app_config(),
946+
search=True,
947+
embeddings=False,
948+
full=False,
949+
project="new_default",
950+
)
951+
952+
project_index.assert_awaited_once()
953+
[indexed_call] = project_index.await_args_list
954+
assert indexed_call.args[0] is stored

‎tests/cli/test_project_add_indexing.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,9 @@ def test_project_add_indexes_files_already_on_disk(tmp_path):
109109

110110
add = _bm(["project", "add", "adopted", str(notes)], env)
111111
assert add.returncode == 0, add.stderr
112+
# The add runs the same visible pass as `bm project index` rather than one
113+
# silent API request (#1635).
114+
assert "Rebuilding full-text search index" in add.stdout
112115

113116
# No reindex is run here. That is the entire point of the test.
114117
search = _bm(["tool", "search-notes", "Alpha Note", "--project", "adopted", "--json"], env)

0 commit comments

Comments
 (0)