diff --git a/CHANGELOG.md b/CHANGELOG.md index d30edb7b4..f2a6ce3a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -100,7 +100,9 @@ (the full `checksum` from a JSON `read_note`, `write_note` or `edit_note` result) and the edit applies only while the note is still that revision. Otherwise nothing is written and the tool error is a revision conflict carrying the current checksum (JSON - `error: "NOTE_REVISION_CONFLICT"`, `currentChecksum`). A guarded `append` or `prepend` + `error: "NOTE_REVISION_CONFLICT"`, `currentChecksum`, with the note's title, permalink + and file path while it still exists, the same shape as `write_note`'s, #1719). A guarded + `append` or `prepend` on a missing note is a conflict, not an auto-create. The v2 `PATCH` entity route accepts the same `x-bm-cloud-note-base-checksum` header the `PUT` route does. @@ -112,7 +114,9 @@ `expected_checksum` with `overwrite=True` (CLI: `--overwrite --expected-checksum`) and the note is replaced only while it is still that revision. Otherwise nothing changes and the result is a revision conflict carrying the current checksum (JSON - `error: "NOTE_REVISION_CONFLICT"`). A checksum for a path no note owns is also a + `error: "NOTE_REVISION_CONFLICT"` and `currentChecksum`, with `checksum` null because + nothing was written, and the note at the path named by its stored title and permalink, + #1719). A checksum for a path no note owns is also a conflict, so a note deleted since you read it is not recreated. Without `expected_checksum`, `overwrite=True` still replaces unconditionally. @@ -361,6 +365,22 @@ `bisync-reset` now print only their own deprecation notice, without Click's extra "DeprecationWarning: The command ... is deprecated." line. +- **#1716**: A note's `created_at` no longer resets on every edit, move or reindex. + Without a frontmatter `created`, the parser falls back to the file's ctime, which each + atomic rewrite or rename moves, and reindexing an existing note copied that time into + the row. Now an existing row keeps its creation time, and only a declared frontmatter + `created` changes it. A brand-new file still takes its file time. + +- **#1717**: `build_context` text output names the source of relations the primary note + does not own. An incoming `relates_to` used to be listed bare under its target, so it + read as a self-link; it now reads `- [[Brewing Notes]] relates_to [[Bean Origins]]`. + Relations the note owns keep the bare form, and JSON output is unchanged. + +- **#1720**: `edit_note` `insert_after_section` no longer puts a blank line between an + inserted list item and the section's list, which split one list in two. The item joins + the list directly, below any blank line under the heading. A paragraph inserted above + a paragraph still gets the blank line that keeps them apart. + - **#1595**: `basic_memory_diagnostics` lists the `BASIC_MEMORY_*` environment variables that override `config.json`, redacted the same way as the file dump. It used to print only the file, which can disagree with what the server is using. Other diff --git a/src/basic_memory/api/v2/routers/note_write_router.py b/src/basic_memory/api/v2/routers/note_write_router.py index c0812812f..c93175c5c 100644 --- a/src/basic_memory/api/v2/routers/note_write_router.py +++ b/src/basic_memory/api/v2/routers/note_write_router.py @@ -129,8 +129,13 @@ async def write_note( return NoteTargetMoved(**asdict(note)) case Locked(message=message): return NoteLocked(message=message) - case RevisionConflict(file_path=file_path, current_db_checksum=current): - return NoteRevisionConflict(file_path=file_path, db_checksum=current) + case RevisionConflict(file_path=file_path, current_db_checksum=current, note=note): + return NoteRevisionConflict( + file_path=file_path, + db_checksum=current, + title=note.title if note else None, + permalink=note.permalink if note else None, + ) case Rejected(rejection=rejection): error = note_content_mutation_error_from_rejection(rejection) raise HTTPException(status_code=error.status_code, detail=error.detail) diff --git a/src/basic_memory/man/man3/write-note(3).md b/src/basic_memory/man/man3/write-note(3).md index 9e3eb64ff..4720ba881 100644 --- a/src/basic_memory/man/man3/write-note(3).md +++ b/src/basic_memory/man/man3/write-note(3).md @@ -55,7 +55,8 @@ that replacement. Locked notes refuse replacement without changing their content Pass `expected_checksum` with `overwrite=True` (CLI: `--expected-checksum`) to make the replacement conditional on the revision you read. If the note changed since, or no note owns the path any more, nothing is written and the tool -returns `NOTE_REVISION_CONFLICT` with the current checksum. Without it, +returns `NOTE_REVISION_CONFLICT` with the current checksum in `currentChecksum`, +the same shape as edit_note's conflict. Without it, `overwrite=True` replaces the note unconditionally. ## PARAMETERS diff --git a/src/basic_memory/mcp/tools/build_context.py b/src/basic_memory/mcp/tools/build_context.py index d5ab2faf4..61b099ada 100644 --- a/src/basic_memory/mcp/tools/build_context.py +++ b/src/basic_memory/mcp/tools/build_context.py @@ -91,11 +91,23 @@ def _format_entity_block(result: ContextResult, *, compact: bool = False) -> str if relation_items: lines.append("") lines.append("### Relations") + primary_external_id = primary.external_id if isinstance(primary, EntitySummary) else None for rel in relation_items: # Unresolved forward references have no resolved entity yet; fall back # to the literal target text instead of rendering [[None]] (#955) target = rel.to_entity or rel.to_name - lines.append(f"- {rel.relation_type} [[{target}]]") + # Trigger: the relation is incoming, or belongs to a neighbor at depth > 1. + # Why: listed bare under this note, `relates_to [[This Note]]` reads as a + # self-link (#1717). + # Outcome: relations this note owns stay bare; others name their source. + is_outgoing = ( + primary_external_id is not None + and rel.from_entity_external_id == primary_external_id + ) + if is_outgoing or rel.from_entity is None: + lines.append(f"- {rel.relation_type} [[{target}]]") + else: + lines.append(f"- [[{rel.from_entity}]] {rel.relation_type} [[{target}]]") # --- Related entities (non-relation related results) --- related_entities: list[EntitySummary | ObservationSummary] = [ diff --git a/src/basic_memory/mcp/tools/edit_note.py b/src/basic_memory/mcp/tools/edit_note.py index 977cf62f0..cbacec208 100644 --- a/src/basic_memory/mcp/tools/edit_note.py +++ b/src/basic_memory/mcp/tools/edit_note.py @@ -33,7 +33,7 @@ from basic_memory.mcp.server import mcp from basic_memory.mcp.tools.utils import _extract_response_data, _response_detail_text from basic_memory.schemas.base import Entity -from basic_memory.schemas.v2.entity import EntityResponseV2 +from basic_memory.schemas.v2.entity import EntityResolveResponse, EntityResponseV2 from basic_memory.services.link_resolver import ( detect_project_from_workspace_identifier_prefix, is_workspace_qualified_plain_identifier, @@ -740,13 +740,16 @@ async def edit_note( # Set once the create or PATCH request leaves this process; the handler below # needs it to tell a refusal from a write whose outcome is unknown. write_sent = False + # The note the edit targets; a revision conflict re-reads it by id (#1719). + target_entity_id: str | None = None + # Import here to avoid circular import + from basic_memory.mcp.clients import KnowledgeClient + + # Use typed KnowledgeClient for API calls; the failure handler below also + # uses it to name the note in a revision conflict. + knowledge_client = KnowledgeClient(client, active_project.external_id) # Use the PATCH endpoint to edit the entity try: - # Import here to avoid circular import - from basic_memory.mcp.clients import KnowledgeClient - - # Use typed KnowledgeClient for API calls - knowledge_client = KnowledgeClient(client, active_project.external_id) unresolved_project_route: UnresolvedProjectRouteError | None = None try: _, entity_identifier, _ = await resolve_project_and_path( @@ -807,6 +810,7 @@ async def edit_note( ), ) entity_id = resolved_entity.external_id + target_entity_id = entity_id except EditRefused: # The cross-project refusal above must not be read as a missing note: # its text says "Not Found", which would route it to auto-create. @@ -1073,12 +1077,26 @@ async def edit_note( conflict = _revision_conflict_detail(e) if expected_checksum is not None else None if conflict is not None: current_checksum = conflict.get("db_checksum") + # Same shape as write_note's conflict (#1719): the note is named while + # it still exists, as it reads now, since the winning write may have + # retitled or moved it after this edit resolved it. A deleted note + # (no current checksum) has no identity. + named_note: EntityResolveResponse | None = None + if current_checksum is not None and target_entity_id is not None: + try: + named_note = await knowledge_client.resolve_entity_response( + target_entity_id, strict=True + ) + except ToolError: + # Deleted after the conflict: the refusal still stands and + # carries its checksum; there is just nothing left to name. + named_note = None _raise_edit_failure( output_format, { - "title": None, - "permalink": None, - "file_path": None, + "title": named_note.title if named_note else None, + "permalink": named_note.permalink if named_note else None, + "file_path": named_note.file_path if named_note else None, "checksum": None, "operation": operation, "fileCreated": False, diff --git a/src/basic_memory/mcp/tools/write_note.py b/src/basic_memory/mcp/tools/write_note.py index ddaae9457..dfb518ad0 100644 --- a/src/basic_memory/mcp/tools/write_note.py +++ b/src/basic_memory/mcp/tools/write_note.py @@ -558,15 +558,28 @@ async def write_note( case NoteLocked(message=message): raise ToolError(message) case NoteRevisionConflict(db_checksum=current_checksum) as conflict: + # Same shape as edit_note's conflict (#1719): nothing was written, + # so checksum is null; currentChecksum is the revision to retry + # against; identity names the note at the path, or is null once + # no note owns it. + conflict_permalink = conflict.permalink + conflict_workspace = current_workspace_permalink_context() + if conflict_permalink and conflict_workspace is not None: + conflict_permalink = build_qualified_permalink_reference( + active_project.permalink, + conflict_permalink, + workspace_permalink=conflict_workspace.workspace_slug, + ) _raise_write_refusal( output_format, { - "title": title, - "permalink": entity.permalink, + "title": conflict.title, + "permalink": conflict_permalink, "file_path": conflict.file_path if current_checksum else None, - "checksum": current_checksum, + "checksum": None, "action": "conflict", "error": "NOTE_REVISION_CONFLICT", + "currentChecksum": current_checksum, }, _format_revision_conflict( title, conflict.file_path, current_checksum, active_project.name diff --git a/src/basic_memory/schemas/v2/note_write.py b/src/basic_memory/schemas/v2/note_write.py index a347593d4..5c2851ffa 100644 --- a/src/basic_memory/schemas/v2/note_write.py +++ b/src/basic_memory/schemas/v2/note_write.py @@ -61,6 +61,10 @@ class NoteRevisionConflict(BaseModel): file_path: str # None when no note owns the path any more. db_checksum: str | None + # The note at the path. Optional so older servers that send only file_path and + # db_checksum still validate; None when no note owns the path any more. + title: str | None = None + permalink: str | None = None type WriteNoteResponse = Annotated[ diff --git a/src/basic_memory/services/entity_service.py b/src/basic_memory/services/entity_service.py index 553a898d1..0fc2d6ea5 100644 --- a/src/basic_memory/services/entity_service.py +++ b/src/basic_memory/services/entity_service.py @@ -767,7 +767,12 @@ def _apply_markdown_entity_fields( entity.permalink = markdown.frontmatter.permalink entity.file_path = file_path.as_posix() entity.content_type = "text/markdown" - entity.created_at = markdown.created + # Trigger: an existing row is re-read from Markdown (edit, move, reindex). + # Why: without a declared `created`, markdown.created is the file's ctime, which + # every atomic rewrite or rename moves (#1716). + # Outcome: only the author's frontmatter `created` changes the creation time. + if markdown.frontmatter.metadata.get("created") is not None: + entity.created_at = markdown.created entity.updated_at = markdown.modified normalized_metadata = normalize_frontmatter_metadata(markdown.frontmatter.metadata or {}) diff --git a/src/basic_memory/services/note_content_writes.py b/src/basic_memory/services/note_content_writes.py index 5b5e72efb..81d81ee12 100644 --- a/src/basic_memory/services/note_content_writes.py +++ b/src/basic_memory/services/note_content_writes.py @@ -522,7 +522,27 @@ async def write_note( "message": str() as message, "db_checksum": (str() | None) as current, } if expected_checksum is not None and message == STALE_BASE_CHECKSUM_MESSAGE: - return RevisionConflict(data.file_path, current_db_checksum=current) + # The pre-lock lookup can predate the winning write, which may have + # retitled the note, so name it as it reads now (#1719). A note + # deleted under the update lock has no current revision and no name. + current_note: NoteLocation | None = None + if target is not None and current is not None: + async with self.session_maker() as session: + winner = await entity_repository.get_by_external_id( + session, target.external_id, load_relations=False + ) + if winner is not None: + current_note = NoteLocation( + str(winner.external_id), + winner.title, + winner.file_path, + winner.permalink, + ) + return RevisionConflict( + data.file_path, + current_db_checksum=current, + note=current_note, + ) case _: raise diff --git a/src/basic_memory/services/note_preparation.py b/src/basic_memory/services/note_preparation.py index d7cce12c1..89eea0352 100644 --- a/src/basic_memory/services/note_preparation.py +++ b/src/basic_memory/services/note_preparation.py @@ -2,7 +2,9 @@ from __future__ import annotations +import re from copy import deepcopy +from itertools import dropwhile from dataclasses import dataclass from datetime import datetime from pathlib import Path @@ -602,6 +604,10 @@ def replace_section_content( return "\n".join([*lines[: section_line_index + 1], new_content, *lines[end_index:]]) +# A Markdown list item line: a bullet (-, *, +) or an ordered marker (1. or 1)). +_LIST_ITEM_LINE = re.compile(r"^\s*(?:[-*+]|\d{1,9}[.)])(?:\s|$)") + + def insert_relative_to_section( current_content: str, section_header: str, @@ -635,6 +641,21 @@ def insert_relative_to_section( insert_lines = ["", *insert_lines] return "\n".join([*before, *insert_lines, "", *lines[index:]]) after = lines[index + 1 :] + spacer = len(after) - len(list(dropwhile(lambda line: not line.strip(), after))) + first_content = after[spacer] if spacer < len(after) else None + # Trigger: a list item is inserted and the section opens with a list, either + # right under the heading or after the conventional blank line. + # Why: a blank line between the inserted item and the list splits it in two + # (#1720). + # Outcome: the item joins the list tightly, below any heading spacer. + if ( + first_content is not None + and _LIST_ITEM_LINE.match(insert_lines[-1]) + and _LIST_ITEM_LINE.match(first_content) + ): + return "\n".join([*lines[: index + 1], *after[:spacer], *insert_lines, *after[spacer:]]) + # Anything else keeps a blank line, so an inserted paragraph never runs into + # the section's first paragraph. if after and after[0].strip(): insert_lines.append("") return "\n".join([*lines[: index + 1], *insert_lines, *after]) diff --git a/src/basic_memory/services/note_write_outcomes.py b/src/basic_memory/services/note_write_outcomes.py index 767da783c..12a44d784 100644 --- a/src/basic_memory/services/note_write_outcomes.py +++ b/src/basic_memory/services/note_write_outcomes.py @@ -52,11 +52,12 @@ class Locked: class RevisionConflict: """The note at the path is no longer the revision the caller expected to replace. - ``current_db_checksum`` is None when no note owns the path any more. + ``current_db_checksum`` and ``note`` are None when no note owns the path any more. """ file_path: str current_db_checksum: str | None + note: NoteLocation | None = None @dataclass(frozen=True, slots=True) diff --git a/test-int/mcp/test_edit_note_integration.py b/test-int/mcp/test_edit_note_integration.py index bda7066fd..4c8f2aff1 100644 --- a/test-int/mcp/test_edit_note_integration.py +++ b/test-int/mcp/test_edit_note_integration.py @@ -1304,6 +1304,66 @@ async def test_edit_note_metadata_null_values_rejected_before_auto_create( assert "status" in error_message +@pytest.mark.asyncio +async def test_edit_note_conflict_names_the_note_as_the_winning_write_left_it( + mcp_server, app, test_project, monkeypatch +): + """A conflict reports the note's identity after the race, not the stale lookup (#1719).""" + async with Client(mcp_server) as client: + created = await client.call_tool( + "write_note", + { + "project": test_project.name, + "title": "Old Edit Title", + "directory": "test", + "content": "# Old Edit Title\n\nBody.", + "output_format": "json", + }, + ) + revision_a = json.loads(created.content[0].text)["checksum"] + + original_patch = KnowledgeClient.patch_entity + retitled = False + + async def retitle_before_patch(self, entity_id, patch_data, *, base_checksum=None): + # Another writer retitles the note (through its H1) after edit_note + # resolved it but before the guarded PATCH lands. + nonlocal retitled + if not retitled: + retitled = True + await original_patch( + self, + entity_id, + { + "operation": "find_replace", + "find_text": "# Old Edit Title", + "content": "# New Edit Title", + }, + ) + return await original_patch(self, entity_id, patch_data, base_checksum=base_checksum) + + monkeypatch.setattr(KnowledgeClient, "patch_entity", retitle_before_patch) + result = await client.call_tool( + "edit_note", + { + "project": test_project.name, + "identifier": "test/old-edit-title", + "operation": "append", + "content": "\nStale edit.", + "expected_checksum": revision_a, + "output_format": "json", + }, + raise_on_error=False, + ) + + assert result.is_error is True + payload = json.loads(result.content[0].text) + assert payload["error"] == "NOTE_REVISION_CONFLICT" + assert payload["currentChecksum"] not in (None, revision_a) + assert payload["title"] == "New Edit Title" + assert payload["file_path"] == "test/Old Edit Title.md" + + @pytest.mark.asyncio async def test_edit_note_expected_checksum_edits_only_the_revision_read( mcp_server, app, test_project @@ -1354,6 +1414,12 @@ async def test_edit_note_expected_checksum_edits_only_the_revision_read( payload = json.loads(stale_json.content[0].text) assert payload["error"] == "NOTE_REVISION_CONFLICT" assert payload["currentChecksum"] == revision_b + # Same shape as write_note's conflict (#1719): the note is named, nothing written. + updated_json = json.loads(updated.content[0].text) + assert payload["checksum"] is None + assert payload["title"] == updated_json["title"] + assert payload["permalink"] == updated_json["permalink"] + assert payload["file_path"] == updated_json["file_path"] stale_text = await client.call_tool( "edit_note", @@ -1401,6 +1467,7 @@ async def test_edit_note_expected_checksum_never_auto_creates(mcp_server, app, t payload = json.loads(result.content[0].text) assert payload["error"] == "NOTE_REVISION_CONFLICT" assert payload["currentChecksum"] is None + assert payload["title"] is None missing = await client.call_tool( "read_note", diff --git a/test-int/mcp/test_write_note_integration.py b/test-int/mcp/test_write_note_integration.py index 258829e49..dc9543898 100644 --- a/test-int/mcp/test_write_note_integration.py +++ b/test-int/mcp/test_write_note_integration.py @@ -694,7 +694,13 @@ async def test_write_note_expected_checksum_replaces_only_the_revision_read( stale_json = _json_content(stale_json_result) assert stale_json["action"] == "conflict" assert stale_json["error"] == "NOTE_REVISION_CONFLICT" - assert stale_json["checksum"] == revision_b + # Same shape as edit_note's conflict (#1719): nothing written, the current + # revision under currentChecksum, and the note at the path named. + assert stale_json["checksum"] is None + assert stale_json["currentChecksum"] == revision_b + assert stale_json["title"] == updated["title"] + assert stale_json["permalink"] == updated["permalink"] + assert stale_json["file_path"] == updated["file_path"] stale_text = await client.call_tool( "write_note", diff --git a/test-int/test_note_write_outcomes.py b/test-int/test_note_write_outcomes.py index efec9753d..80b461461 100644 --- a/test-int/test_note_write_outcomes.py +++ b/test-int/test_note_write_outcomes.py @@ -258,6 +258,9 @@ async def test_expected_checksum_replaces_only_the_revision_the_caller_read( "kind": "revision_conflict", "file_path": "notes/Conditional.md", "db_checksum": revision_b, + # The note at the path is named, so callers need not derive it (#1719). + "title": "Conditional", + "permalink": updated.json()["entity"]["permalink"], } assert path.read_bytes() == after_b @@ -279,6 +282,8 @@ async def test_expected_checksum_never_creates_a_missing_note( "kind": "revision_conflict", "file_path": "notes/Deleted Since.md", "db_checksum": None, + "title": None, + "permalink": None, } assert not (Path(test_project.path) / "notes/Deleted Since.md").exists() @@ -346,6 +351,8 @@ async def move_before_update(self: NoteContentMutationService, **kwargs: Any): "kind": "revision_conflict", "file_path": "notes/Moving Target.md", "db_checksum": entity["db_checksum"], + "title": "Moving Target", + "permalink": entity["permalink"], } current = (await client.get(entity_url)).json() assert current["file_path"] == "archive/Moving Target.md" @@ -353,6 +360,60 @@ async def move_before_update(self: NoteContentMutationService, **kwargs: Any): assert "Stale replacement" not in current["content"] +async def test_revision_conflict_names_the_note_as_the_winning_write_left_it( + client: AsyncClient, + test_project: Project, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A conflict reports the note's identity after the race, not the stale lookup (#1719).""" + endpoint = f"/v2/projects/{test_project.external_id}/knowledge/write" + note = {"title": "Old Title", "directory": "notes", "content": "# Old Title\n\nBody"} + created = await client.post(endpoint, json={"note": note}) + entity = created.json()["entity"] + entity_url = ( + f"/v2/projects/{test_project.external_id}/knowledge/entities/{entity['external_id']}" + ) + original_update = NoteContentMutationService.update_note + retitled = False + + async def retitle_before_update(self: NoteContentMutationService, **kwargs: Any): + # Another writer retitles the note (through its H1) after write_note looked + # up the path but before the guarded replacement takes the note lock. + nonlocal retitled + if not retitled: + retitled = True + edited = await client.patch( + entity_url, + json={ + "operation": "find_replace", + "find_text": "# Old Title", + "content": "# New Title", + }, + ) + assert edited.status_code == 202, edited.text + return await original_update(self, **kwargs) + + monkeypatch.setattr(NoteContentMutationService, "update_note", retitle_before_update) + response = await client.post( + endpoint, + json={ + "note": {**note, "content": "# Old Title\n\nStale replacement"}, + "overwrite": True, + "expected_checksum": entity["db_checksum"], + }, + ) + + current = (await client.get(entity_url)).json() + assert current["title"] == "New Title" + assert response.json() == { + "kind": "revision_conflict", + "file_path": "notes/Old Title.md", + "db_checksum": current["db_checksum"], + "title": "New Title", + "permalink": current["permalink"], + } + + @pytest.mark.parametrize("conditional", [True, False], ids=["conditional", "unconditional"]) async def test_a_note_deleted_under_the_update_lock_is_a_revision_conflict_when_pinned( client: AsyncClient, @@ -382,6 +443,8 @@ async def content_deleted(*_args: object, **_kwargs: object) -> None: "kind": "revision_conflict", "file_path": "notes/Vanishing.md", "db_checksum": None, + "title": None, + "permalink": None, } else: # Without a pinned revision the missing content stays Core's backfill refusal. diff --git a/tests/cli/test_cli_tool_json_output.py b/tests/cli/test_cli_tool_json_output.py index 112b268cf..76e378781 100644 --- a/tests/cli/test_cli_tool_json_output.py +++ b/tests/cli/test_cli_tool_json_output.py @@ -195,7 +195,14 @@ def test_write_note_expected_checksum_passthrough(mock_mcp_write): "basic_memory.mcp.tools.write_note", new_callable=AsyncMock, side_effect=ToolError( - json.dumps({"action": "conflict", "error": "NOTE_REVISION_CONFLICT", "checksum": "b" * 64}) + json.dumps( + { + "action": "conflict", + "error": "NOTE_REVISION_CONFLICT", + "checksum": None, + "currentChecksum": "b" * 64, + } + ) ), ) def test_write_note_revision_conflict_exits_nonzero(mock_mcp_write): @@ -221,7 +228,7 @@ def test_write_note_revision_conflict_exits_nonzero(mock_mcp_write): assert "Error: NOTE_REVISION_CONFLICT" in result.stderr payload = json.loads(result.stdout) assert payload["error"] == "NOTE_REVISION_CONFLICT" - assert payload["checksum"] == "b" * 64 + assert payload["currentChecksum"] == "b" * 64 @patch( diff --git a/tests/indexing/test_batch_indexer.py b/tests/indexing/test_batch_indexer.py index a3f858fa1..fd8823c9e 100644 --- a/tests/indexing/test_batch_indexer.py +++ b/tests/indexing/test_batch_indexer.py @@ -409,6 +409,66 @@ async def test_batch_indexer_preserves_markdown_semantic_timestamps_on_reindex( assert excluded == [] +@pytest.mark.asyncio +async def test_batch_indexer_keeps_created_at_when_a_rewrite_moves_the_file_ctime( + app_config, + entity_service, + entity_repository, + relation_repository, + search_service, + file_service, + project_config, +): + """Reindexing a rewritten note keeps its creation time (#1716). + + Without a frontmatter `created`, the parser falls back to the file's ctime, + which every atomic rewrite moves. Only a declared `created` may change it. + """ + path = "notes/undeclared-created.md" + absolute_path = project_config.home / path + frontmatter = "---\ntitle: Undeclared Created\ntype: note\n---\n" + first_ctime = datetime(2024, 3, 1, 9, 0, tzinfo=UTC) + rewrite_ctime = datetime(2025, 6, 1, 9, 0, tzinfo=UTC) + batch_indexer = _make_batch_indexer( + app_config, + entity_service, + entity_repository, + relation_repository, + search_service, + file_service, + ) + + await _create_file(absolute_path, f"{frontmatter}First body\n") + first_input = await _load_input(file_service, path) + first_input.created_at = first_ctime + assert (await batch_indexer.index_files({path: first_input}, max_concurrent=1)).errors == [] + + await _create_file(absolute_path, f"{frontmatter}Second body\n") + rewrite_input = await _load_input(file_service, path) + rewrite_input.created_at = rewrite_ctime + assert (await batch_indexer.index_files({path: rewrite_input}, max_concurrent=1)).errors == [] + + async with db.scoped_session(search_service.session_maker) as session: + rewritten = await entity_repository.get_by_file_path(session, path) + assert rewritten is not None + assert rewritten.created_at == first_ctime + + # The author's declared creation time is the one value allowed to move it. + await _create_file( + absolute_path, + "---\ntitle: Undeclared Created\ntype: note\ncreated: 2023-01-02T03:04:05Z\n---\n" + "Third body\n", + ) + declared_input = await _load_input(file_service, path) + declared_input.created_at = rewrite_ctime + assert (await batch_indexer.index_files({path: declared_input}, max_concurrent=1)).errors == [] + + async with db.scoped_session(search_service.session_maker) as session: + declared = await entity_repository.get_by_file_path(session, path) + assert declared is not None + assert declared.created_at == datetime(2023, 1, 2, 3, 4, 5, tzinfo=UTC) + + @pytest.mark.asyncio async def test_batch_indexer_returns_original_markdown_content_when_no_frontmatter_rewrite( app_config, diff --git a/tests/mcp/test_tool_build_context.py b/tests/mcp/test_tool_build_context.py index b31239bbb..cecc6d39b 100644 --- a/tests/mcp/test_tool_build_context.py +++ b/tests/mcp/test_tool_build_context.py @@ -313,6 +313,7 @@ def test_format_entity_block_renders_unresolved_relations_by_name(): permalink="man3/write-note-3/see-also/edit-note-3", relation_type="see_also", from_entity="write-note(3)", + from_entity_external_id="entity-1", to_entity=None, to_name="edit-note(3)", created_at=now, @@ -323,6 +324,7 @@ def test_format_entity_block_renders_unresolved_relations_by_name(): permalink="man3/write-note-3/see-also/bm-note-5", relation_type="see_also", from_entity="write-note(3)", + from_entity_external_id="entity-1", to_entity="bm-note(5)", to_name="bm-note(5)", created_at=now, @@ -336,6 +338,36 @@ def test_format_entity_block_renders_unresolved_relations_by_name(): assert "[[None]]" not in block +@pytest.mark.asyncio +async def test_build_context_text_names_the_source_of_incoming_relations(client, test_project): + """An incoming relation names its source instead of reading as a self-link (#1717).""" + await write_note( + project=test_project.name, + title="Bean Origins", + directory="coffee", + content="# Bean Origins\n\n- informs [[Brewing Notes]]\n", + ) + await write_note( + project=test_project.name, + title="Brewing Notes", + directory="coffee", + content="# Brewing Notes\n\n- relates_to [[Bean Origins]]\n", + ) + + result = await build_context( + project=test_project.name, + url="memory://coffee/bean-origins", + output_format="text", + ) + + assert isinstance(result, str) + # Owned by the primary note: unchanged, bare form. + assert "- informs [[Brewing Notes]]" in result + # Incoming: the source is named, so it no longer reads as Bean Origins -> itself. + assert "- [[Brewing Notes]] relates_to [[Bean Origins]]" in result + assert "\n- relates_to [[Bean Origins]]" not in result + + @pytest.mark.asyncio @pytest.mark.parametrize( "url_template", diff --git a/tests/services/test_entity_service.py b/tests/services/test_entity_service.py index f4430ef47..01ea3f147 100644 --- a/tests/services/test_entity_service.py +++ b/tests/services/test_entity_service.py @@ -1871,6 +1871,64 @@ async def test_edit_entity_insert_after_section( assert file_content.index("## Section 1") < file_content.index( "Inserted after section 1 heading" ) + # A paragraph inserted above a paragraph keeps a blank line, so they stay apart. + assert "Inserted after section 1 heading\n\nSection 1 content" in file_content + + +@pytest.mark.asyncio +async def test_edit_entity_insert_after_section_keeps_a_list_tight( + entity_service: EntityService, file_service: FileService +): + """A list item inserted above a list item does not split the list (#1720).""" + entity = await entity_service.create_entity( + EntitySchema( + title="Insert Into List", + directory="docs", + note_type="note", + content="# Origins\n\n## Observations\n- [origin] Ethiopia\n- [origin] Colombia\n", + ) + ) + + updated = await entity_service.edit_entity( + identifier=_permalink(entity), + operation="insert_after_section", + content="- [origin] Kenya", + section="## Observations", + ) + + file_content, _ = await file_service.read_file(file_service.get_entity_path(updated)) + assert ( + "## Observations\n- [origin] Kenya\n- [origin] Ethiopia\n- [origin] Colombia" + in file_content + ) + + +@pytest.mark.asyncio +async def test_edit_entity_insert_after_section_keeps_a_spaced_list_tight( + entity_service: EntityService, file_service: FileService +): + """With a blank line under the heading, the item joins the list below it (#1720).""" + entity = await entity_service.create_entity( + EntitySchema( + title="Insert Into Spaced List", + directory="docs", + note_type="note", + content="# Origins\n\n## Observations\n\n- [origin] Ethiopia\n- [origin] Colombia\n", + ) + ) + + updated = await entity_service.edit_entity( + identifier=_permalink(entity), + operation="insert_after_section", + content="- [origin] Kenya", + section="## Observations", + ) + + file_content, _ = await file_service.read_file(file_service.get_entity_path(updated)) + assert ( + "## Observations\n\n- [origin] Kenya\n- [origin] Ethiopia\n- [origin] Colombia" + in file_content + ) @pytest.mark.asyncio