From aa84ced3ba0a364846a39e215d0f3bdc03ccac57 Mon Sep 17 00:00:00 2001 From: TPSJASPER <592827953@qq.com> Date: Sun, 25 Oct 2026 16:20:30 +0800 Subject: [PATCH 1/3] fix(mcp): address manual tool regressions Report only the accepted revision from bm_write, bm_edit and bm_move: the graph counts came from the pre-index response and always read zero. Make bm_search's line view honest. A one-note search says when no line matches, ranked candidates without a literal matching line are dropped, and filtered line totals are marked inexact while the next page stays reachable. bm_context pattern searches prefer notes; a pattern that names no note still selects relation or observation permalinks directly, and later note pages do not fall back to graph rows. Return the first nonblank line when bm_read's token budget is smaller than it, so a tiny budget cannot produce a whitespace-only page. Point bm_schema guidance at the bm_* tools and at bm reindex. Fixes #1843 Signed-off-by: TPSJASPER <592827953@qq.com> Signed-off-by: phernandez --- src/basic_memory/man/man3/bm-context(3).md | 4 + src/basic_memory/man/man3/bm-read(3).md | 3 + src/basic_memory/man/man3/bm-search(3).md | 7 +- src/basic_memory/man/man3/bm-write(3).md | 3 + src/basic_memory/markdown/sections.py | 19 ++- src/basic_memory/mcp/note_writes.py | 12 +- src/basic_memory/mcp/tools/ranked_search.py | 49 +++--- src/basic_memory/mcp/tools/schema.py | 63 ++++---- src/basic_memory/services/context_service.py | 50 +++++- tests/markdown/test_token_budget_nonempty.py | 34 ++++ tests/mcp/test_note_writes.py | 22 ++- tests/mcp/test_tool_bm_manual_regressions.py | 153 ++++++++++++++++++ tests/mcp/test_tool_bm_search.py | 100 ++++++++++++ tests/mcp/test_tool_bm_write.py | 4 +- tests/mcp/test_tool_schema.py | 10 +- .../test_context_glob_primary_notes.py | 88 ++++++++++ 16 files changed, 539 insertions(+), 82 deletions(-) create mode 100644 tests/markdown/test_token_budget_nonempty.py create mode 100644 tests/mcp/test_tool_bm_manual_regressions.py create mode 100644 tests/services/test_context_glob_primary_notes.py diff --git a/src/basic_memory/man/man3/bm-context(3).md b/src/basic_memory/man/man3/bm-context(3).md index 0f562cfc8..a70aef923 100644 --- a/src/basic_memory/man/man3/bm-context(3).md +++ b/src/basic_memory/man/man3/bm-context(3).md @@ -29,6 +29,10 @@ Returns a note, its observations, and the notes its relations reach, up to pattern such as `plans/*`. `since` limits related results to recent changes and `max_related` caps them. +A pattern selects notes as the primary results. Their observations and relations +remain grouped under those notes. A pattern that names no note but addresses +relation or observation permalinks still selects those rows directly. + `compact` returns the graph without note bodies or observations; read chosen notes with bm_read. Output defaults to JSON because the result is a graph; text output names each note's path and ends with the next-page call when there is diff --git a/src/basic_memory/man/man3/bm-read(3).md b/src/basic_memory/man/man3/bm-read(3).md index 7d8e71b79..2df769560 100644 --- a/src/basic_memory/man/man3/bm-read(3).md +++ b/src/basic_memory/man/man3/bm-read(3).md @@ -97,6 +97,9 @@ bm_read(identifier="plans/missing.md", project="demo") default project is set. Pass `project`, or qualify the path (`demo/plans/...`). - A section read returns numbered lines, like `start_line`/`end_line`; a whole note comes back without line numbers. +- The token budget is approximate. The first nonblank line is returned whole + even when it exceeds the budget, so a read does not return only leading blank + lines. Follow the returned line coordinates to continue. ## SEE ALSO diff --git a/src/basic_memory/man/man3/bm-search(3).md b/src/basic_memory/man/man3/bm-search(3).md index 425eaa4ee..11ac2c760 100644 --- a/src/basic_memory/man/man3/bm-search(3).md +++ b/src/basic_memory/man/man3/bm-search(3).md @@ -93,7 +93,12 @@ bm_search(query="release", mode="text", page_size=1, output_format="json", proje refused. - A search across projects reaches at most result 1000 (`page * page_size`). - `hybrid` mode also returns notes that are only near in meaning. With `lines`, - use `mode="text"` when you want only notes that contain the words. + candidates without a matching line are omitted in every mode. Searching one + note with `identifier` explicitly says when no lines match. +- Line matching filters one ranked page at a time. An empty page can still have + a next-page call; follow it to check the remaining candidates. +- Line results report `total_is_exact: false`; `total` counts ranked candidates, + so page with `has_more` instead of using it as a result count. ## SEE ALSO diff --git a/src/basic_memory/man/man3/bm-write(3).md b/src/basic_memory/man/man3/bm-write(3).md index cbc185dbc..d7b822fb0 100644 --- a/src/basic_memory/man/man3/bm-write(3).md +++ b/src/basic_memory/man/man3/bm-write(3).md @@ -40,6 +40,9 @@ relations (`- relation_type [[Other Note]]`). Observations and relations are indexed after the write, so they show up in bm_context and bm_search moments later. +The result reports the accepted note's path and checksum. Read its indexed +observations and relations with bm_context. + ## PARAMETERS - **content** (string, required) — The note's Markdown. Saved as given; it may start with frontmatter. diff --git a/src/basic_memory/markdown/sections.py b/src/basic_memory/markdown/sections.py index b209c7ba2..86b7cde57 100644 --- a/src/basic_memory/markdown/sections.py +++ b/src/basic_memory/markdown/sections.py @@ -483,14 +483,17 @@ def _token_budget_line_count(text: str, text_lines: list[str], budget_chars: int Cut preference: section boundary first, then paragraph (blank-line) boundary, then the last full line within budget. Never cuts mid-line, and always keeps - at least one line so a continue-loop makes forward progress. A single line - over budget is served whole — there is no boundary to cut at. + the first nonblank line so a small budget cannot return only whitespace. An + over-budget first content line is served whole — there is no boundary to cut at. """ if len(text) <= budget_chars: return None line_count = len(text_lines) if line_count <= 1: return None + minimum = next((index + 1 for index, line in enumerate(text_lines) if line.strip()), line_count) + if minimum == line_count: + return None # prefix_lengths[k] == len("\n".join(text_lines[:k])) prefix_lengths = [0] @@ -506,24 +509,24 @@ def fits(kept: int) -> bool: section_cuts = [ section.start_line - 1 for section in scan_sections(text) if section.start_line > 1 ] - fitting = [kept for kept in section_cuts if kept >= 1 and fits(kept)] + fitting = [kept for kept in section_cuts if kept >= minimum and fits(kept)] if fitting: return max(fitting) paragraph_cuts = [ index for index, line in enumerate(text_lines) if index >= 1 and not line.strip() ] - fitting = [kept for kept in paragraph_cuts if fits(kept)] + fitting = [kept for kept in paragraph_cuts if kept >= minimum and fits(kept)] if fitting: return max(fitting) # Line backstop: the largest whole-line prefix within budget; when even the - # first line exceeds the budget, keep it anyway — forward progress beats an - # empty slice. - for kept in range(line_count - 1, 0, -1): + # first content line exceeds the budget, keep it and its leading whitespace + # anyway, preserving coordinates and serving actual content. + for kept in range(line_count - 1, minimum - 1, -1): if fits(kept): return kept - return 1 + return minimum def slice_note_content( diff --git a/src/basic_memory/mcp/note_writes.py b/src/basic_memory/mcp/note_writes.py index b7597f58f..891c3cd98 100644 --- a/src/basic_memory/mcp/note_writes.py +++ b/src/basic_memory/mcp/note_writes.py @@ -28,9 +28,6 @@ class NoteChange: # The accepted revision's checksum, which bm_write and bm_edit take back as # expected_checksum. file_checksum lags behind materialization. checksum: str | None - observations: int - relations: int - unresolved_relations: int # The path the caller asked for, when the note landed in an existing folder's # spelling instead. requested_file_path: str | None = None @@ -56,9 +53,6 @@ def of( permalink=note.permalink, file_path=paths.path(note.file_path), checksum=note.db_checksum, - observations=len(note.observations), - relations=len(note.relations), - unresolved_relations=sum(1 for relation in note.relations if not relation.to_id), requested_file_path=None if requested is None else paths.path(requested), previous_file_path=( None if previous_file_path is None else paths.path(previous_file_path) @@ -75,7 +69,7 @@ def as_json(self) -> dict[str, Any]: } def text_lines(self, *, detail: str | None = None) -> list[str]: - """The text result: what changed and where, then identity and graph counts.""" + """Report the accepted revision, whose derived graph may not be indexed yet.""" match self.action: case "moved": headline = f"Moved {self.previous_file_path} to {self.file_path}" @@ -86,13 +80,9 @@ def text_lines(self, *, detail: str | None = None) -> list[str]: lines = [headline] if self.requested_file_path is not None: lines.append(f"requested: {self.requested_file_path}") - relations = f"relations: {self.relations}" - if self.unresolved_relations: - relations += f" ({self.unresolved_relations} not yet resolved)" lines += [ f"title: {self.title}", f"permalink: {self.permalink}", f"checksum: {self.checksum}", - f"observations: {self.observations}, {relations}", ] return lines diff --git a/src/basic_memory/mcp/tools/ranked_search.py b/src/basic_memory/mcp/tools/ranked_search.py index 5a3d5edc0..a912b1828 100644 --- a/src/basic_memory/mcp/tools/ranked_search.py +++ b/src/basic_memory/mcp/tools/ranked_search.py @@ -549,26 +549,31 @@ async def _with_matching_lines( """Replace each hit's excerpt with the matching lines of its accepted Markdown. Line numbers count the whole document, frontmatter included, as bm_read's do. - Hits with no accepted Markdown (files that are not notes) have no lines to show. + Only notes with accepted Markdown and matching lines are returned. """ note_hits = [hit for hit in response.results if hit.external_id is not None] contents = await KnowledgeClient( target.client, target.project.external_id ).accepted_note_contents([hit.external_id for hit in note_hits if hit.external_id]) payload = response.model_dump(mode="json", exclude_none=True) - payload["results"] = [ - _line_row( - hit, - scan_lines( - contents[hit.external_id], - terms, - context_lines=context_lines, - max_matches=max_matches, - ), + rows: list[dict[str, Any]] = [] + for hit in note_hits: + if hit.external_id not in contents: + continue + scan = scan_lines( + contents[hit.external_id], + terms, + context_lines=context_lines, + max_matches=max_matches, ) - for hit in note_hits - if hit.external_id in contents - ] + if scan.match_count > 0: + rows.append(_line_row(hit, scan)) + payload["results"] = rows + # Trigger: line matching can drop ranked rows from this page. + # Why: `total` counts ranked candidates, not rows with a matching line, so it + # cannot be used as an exact result count once rows are filtered. + # Outcome: JSON callers page with `has_more` instead of treating `total` as exact. + payload["total_is_exact"] = False return payload @@ -633,9 +638,17 @@ def _format_results( """ results = payload["results"] heading = f"Results for '{query}'" if query else "Results" + continuation = ( + f"\n\nMore results available. Call {next_call} to continue." + if next_call is not None + else "" + ) if not results: - return f"No results for '{query or ''}' in {scope}." + ( - f"\n\n{payload['query_hint']}" if payload.get("query_hint") else "" + page_hint = " on this page" if next_call is not None else "" + return ( + f"No results for '{query or ''}'{page_hint} in {scope}." + + (f"\n\n{payload['query_hint']}" if payload.get("query_hint") else "") + + continuation ) lines = [f"{heading} in {scope}:", ""] @@ -645,6 +658,8 @@ def _format_results( header += f" | score {row['score']:.3f}" lines.append(header) if "windows" in row: + if row["match_count"] == 0: + lines.append(" No matching lines.") for window in row["windows"]: for number, text in enumerate( window["content"].split("\n"), start=window["start_line"] @@ -654,6 +669,4 @@ def _format_results( lines.append(f" (more matches from line {row['next_match_line']})") elif row.get("matched_chunk"): lines.append(f" {row['matched_chunk'][:200]}") - if next_call is not None: - lines.extend(["", f"More results available. Call {next_call} to continue."]) - return "\n".join(lines) + return "\n".join(lines) + continuation diff --git a/src/basic_memory/mcp/tools/schema.py b/src/basic_memory/mcp/tools/schema.py index 2d71f6835..5a726978f 100644 --- a/src/basic_memory/mcp/tools/schema.py +++ b/src/basic_memory/mcp/tools/schema.py @@ -174,7 +174,7 @@ def _nothing_to_report(output_format: OutputFormat, message: str, guidance: str) return refusal("NOT_FOUND", text, output_format=output_format) -def _no_notes_guidance(note_type: str, tool_name: str) -> str: +def _no_notes_guidance(note_type: str, command: Literal["validate", "infer", "diff"]) -> str: """Build guidance string when no notes of a given type exist. Used by schema_validate when the project has zero notes of the @@ -182,38 +182,38 @@ def _no_notes_guidance(note_type: str, tool_name: str) -> str: """ return ( f"# No Notes Found of Type '{note_type}'\n\n" - f"`{tool_name}` found no notes with type '{note_type}' in the project.\n\n" + f"`bm_schema(command={command!r}, note_type={note_type!r})` found no notes " + f"with type '{note_type}' in the project.\n\n" f"## Next Steps\n\n" - f"1. **Create notes of this type** — use `write_note` with " + f"1. **Create notes of this type** — use `bm_write` with " f'`note_type="{note_type}"` to create notes\n' - f"2. **Check existing types** — use `search_notes` with `note_types` " + f"2. **Check existing types** — use `bm_search` with `note_types` " f"filter to see what types exist\n" - f"3. **Browse content** — use `list_directory` or `recent_activity` to " - f"see what's in the project\n" + f"3. **Browse content** — use `bm_list` to see what's in the project\n" ) -def _no_schemas_defined_guidance(tool_name: str) -> str: +def _no_schemas_defined_guidance() -> str: """Build guidance string when validating all types but no schemas exist. Used by schema_validate when called without arguments in a project that has no schema notes — there is nothing to validate yet. """ return ( - f"# No Schemas Defined\n\n" - f"`{tool_name}` was called without `note_type` or `identifier`, which " - f"validates every note type that has a schema — but this project has " - f"no schema notes yet, so there is nothing to validate.\n\n" - f"## Next Steps\n\n" - f'1. **Infer a schema** — run `schema_infer("")` to analyze ' - f"existing notes and get a suggested schema\n" - f"2. **Create a schema note** — write a note with `type: schema` and an " - f"`entity` field naming the note type it validates\n" - f"3. **Re-run** — call `{tool_name}()` again once a schema exists\n" + "# No Schemas Defined\n\n" + "`bm_schema(command='validate')` was called without `note_type` or `identifier`, which " + "validates every note type that has a schema — but this project has " + "no schema notes yet, so there is nothing to validate.\n\n" + "## Next Steps\n\n" + "1. **Infer a schema** — run `bm_schema(command='infer', note_type='')` to analyze " + "existing notes and get a suggested schema\n" + "2. **Create a schema note** — write a note with `type: schema` and an " + "`entity` field naming the note type it validates\n" + "3. **Re-run** — call `bm_schema(command='validate')` again once a schema exists\n" ) -def _no_schema_guidance(note_type: str, tool_name: str) -> str: +def _no_schema_guidance(note_type: str, command: Literal["validate", "diff"]) -> str: """Build guidance string when no schema exists for a note type. Used by schema_validate and schema_diff to explain what happened @@ -221,9 +221,10 @@ def _no_schema_guidance(note_type: str, tool_name: str) -> str: """ return ( f"# No Schema Found for '{note_type}'\n\n" - f"`{tool_name}` requires a schema note to exist for type '{note_type}'.\n\n" + f"`bm_schema(command={command!r}, note_type={note_type!r})` requires a schema " + f"note to exist for type '{note_type}'.\n\n" f"## How to Create a Schema\n\n" - f'1. **Infer from existing notes** — run `schema_infer("{note_type}")` to ' + f"1. **Infer from existing notes** — run `bm_schema(command='infer', note_type={note_type!r})` to " f"analyze your notes and get a suggested schema\n" f"2. **Create a schema note** — write a markdown file with this frontmatter:\n\n" f"```yaml\n" @@ -243,9 +244,9 @@ def _no_schema_guidance(note_type: str, tool_name: str) -> str: f"- `field_name: type, description` — required field\n" f"- `field_name?: type, description` — optional field\n" f"- Supported types: `string`, `number`, `boolean`, `string[]`\n\n" - f"3. **Index** — run `basic-memory db reindex --search` or wait for the " + f"3. **Index** — run `bm reindex --search` or wait for the " f"file watcher to pick up the new schema note\n" - f'4. **Re-run** — call `{tool_name}("{note_type}")` again\n' + f"4. **Re-run** — call `bm_schema(command={command!r}, note_type={note_type!r})` again\n" ) @@ -351,7 +352,7 @@ async def schema_validate( raise _nothing_to_report( output_format, "No schemas defined in this project", - _no_schemas_defined_guidance("schema_validate"), + _no_schemas_defined_guidance(), ) if output_format == "json": return result.model_dump(mode="json", exclude_none=True) @@ -366,7 +367,7 @@ async def schema_validate( raise _nothing_to_report( output_format, f"No notes found of type '{effective_type}'", - _no_notes_guidance(effective_type, "schema_validate"), + _no_notes_guidance(effective_type, "validate"), ) # --- No schema guard --- @@ -377,7 +378,7 @@ async def schema_validate( raise _nothing_to_report( output_format, f"No schema found for type '{effective_type}'", - _no_schema_guidance(effective_type, "schema_validate"), + _no_schema_guidance(effective_type, "validate"), ) if output_format == "json": @@ -476,7 +477,7 @@ async def schema_infer( raise _nothing_to_report( output_format, f"No notes found of type '{note_type}'", - _no_notes_guidance(note_type, "schema_infer"), + _no_notes_guidance(note_type, "infer"), ) # --- Empty schema guard --- @@ -494,12 +495,12 @@ async def schema_infer( f"This usually means '{note_type}' is too broad — the notes don't " f"share a consistent structure.\n\n" f"## Suggestions\n" - f"1. **Use a more specific type** — try `search_notes` with " + f"1. **Use a more specific type** — try `bm_search` with " f"`note_types` filter to see what types exist\n" f"2. **Lower the threshold** — " - f'`schema_infer("{note_type}", threshold=0.1)` to include ' + f"`bm_schema(command='infer', note_type={note_type!r}, threshold=0.1)` to include " f"rarer fields\n" - f"3. **Create typed notes** — use `write_note` with a specific " + f"3. **Create typed notes** — use `bm_write` with a specific " f'`note_type` (e.g., "person", "meeting") to build consistent ' f"structure\n", ) @@ -595,7 +596,7 @@ async def schema_diff( raise _nothing_to_report( output_format, f"No schema found for type '{note_type}'", - _no_schema_guidance(note_type, "schema_diff"), + _no_schema_guidance(note_type, "diff"), ) # --- No notes guard --- @@ -606,7 +607,7 @@ async def schema_diff( raise _nothing_to_report( output_format, f"No notes found of type '{note_type}'", - _no_notes_guidance(note_type, "schema_diff"), + _no_notes_guidance(note_type, "diff"), ) if output_format == "json": diff --git a/src/basic_memory/services/context_service.py b/src/basic_memory/services/context_service.py index df401ce09..61e9ed4b6 100644 --- a/src/basic_memory/services/context_service.py +++ b/src/basic_memory/services/context_service.py @@ -118,6 +118,46 @@ def _require_session_maker(self) -> async_sessionmaker[AsyncSession]: raise ValueError("session_maker is required for ContextService") return self.session_maker + async def _search_pattern_primary( + self, permalink_match: str, *, limit: int, offset: int + ) -> List[SearchIndexRow]: + """Match a pattern to its primary rows, preferring notes. + + A pattern such as ``plans/*`` selects notes, so observation and relation + rows it also matches are not primary results (#1843). Patterns also + address graph rows directly (``components/*/part_of/*workflow*``), so + fall back to the unfiltered match only when the pattern names no note. + """ + notes = await self.search_repository.search( + permalink_match=permalink_match, + search_item_types=[SearchItemType.ENTITY], + limit=limit, + offset=offset, + ) + if notes: + return notes + + # Trigger: a page after the first has no notes for this pattern. + # Why: falling back here would put relation rows on page two of a note + # pattern whose page one returned notes. + # Outcome: only a pattern that names no note at all reaches the graph-row + # fallback, and pagination stays within one row kind. + if offset: + first_page_notes = await self.search_repository.search( + permalink_match=permalink_match, + search_item_types=[SearchItemType.ENTITY], + limit=1, + offset=0, + ) + if first_page_notes: + return [] + + return await self.search_repository.search( + permalink_match=permalink_match, + limit=limit, + offset=offset, + ) + async def build_context( self, memory_url: Optional[MemoryUrl] = None, @@ -163,8 +203,8 @@ async def build_context( ] normalized_path = "*".join(normalized_parts) logger.debug(f"Pattern search for '{normalized_path}'") - primary = await self.search_repository.search( - permalink_match=normalized_path, limit=fetch_limit, offset=offset + primary = await self._search_pattern_primary( + normalized_path, limit=fetch_limit, offset=offset ) # Trigger: a workspace-qualified pattern matched nothing while a @@ -184,10 +224,8 @@ async def build_context( f"Pattern search fallback without workspace prefix: " f"'{fallback_path}'" ) - primary = await self.search_repository.search( - permalink_match=fallback_path, - limit=fetch_limit, - offset=offset, + primary = await self._search_pattern_primary( + fallback_path, limit=fetch_limit, offset=offset ) else: normalized_path = generate_permalink(path, split_extension=False) diff --git a/tests/markdown/test_token_budget_nonempty.py b/tests/markdown/test_token_budget_nonempty.py new file mode 100644 index 000000000..deb4b1b29 --- /dev/null +++ b/tests/markdown/test_token_budget_nonempty.py @@ -0,0 +1,34 @@ +"""A small read budget must not return only leading whitespace (#1843).""" + +import pytest + +from basic_memory.markdown.sections import LineRange, NoteSlice, slice_note_content + + +@pytest.mark.parametrize("newline", ["\n", "\r", "\r\n"]) +@pytest.mark.parametrize("explicit_lines", [False, True]) +def test_tiny_budget_keeps_the_first_nonblank_line(newline, explicit_lines): + first_line = "An opening paragraph too long for the budget." + document = newline.join(["---", "title: Plan", "---", "", " ", first_line, "", "Tail"]) + result = slice_note_content( + document, max_tokens=1, lines=LineRange(4, None) if explicit_lines else None + ) + + assert isinstance(result, NoteSlice) + assert first_line in result.content + assert result.start_line == 4 + assert result.end_line == 6 + assert result.continue_line == 7 + following = slice_note_content(document, max_tokens=1, lines=LineRange(7, None)) + assert isinstance(following, NoteSlice) + assert following.content.lstrip() == "Tail" + assert following.continue_line is None + + +def test_whitespace_only_body_does_not_create_empty_continuation_pages(): + result = slice_note_content("---\ntitle: Blank\n---\n\n \n \n", max_tokens=1) + + assert isinstance(result, NoteSlice) + assert result.content.strip() == "" + assert result.continue_line is None + assert result.truncated is False diff --git a/tests/mcp/test_note_writes.py b/tests/mcp/test_note_writes.py index 7d984556e..66781ac1b 100644 --- a/tests/mcp/test_note_writes.py +++ b/tests/mcp/test_note_writes.py @@ -2,6 +2,8 @@ from datetime import datetime, timezone +import pytest + from basic_memory.mcp.note_targets import PathQualifier from basic_memory.mcp.note_writes import NoteChange from basic_memory.schemas.v2.entity import EntityResponseV2 @@ -59,4 +61,22 @@ def test_text_headline_carries_the_detail(): change = NoteChange.of(_note(), action="edited", paths=PathQualifier(prefix=None)) assert change.text_lines(detail="append")[0] == "Edited Plans/q4.md (append)" - assert change.text_lines()[-1] == "observations: 0, relations: 0" + assert change.text_lines()[-1] == "checksum: sum" + + +@pytest.mark.parametrize("action", ["created", "updated", "edited", "moved"]) +def test_accepted_change_does_not_report_unavailable_graph_counts(action): + note = _note().model_copy( + update={ + "content": "- [goal] ship\n- relates_to [[Missing]]", + "observations": [], + "relations": [], + } + ) + change = NoteChange.of( + note, action=action, paths=PathQualifier(prefix=None), previous_file_path="notes/q4.md" + ) + + assert {"observations", "relations", "unresolved_relations"}.isdisjoint(change.as_json()) + assert "observations:" not in "\n".join(change.text_lines()) + assert "relations:" not in "\n".join(change.text_lines()) diff --git a/tests/mcp/test_tool_bm_manual_regressions.py b/tests/mcp/test_tool_bm_manual_regressions.py new file mode 100644 index 000000000..29d3ed25d --- /dev/null +++ b/tests/mcp/test_tool_bm_manual_regressions.py @@ -0,0 +1,153 @@ +"""Remaining bm_* manual regressions from #1843, through the local ASGI API.""" + +from typing import Any + +import pytest +from fastmcp.exceptions import ToolError + +from basic_memory.mcp.tools import bm_context, bm_edit, bm_move, bm_read, bm_schema, bm_write + + +@pytest.mark.asyncio +@pytest.mark.parametrize("output_format", ["text", "json"]) +async def test_note_changes_do_not_report_unindexed_graph_counts( + client, test_project, output_format +): + created = await bm_write( + identifier="plans/launch.md", + content="# Launch\n\n- [goal] ship the release\n\n- relates_to [[Other Plan]]", + output_format=output_format, + project=test_project.name, + ) + edited = await bm_edit( + identifier="plans/launch.md", + command="append", + content="\n- [decision] freeze the scope\n", + output_format=output_format, + project=test_project.name, + ) + moved = await bm_move( + identifier="plans/launch.md", + destination="archive/", + output_format=output_format, + project=test_project.name, + ) + + for result in (created, edited, moved): + if isinstance(result, dict): + assert {"observations", "relations", "unresolved_relations"}.isdisjoint(result) + assert result["checksum"] + else: + assert isinstance(result, str) + assert "observations:" not in result + assert "relations:" not in result + assert "checksum:" in result + note = await bm_read("archive/launch.md", project=test_project.name) + assert isinstance(note, str) + assert "[goal] ship the release" in note + assert "[decision] freeze the scope" in note + assert "relates_to [[Other Plan]]" in note + + +@pytest.mark.asyncio +@pytest.mark.parametrize("compact", [False, True]) +async def test_context_glob_pages_only_notes_and_keeps_their_graph(client, test_project, compact): + await bm_write( + identifier="plans/other.md", + content="# Other Plan\n\n- [goal] support the launch", + project=test_project.name, + ) + await bm_write( + identifier="plans/launch.md", + content="# Launch Plan\n\n- [goal] ship\n\n- relates_to [[other]]", + project=test_project.name, + ) + + pages: list[dict[str, Any]] = [] + for page_number in (1, 2): + result = await bm_context( + identifier="plans/*", + page=page_number, + page_size=1, + compact=compact, + project=test_project.name, + ) + assert isinstance(result, dict) + pages.append(result) + + for page in pages: + assert isinstance(page, dict) + assert len(page["results"]) == 1 + assert page["results"][0]["primary_result"]["type"] == "entity" + assert pages[0]["has_more"] is True + assert pages[1]["has_more"] is False + assert {page["results"][0]["primary_result"]["file_path"] for page in pages} == { + "plans/other.md", + "plans/launch.md", + } + if not compact: + assert all(page["results"][0]["observations"] for page in pages) + assert any( + row["type"] == "relation" + for page in pages + for row in page["results"][0]["related_results"] + ) + + +@pytest.mark.asyncio +@pytest.mark.parametrize("max_tokens", [1, 20]) +async def test_tiny_read_budget_returns_content_and_a_working_continuation( + client, test_project, max_tokens +): + first_line = "A long opening paragraph " * 30 + await bm_write( + identifier="plans/long.md", + content=f"\n\n{first_line}\n\nLast paragraph.", + project=test_project.name, + ) + + first = await bm_read( + "plans/long.md", max_tokens=max_tokens, output_format="json", project=test_project.name + ) + text = await bm_read("plans/long.md", max_tokens=max_tokens, project=test_project.name) + + assert isinstance(first, dict) + assert isinstance(text, str) + assert first_line in first["content"] + assert first_line in text + assert first["has_more"] is True + assert first["next_start_line"] > first["start_line"] + following = await bm_read( + "plans/long.md", + start_line=first["next_start_line"], + max_tokens=max_tokens, + output_format="json", + project=test_project.name, + ) + assert isinstance(following, dict) + assert "Last paragraph." in following["content"] + assert following["has_more"] is False + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + ("command", "note_type"), + [("validate", None), ("validate", "plan"), ("diff", "plan"), ("infer", "missing")], +) +async def test_schema_guidance_names_the_public_bm_tools(client, test_project, command, note_type): + await bm_write( + identifier="plans/launch.md", + content="---\ntype: plan\n---\n\n- [goal] ship", + project=test_project.name, + ) + + with pytest.raises(ToolError) as refused: + await bm_schema(command=command, note_type=note_type, project=test_project.name) + + guidance = str(refused.value) + assert "bm_schema(command=" in guidance + assert "schema_infer(" not in guidance + assert "schema_validate" not in guidance + assert "schema_diff" not in guidance + assert "`write_note`" not in guidance + assert "`search_notes`" not in guidance diff --git a/tests/mcp/test_tool_bm_search.py b/tests/mcp/test_tool_bm_search.py index 0e182430e..b24bc9f6d 100644 --- a/tests/mcp/test_tool_bm_search.py +++ b/tests/mcp/test_tool_bm_search.py @@ -14,6 +14,7 @@ from basic_memory.mcp.tools import ranked_search as ranked_search_module from basic_memory.mcp.tools.ranked_search import _query_terms from basic_memory.mcp.tools.search import MultiProjectFailed, MultiProjectNotIndexed +from basic_memory.schemas.search import SearchResponse async def search_text(**arguments: Any) -> str: @@ -163,6 +164,86 @@ async def test_lines_match_any_query_term(client, test_project): assert row["match_count"] == 2 +@pytest.mark.asyncio +@pytest.mark.parametrize("lines", [0, 1]) +@pytest.mark.parametrize( + ("query", "expected_titles"), [("rollback", ["Deploy Plan"]), ("zebra", [])] +) +async def test_lines_drop_ranked_hits_without_matching_lines( + client, test_project, mocker, lines, query, expected_titles +): + await _seed(test_project.name) + candidates = SearchResponse.model_validate( + await search_json(query="deploy", mode="text", items=["note"], project=test_project.name) + ) + # Pin semantic candidates, but read and scan the real accepted note content. + mocker.patch.object(ranked_search_module.SearchClient, "search", return_value=candidates) + + payload = await search_json(query=query, lines=lines, project=test_project.name) + text = await search_text(query=query, lines=lines, project=test_project.name) + ranked = await search_json(query=query, project=test_project.name) + + assert [row["title"] for row in payload["results"]] == expected_titles + assert all(row["match_count"] > 0 and row["windows"] for row in payload["results"]) + assert payload["total_is_exact"] is False + assert "Deploy History" not in text + if expected_titles: + assert ": Rollback is manual." in text + else: + assert text == f"No results for '{query}' in project '{test_project.name}'." + assert {row["title"] for row in ranked["results"]} == {"Deploy Plan", "Deploy History"} + + +@pytest.mark.asyncio +async def test_filtered_lines_page_keeps_the_next_page_call(client, test_project, mocker): + await _seed(test_project.name) + candidates = SearchResponse.model_validate( + await search_json(query="deploy", mode="text", items=["note"], project=test_project.name) + ) + by_title = {hit.title: hit for hit in candidates.results} + first_page = candidates.model_copy( + update={ + "results": [by_title["Deploy History"]], + "current_page": 1, + "page_size": 1, + "has_more": True, + } + ) + second_page = candidates.model_copy( + update={ + "results": [by_title["Deploy Plan"]], + "current_page": 2, + "page_size": 1, + "has_more": False, + } + ) + search = mocker.patch.object( + ranked_search_module.SearchClient, + "search", + side_effect=[first_page, first_page, second_page], + ) + + first = await search_json(query="rollback", lines=0, page_size=1, project=test_project.name) + text = await search_text(query="rollback", lines=0, page_size=1, project=test_project.name) + second = await search_json( + query="rollback", lines=0, page=2, page_size=1, project=test_project.name + ) + + assert first["results"] == [] + assert first["has_more"] is True + assert first["total_is_exact"] is False + assert first["current_page"] == 1 + assert first["page_size"] == 1 + assert "on this page" in text + assert ( + f"Call bm_search(query='rollback', lines=0, max_matches=10, project='{test_project.name}', " + "page=2, page_size=1) to continue." + ) in text + assert [row["title"] for row in second["results"]] == ["Deploy Plan"] + assert second["has_more"] is False + assert search.call_args.kwargs["page"] == 2 + + # --- One note --- @@ -179,6 +260,25 @@ async def test_identifier_searches_one_note(client, test_project): assert row["windows"] +@pytest.mark.asyncio +async def test_identifier_no_matches_is_explicit_and_keeps_json_identity(client, test_project): + await _seed(test_project.name) + + text = await search_text( + query="zebra", identifier="tasks/Deploy Plan.md", project=test_project.name + ) + row = await search_json( + query="zebra", identifier="tasks/Deploy Plan.md", project=test_project.name + ) + + assert "No matching lines" in text + assert "tasks/Deploy Plan.md" in text + assert "to continue" not in text + assert row["file_path"] == "tasks/Deploy Plan.md" + assert row["match_count"] == 0 + assert row["windows"] == [] + + @pytest.mark.asyncio async def test_identifier_that_names_a_non_note_file( client, test_project, test_files, project_repository, session_maker diff --git a/tests/mcp/test_tool_bm_write.py b/tests/mcp/test_tool_bm_write.py index dd23ac189..b528dc911 100644 --- a/tests/mcp/test_tool_bm_write.py +++ b/tests/mcp/test_tool_bm_write.py @@ -229,7 +229,9 @@ async def test_frontmatter_without_title_and_unresolved_relations(client, test_p ) assert "title: linked" in text - assert "relations: 1 (1 not yet resolved)" in text + assert "relations:" not in text + note = await bm_read("notes/linked.md", project=test_project.name) + assert isinstance(note, str) and "relates_to [[No Such Note]]" in note @pytest.mark.asyncio diff --git a/tests/mcp/test_tool_schema.py b/tests/mcp/test_tool_schema.py index d8952b2f6..6d52430ad 100644 --- a/tests/mcp/test_tool_schema.py +++ b/tests/mcp/test_tool_schema.py @@ -465,7 +465,7 @@ async def test_schema_validate_all_types_no_schemas_refuses_with_guidance( assert isinstance(result, str) assert "No Schemas Defined" in result assert "unknown" not in result - assert "schema_infer" in result + assert "bm_schema(command='infer'" in result @pytest.mark.asyncio @@ -668,8 +668,8 @@ async def test_schema_validate_no_notes_refuses_with_guidance(app, test_project, assert isinstance(result, str) assert "No Notes Found" in result assert "employee" in result - assert "write_note" in result - assert "search_notes" in result + assert "bm_write" in result + assert "bm_search" in result @pytest.mark.asyncio @@ -699,7 +699,7 @@ async def test_schema_validate_no_schema_refuses_with_guidance(app, test_project assert isinstance(result, str) assert "No Schema Found" in result assert "person" in result - assert "schema_infer" in result + assert "bm_schema(command='infer'" in result assert "How to Create a Schema" in result @@ -729,7 +729,7 @@ async def test_schema_diff_no_schema_refuses_with_guidance(app, test_project, in assert isinstance(result, str) assert "No Schema Found" in result assert "person" in result - assert "schema_infer" in result + assert "bm_schema(command='infer'" in result assert "How to Create a Schema" in result diff --git a/tests/services/test_context_glob_primary_notes.py b/tests/services/test_context_glob_primary_notes.py new file mode 100644 index 000000000..4c8c88fab --- /dev/null +++ b/tests/services/test_context_glob_primary_notes.py @@ -0,0 +1,88 @@ +"""Glob context selects notes before paging, including legacy workspace fallback.""" + +import pytest + +from basic_memory.services.context_service import ContextService +from basic_memory.workspace_context import workspace_permalink_context + +GRAPH_FILE_PATHS = { + "test/Connected Entity 1.md", + "test/Connected Entity 2.md", + "test/Deep Entity.md", + "test/Deeper Entity.md", + "test/Root.md", +} + + +@pytest.mark.asyncio +@pytest.mark.parametrize("workspace", [None, "team-paul"]) +async def test_glob_primary_rows_are_notes_in_both_permalink_forms( + search_repository, + entity_repository, + observation_repository, + session_maker, + test_graph, + workspace, +): + service = ContextService( + search_repository, entity_repository, observation_repository, session_maker=session_maker + ) + prefix = f"{workspace}/" if workspace else "" + + with workspace_permalink_context( + workspace_slug=workspace, workspace_type="organization" if workspace else None + ): + result = await service.build_context(f"memory://{prefix}test-project/test/*", limit=100) + + assert all(item.primary_result.type == "entity" for item in result.results) + assert {item.primary_result.file_path for item in result.results} == GRAPH_FILE_PATHS + assert result.metadata.primary_count == len(result.results) + assert result.metadata.has_more is False + + +@pytest.mark.asyncio +async def test_note_pattern_later_page_does_not_fall_back_to_graph_rows( + search_repository, + entity_repository, + observation_repository, + session_maker, + test_graph, +): + service = ContextService( + search_repository, entity_repository, observation_repository, session_maker=session_maker + ) + + with workspace_permalink_context(workspace_slug=None, workspace_type=None): + result = await service.build_context("memory://test-project/test/*", limit=2, offset=6) + + assert result.results == [] + assert result.metadata.primary_count == 0 + + +@pytest.mark.asyncio +async def test_graph_row_pattern_still_reaches_observation_and_relation_rows( + search_repository, + entity_repository, + observation_repository, + session_maker, + test_graph, +): + service = ContextService( + search_repository, entity_repository, observation_repository, session_maker=session_maker + ) + + with workspace_permalink_context(workspace_slug=None, workspace_type=None): + relation_result = await service.build_context( + "memory://test-project/test/*/connects-to/*", limit=100 + ) + observation_result = await service.build_context( + "memory://test-project/test/*/observations/*", limit=100 + ) + + assert relation_result.results + assert all(item.primary_result.type == "relation" for item in relation_result.results) + assert any( + "connects-to" in (item.primary_result.permalink or "") for item in relation_result.results + ) + assert observation_result.results + assert all(item.primary_result.type == "observation" for item in observation_result.results) From 200614daf5b2acb6ee0ac795478a52db5d48fbe6 Mon Sep 17 00:00:00 2001 From: phernandez Date: Sun, 25 Oct 2026 11:04:49 -0500 Subject: [PATCH 2/3] test(mcp): exercise deferred manual contracts without mocks Signed-off-by: phernandez --- .../mcp/test_deferred_manual_contracts.py | 346 ++++++++++++++++++ tests/mcp/test_tool_bm_search.py | 81 ---- 2 files changed, 346 insertions(+), 81 deletions(-) create mode 100644 test-int/mcp/test_deferred_manual_contracts.py diff --git a/test-int/mcp/test_deferred_manual_contracts.py b/test-int/mcp/test_deferred_manual_contracts.py new file mode 100644 index 000000000..c3d7b3570 --- /dev/null +++ b/test-int/mcp/test_deferred_manual_contracts.py @@ -0,0 +1,346 @@ +"""Public MCP contracts with real deferred writes, files, indexes and embeddings. + +The ordinary suite indexes inline for speed and simple assertions. These tests +use production scheduling and a real queue barrier to distinguish acceptance +from materialization without replacing application code or guessing sleep times. +""" + +import asyncio +from collections.abc import AsyncIterator +from contextlib import AbstractAsyncContextManager, asynccontextmanager +from pathlib import Path + +import pytest +from fastapi import FastAPI +from fastmcp import Client, FastMCP +from fastmcp.exceptions import ToolError +from fastmcp.client.transports import FastMCPTransport + +from basic_memory.config import BasicMemoryConfig, ConfigManager +from basic_memory.index import note_content_materialization +from basic_memory.index.local_schedulers import drain_background_tasks +from basic_memory.index.note_content_materialization import drain_pending_materializations +from basic_memory.models import Project + +pytestmark = [pytest.mark.asyncio, pytest.mark.real_embedder] + +type McpSession = AbstractAsyncContextManager[Client[FastMCPTransport]] + + +async def settle_background_work() -> None: + """Wait for actual queue completion, including relation-resolution follow-ups.""" + async with asyncio.timeout(90): + await drain_pending_materializations() + await drain_background_tasks() + + +@asynccontextmanager +async def held_materialization() -> AsyncIterator[None]: + """Occupy the single real worker before a request enqueues its file write.""" + release = asyncio.Event() + # One worker makes FIFO queue ordering a deterministic acceptance boundary. + # This is ordinary queued work; no writer, indexer or scheduler is replaced. + note_content_materialization._materialization_pool.submit(release.wait(), workers=1, key=(0, 0)) + try: + yield + finally: + release.set() + await settle_background_work() + + +@asynccontextmanager +async def production_client( + server: FastMCP, config: BasicMemoryConfig, config_manager: ConfigManager +) -> AsyncIterator[Client[FastMCPTransport]]: + config.env = "dev" + config.materialization_workers = 1 + config_manager.save_config(config) + # is_test_env also consults pytest's per-phase environment variable. Enter + # this context inside the test call so pytest cannot restore it before use. + with pytest.MonkeyPatch.context() as environment: + environment.setenv("BASIC_MEMORY_ENV", "dev") + environment.delenv("PYTEST_CURRENT_TEST", raising=False) + assert not config.is_test_env + try: + async with Client(server) as client: + yield client + finally: + await settle_background_work() + await note_content_materialization._materialization_pool.aclose() + await note_content_materialization._vector_sync_pool.aclose() + + +@pytest.fixture +def deferred_mcp( + mcp_server: FastMCP, + app: FastAPI, + app_config: BasicMemoryConfig, + config_manager: ConfigManager, +) -> McpSession: + """Keep real API dependencies while selecting production's deferred lifecycle.""" + return production_client(mcp_server, app_config, config_manager) + + +@pytest.mark.parametrize("output_format", ["text", "json"]) +async def test_write_edit_move_report_acceptance_before_materialization( + deferred_mcp: McpSession, test_project: Project, output_format: str +) -> None: + root = Path(test_project.path) + path = "plans/launch.md" + content = "# Launch\n\n- [goal] ship\n- relates_to [[Other Plan]]\n" + async with deferred_mcp as client: + await client.call_tool( + "bm_write", + { + "identifier": "plans/other.md", + "content": "# Other Plan", + "project": test_project.name, + }, + ) + await settle_background_work() + + # --- Acceptance is readable while the portable file is still queued --- + async with held_materialization(): + created = await client.call_tool( + "bm_write", + { + "identifier": path, + "content": content, + "project": test_project.name, + "output_format": output_format, + }, + ) + assert not (root / path).exists() + immediate = await client.call_tool( + "bm_read", + {"identifier": path, "project": test_project.name, "output_format": "json"}, + ) + assert isinstance(immediate.data, dict) + assert "[goal] ship" in immediate.data["content"] + materialized = (root / path).read_bytes() + + async with held_materialization(): + edited = await client.call_tool( + "bm_edit", + { + "identifier": path, + "command": "append", + "content": "\n- [decision] freeze\n", + "project": test_project.name, + "output_format": output_format, + }, + ) + assert (root / path).read_bytes() == materialized + immediate = await client.call_tool( + "bm_read", + {"identifier": path, "project": test_project.name, "output_format": "json"}, + ) + assert isinstance(immediate.data, dict) + assert "[decision] freeze" in immediate.data["content"] + + async with held_materialization(): + moved = await client.call_tool( + "bm_move", + { + "identifier": path, + "destination": "archive/", + "project": test_project.name, + "output_format": output_format, + }, + ) + assert (root / path).is_file() + assert not (root / "archive/launch.md").exists() + immediate = await client.call_tool( + "bm_read", + { + "identifier": "archive/launch.md", + "project": test_project.name, + "output_format": "json", + }, + ) + assert isinstance(immediate.data, dict) + assert "[decision] freeze" in immediate.data["content"] + + # --- Accepted results never pretend the derived graph was counted --- + for result in (created, edited, moved): + if output_format == "json": + assert isinstance(result.data, dict) + assert {"observations", "relations", "unresolved_relations"}.isdisjoint(result.data) + assert result.data["checksum"] + else: + assert isinstance(result.data, str) + assert "observations:" not in result.data + assert "relations:" not in result.data + assert "checksum:" in result.data + + assert not (root / path).exists() + assert "[decision] freeze" in (root / "archive/launch.md").read_text(encoding="utf-8") + context = await client.call_tool( + "bm_context", {"identifier": "archive/launch.md", "project": test_project.name} + ) + assert isinstance(context.data, dict) + (note,) = context.data["results"] + assert len(note["observations"]) == 2 + assert any(row["type"] == "relation" for row in note["related_results"]) + + +@pytest.mark.parametrize("mode", ["vector", "hybrid"]) +@pytest.mark.parametrize("lines", [0, 1]) +async def test_real_semantic_candidates_are_filtered_without_losing_pagination( + deferred_mcp: McpSession, test_project: Project, mode: str, lines: int +) -> None: + async with deferred_mcp as client: + for path, content in ( + ("plans/launch.md", "# Launch\n\nRollback is manual."), + ("plans/history.md", "# History\n\nThe previous deployment was quiet."), + ): + await client.call_tool( + "bm_write", {"identifier": path, "content": content, "project": test_project.name} + ) + await settle_background_work() + arguments = { + "query": "zebra", + "mode": mode, + "items": ["note"], + "min_similarity": 0.0, + "page_size": 1, + "project": test_project.name, + } + ranked = await client.call_tool("bm_search", {**arguments, "output_format": "json"}) + assert isinstance(ranked.data, dict) + assert len(ranked.data["results"]) == 1 + assert ranked.data["has_more"] is True + + first = await client.call_tool( + "bm_search", {**arguments, "lines": lines, "output_format": "json"} + ) + assert isinstance(first.data, dict) + assert first.data["results"] == [] + assert first.data["has_more"] is True + assert first.data["total_is_exact"] is False + text = await client.call_tool("bm_search", {**arguments, "lines": lines}) + assert isinstance(text.data, str) + assert "on this page" in text.data + assert "Call bm_search(" in text.data and "page=2" in text.data + second = await client.call_tool( + "bm_search", {**arguments, "lines": lines, "page": 2, "output_format": "json"} + ) + assert isinstance(second.data, dict) + assert second.data["results"] == [] + assert second.data["has_more"] is False + + matched = await client.call_tool( + "bm_search", + { + **arguments, + "query": "rollback", + "lines": lines, + "page_size": 10, + "output_format": "json", + }, + ) + assert isinstance(matched.data, dict) + assert [row["file_path"] for row in matched.data["results"]] == ["plans/launch.md"] + assert all(row["match_count"] > 0 and row["windows"] for row in matched.data["results"]) + + no_match = await client.call_tool( + "bm_search", + { + "query": "zebra", + "identifier": "plans/launch.md", + "project": test_project.name, + "output_format": "json", + }, + ) + assert isinstance(no_match.data, dict) + assert no_match.data["match_count"] == 0 and no_match.data["windows"] == [] + text = await client.call_tool( + "bm_search", + {"query": "zebra", "identifier": "plans/launch.md", "project": test_project.name}, + ) + assert isinstance(text.data, str) and "No matching lines." in text.data + + +@pytest.mark.parametrize(("compact", "max_tokens"), [(False, 1), (True, 20)]) +async def test_deferred_notes_have_honest_graph_pages_read_budgets_and_schema_guidance( + deferred_mcp: McpSession, + test_project: Project, + compact: bool, + max_tokens: int, +) -> None: + first_line = "A long opening paragraph " * 30 + async with deferred_mcp as client: + await client.call_tool( + "bm_write", + { + "identifier": "plans/other.md", + "content": "# Other Plan\n- [goal] support", + "note_type": "plan", + "project": test_project.name, + }, + ) + await client.call_tool( + "bm_write", + { + "identifier": "plans/launch.md", + "note_type": "plan", + "content": f"\n\n{first_line}\n\nLast paragraph.\n\n- [goal] ship\n" + "- relates_to [[Other Plan]]", + "project": test_project.name, + }, + ) + await settle_background_work() + + paths: set[str] = set() + for page in (1, 2): + graph = await client.call_tool( + "bm_context", + { + "identifier": "plans/*", + "page": page, + "page_size": 1, + "compact": compact, + "project": test_project.name, + }, + ) + assert isinstance(graph.data, dict) + (note,) = graph.data["results"] + assert note["primary_result"]["type"] == "entity" + paths.add(note["primary_result"]["file_path"]) + assert graph.data["has_more"] is (page == 1) + if not compact: + assert note["observations"] + assert paths == {"plans/launch.md", "plans/other.md"} + + start_line = 1 + contents: list[str] = [] + for page in range(10): + arguments = { + "identifier": "plans/launch.md", + "max_tokens": max_tokens, + "project": test_project.name, + "output_format": "json", + } + # The first call exercises the default body slice after frontmatter. + if page: + arguments["start_line"] = start_line + read = await client.call_tool("bm_read", arguments) + assert isinstance(read.data, dict) + assert read.data["content"].strip() + contents.append(read.data["content"]) + if not read.data["has_more"]: + break + assert read.data["next_start_line"] > read.data["start_line"] + start_line = read.data["next_start_line"] + else: + pytest.fail("The read continuation did not reach the end of the note") + assert first_line in contents[0] + assert "Last paragraph." in "\n".join(contents) + + with pytest.raises(ToolError) as refused: + await client.call_tool( + "bm_schema", {"command": "validate", "project": test_project.name} + ) + guidance = str(refused.value) + assert "bm_schema(command='infer'" in guidance + assert "schema_infer(" not in guidance and "schema_validate" not in guidance diff --git a/tests/mcp/test_tool_bm_search.py b/tests/mcp/test_tool_bm_search.py index b24bc9f6d..677b60a7f 100644 --- a/tests/mcp/test_tool_bm_search.py +++ b/tests/mcp/test_tool_bm_search.py @@ -14,7 +14,6 @@ from basic_memory.mcp.tools import ranked_search as ranked_search_module from basic_memory.mcp.tools.ranked_search import _query_terms from basic_memory.mcp.tools.search import MultiProjectFailed, MultiProjectNotIndexed -from basic_memory.schemas.search import SearchResponse async def search_text(**arguments: Any) -> str: @@ -164,86 +163,6 @@ async def test_lines_match_any_query_term(client, test_project): assert row["match_count"] == 2 -@pytest.mark.asyncio -@pytest.mark.parametrize("lines", [0, 1]) -@pytest.mark.parametrize( - ("query", "expected_titles"), [("rollback", ["Deploy Plan"]), ("zebra", [])] -) -async def test_lines_drop_ranked_hits_without_matching_lines( - client, test_project, mocker, lines, query, expected_titles -): - await _seed(test_project.name) - candidates = SearchResponse.model_validate( - await search_json(query="deploy", mode="text", items=["note"], project=test_project.name) - ) - # Pin semantic candidates, but read and scan the real accepted note content. - mocker.patch.object(ranked_search_module.SearchClient, "search", return_value=candidates) - - payload = await search_json(query=query, lines=lines, project=test_project.name) - text = await search_text(query=query, lines=lines, project=test_project.name) - ranked = await search_json(query=query, project=test_project.name) - - assert [row["title"] for row in payload["results"]] == expected_titles - assert all(row["match_count"] > 0 and row["windows"] for row in payload["results"]) - assert payload["total_is_exact"] is False - assert "Deploy History" not in text - if expected_titles: - assert ": Rollback is manual." in text - else: - assert text == f"No results for '{query}' in project '{test_project.name}'." - assert {row["title"] for row in ranked["results"]} == {"Deploy Plan", "Deploy History"} - - -@pytest.mark.asyncio -async def test_filtered_lines_page_keeps_the_next_page_call(client, test_project, mocker): - await _seed(test_project.name) - candidates = SearchResponse.model_validate( - await search_json(query="deploy", mode="text", items=["note"], project=test_project.name) - ) - by_title = {hit.title: hit for hit in candidates.results} - first_page = candidates.model_copy( - update={ - "results": [by_title["Deploy History"]], - "current_page": 1, - "page_size": 1, - "has_more": True, - } - ) - second_page = candidates.model_copy( - update={ - "results": [by_title["Deploy Plan"]], - "current_page": 2, - "page_size": 1, - "has_more": False, - } - ) - search = mocker.patch.object( - ranked_search_module.SearchClient, - "search", - side_effect=[first_page, first_page, second_page], - ) - - first = await search_json(query="rollback", lines=0, page_size=1, project=test_project.name) - text = await search_text(query="rollback", lines=0, page_size=1, project=test_project.name) - second = await search_json( - query="rollback", lines=0, page=2, page_size=1, project=test_project.name - ) - - assert first["results"] == [] - assert first["has_more"] is True - assert first["total_is_exact"] is False - assert first["current_page"] == 1 - assert first["page_size"] == 1 - assert "on this page" in text - assert ( - f"Call bm_search(query='rollback', lines=0, max_matches=10, project='{test_project.name}', " - "page=2, page_size=1) to continue." - ) in text - assert [row["title"] for row in second["results"]] == ["Deploy Plan"] - assert second["has_more"] is False - assert search.call_args.kwargs["page"] == 2 - - # --- One note --- From b3f4a6725b7f35ec86aab60d009b5884e6c7e1c0 Mon Sep 17 00:00:00 2001 From: phernandez Date: Sun, 25 Oct 2026 11:15:38 -0500 Subject: [PATCH 3/3] test(mcp): check indexed graph through context after note changes Signed-off-by: phernandez --- test-int/mcp/test_edit_note_integration.py | 35 ++++++++++++++++++--- test-int/mcp/test_write_note_integration.py | 20 ++++++++---- 2 files changed, 44 insertions(+), 11 deletions(-) diff --git a/test-int/mcp/test_edit_note_integration.py b/test-int/mcp/test_edit_note_integration.py index 81326194a..862be363c 100644 --- a/test-int/mcp/test_edit_note_integration.py +++ b/test-int/mcp/test_edit_note_integration.py @@ -12,6 +12,7 @@ from basic_memory import file_utils from basic_memory.file_utils import FileWriteError, parse_frontmatter +from basic_memory.index.note_content_materialization import drain_pending_materializations from basic_memory.mcp.clients import KnowledgeClient from basic_memory.repository.note_content_repository import ( NoteContentRepository, @@ -345,13 +346,12 @@ async def test_edit_note_with_observations_and_relations(mcp_server, app, test_p }, ) - # Should return edit summary with observation and relation counts assert len(edit_result.content) == 1 edit_text = edit_result.content[0].text assert "(append)" in edit_text - assert "observations:" in edit_text - assert "relations: 5" in edit_text - # Should have feature, tech, status, security categories + assert "checksum:" in edit_text + assert "observations:" not in edit_text + assert "relations:" not in edit_text # Verify the content was added and processed read_result = await client.call_tool( @@ -367,6 +367,21 @@ async def test_edit_note_with_observations_and_relations(mcp_server, app, test_p assert "Added payment processing endpoints" in content assert "integrates_with [[Payment Gateway]]" in content + await drain_pending_materializations() + graph = await client.call_tool( + "bm_context", {"identifier": "API Documentation", "project": test_project.name} + ) + assert isinstance(graph.data, dict) + (note,) = graph.data["results"] + assert len(note["observations"]) == 6 + assert {row["category"] for row in note["observations"]} == { + "feature", + "tech", + "status", + "security", + } + assert sum(row["type"] == "relation" for row in note["related_results"]) == 5 + @pytest.mark.asyncio async def test_edit_note_error_handling_note_not_found(mcp_server, app, test_project): @@ -758,7 +773,8 @@ def test_function(): assert len(edit_result.content) == 1 edit_text = edit_result.content[0].text assert "(append)" in edit_text - assert "observations:" in edit_text + assert "checksum:" in edit_text + assert "observations:" not in edit_text # Verify the special content was added correctly read_result = await client.call_tool( @@ -777,6 +793,15 @@ def test_function(): assert "def test_function():" in content assert "[[Unicode Standards]]" in content + await drain_pending_materializations() + graph = await client.call_tool( + "bm_context", {"identifier": "Special Chars Test", "project": test_project.name} + ) + assert isinstance(graph.data, dict) + (note,) = graph.data["results"] + assert {row["category"] for row in note["observations"]} == {"unicode", "markdown"} + assert sum(row["type"] == "relation" for row in note["related_results"]) == 1 + @pytest.mark.asyncio async def test_edit_note_using_different_identifiers(mcp_server, app, test_project): diff --git a/test-int/mcp/test_write_note_integration.py b/test-int/mcp/test_write_note_integration.py index b9af00b24..f52c289e1 100644 --- a/test-int/mcp/test_write_note_integration.py +++ b/test-int/mcp/test_write_note_integration.py @@ -15,6 +15,7 @@ from fastmcp.exceptions import ToolError from basic_memory.config import ConfigManager +from basic_memory.index.note_content_materialization import drain_pending_materializations from basic_memory.schemas.project_info import ProjectItem @@ -349,12 +350,8 @@ async def test_write_note_complex_content_with_observations_relations( assert "knowledge/Complex Knowledge Note.md" in response_text assert f"permalink: {test_project.name}/knowledge/complex-knowledge-note" in response_text - # Should show observation and relation counts - assert "observations: 3" in response_text - - assert "relations: 3" in response_text - # Should show outgoing relations - + assert "observations:" not in response_text + assert "relations:" not in response_text assert "checksum:" in response_text read = await client.call_tool( "bm_read", @@ -366,6 +363,17 @@ async def test_write_note_complex_content_with_observations_relations( ) assert _json_content(read)["frontmatter"]["tags"] == ["complex", "knowledge", "relations"] + # Graph counts belong to the indexed projection, after materialization. + await drain_pending_materializations() + graph = _json_content( + await client.call_tool( + "bm_context", {"identifier": "Complex Knowledge Note", "project": test_project.name} + ) + ) + (note,) = graph["results"] + assert len(note["observations"]) == 3 + assert sum(row["type"] == "relation" for row in note["related_results"]) == 3 + @pytest.mark.asyncio async def test_write_note_preserve_frontmatter(mcp_server, app, test_project):