Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 22 additions & 9 deletions src/basic_memory/services/entity_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -1269,13 +1269,22 @@ def replace_section_content(
) -> str:
"""Replace content under a specific markdown section header.

This method uses a simple, safe approach: when replacing a section, it only
replaces the immediate content under that header until it encounters the next
header of ANY level. This means:
When replacing a section, we consume the original content under the header until
we hit any other heading, with one important refinement: a same-level or
higher-level heading that appears **directly** after the target (no intervening
subsections) is also consumed, so replacement content can authoritatively
re-define that sibling without leaving the original duplicated below the
replacement (issue #1012). A strictly deeper heading (`###` under a replaced
`##`) is always treated as a subsection and preserved.

In practice:

- Replacing "# Header" replaces content until "## Subsection" (preserves subsections)
- Replacing "## Section" replaces content until "### Subsection" (preserves subsections)
- More predictable and safer than trying to consume entire hierarchies
- Replacing "## Section" consumes a directly-following "## Sibling" or
"# Sibling" so replacement content can re-author it
- A same-level sibling that appears only after one or more preserved
subsections is left untouched (it belongs to the next top-level block)

Args:
current_content: The current markdown content
Expand Down Expand Up @@ -1323,6 +1332,7 @@ def replace_section_content(
# Replace the single matching section
result_lines = []
section_line_idx = matching_sections[0]
section_header_level = len(section_header) - len(section_header.lstrip("#"))

i = 0
while i < len(lines):
Expand All @@ -1335,15 +1345,18 @@ def replace_section_content(
result_lines.append(new_content)
i += 1

# Skip the original section content until next header or end
# Consume content directly after the target until we hit a header.
# Same-level/higher-level headings here are consumed so the
# replacement can re-author them without leaving duplicates below
# (issue #1012). Strictly-deeper headings (subsections) stop the
# consume run and are preserved.
while i < len(lines):
next_line = lines[i]
# Stop consuming when we hit any header (preserve subsections)
if next_line.startswith("#"):
# We found another header - continue processing from here
break
next_level = len(next_line) - len(next_line.lstrip("#"))
if next_level > section_header_level:
break
Comment on lines 1355 to +1358

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Stop deleting unrelated sibling sections

When a user performs a normal targeted replacement without re-authoring later sections, such as replacing ## Section 1 with only new Section 1 content in a note whose next heading is ## Section 2, this condition keeps consuming same- and higher-level headings and silently drops the rest of that tail from the file. The #1012 duplication case only requires removing a sibling that the replacement actually includes; otherwise this changes replace_section from a section-local edit into a data-losing overwrite of unrelated sections and relations.

Useful? React with 👍 / 👎.

i += 1
# Continue processing from the next header (don't increment i again)
continue

# Add all other lines (including subsequent sections)
Expand Down
85 changes: 82 additions & 3 deletions tests/services/test_entity_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,79 @@ async def test_delete_nonexistent_entity(entity_service: EntityService):
assert await entity_service.delete_entity("test/non_existent") is True


@pytest.mark.asyncio
async def test_replace_section_consumes_same_level_siblings(entity_service: EntityService):
"""replace_section must consume same-level sibling headings when replacement re-authors them.

Regression for #1012: the original implementation stopped at any `#` heading after
the target section, so a replacement that re-defined a sibling (e.g. replaced
`## Observations` with content that itself includes `## Description`) left the
original sibling duplicated below the replacement. Subsections (`### ...`) under
the target must still be preserved.
"""

current = (
"## Observations\n"
"- [status] open\n"
"- [priority] high\n"
"\n"
"## Description\n"
"Jamie called in...\n"
"\n"
"## Relations\n"
"- reported_by [[Jamie Rivera]]\n"
)

new_content = (
"- [status] resolved\n"
"- [resolution] shipped\n"
"\n"
"## Description\n"
"Updated...\n"
"\n"
"## Resolution\n"
"Fix details...\n"
)

result = entity_service.replace_section_content(current, "## Observations", new_content)

# Replacement content is present.
assert "- [status] resolved" in result
assert "## Resolution" in result
# Replacement re-authored `## Description`; original `Jamie called in...` must be gone.
assert "Jamie called in..." not in result
assert result.count("## Description") == 1
# Original `## Relations` (sibling of `## Observations`) is consumed by the
# replacement -- the user is replacing the whole observations block, not appending.
assert "reported_by" not in result


@pytest.mark.asyncio
async def test_replace_section_preserves_deeper_subsections(entity_service: EntityService):
"""replace_section must preserve subsections (strictly deeper headings) under the target."""

current = (
"## Implementation\n"
"Top-level summary.\n"
"\n"
"### Detail A\n"
"Detail A body.\n"
"\n"
"### Detail B\n"
"Detail B body.\n"
)

result = entity_service.replace_section_content(
current,
"## Implementation",
"New top-level summary.\n",
)

assert "New top-level summary." in result
assert "### Detail A" in result and "Detail A body." in result
assert "### Detail B" in result and "Detail B body." in result


@pytest.mark.asyncio
async def test_create_entity_with_special_chars(entity_service: EntityService):
"""Test entity creation with special characters in name and description."""
Expand Down Expand Up @@ -892,12 +965,15 @@ async def test_edit_entity_replace_section(
section="## Section 1",
)

# Verify section was replaced
# Verify section was replaced. Directly-following same-level siblings are
# consumed so replacement content can authoritatively re-author them without
# leaving the original duplicated (issue #1012).
file_path = file_service.get_entity_path(updated)
file_content, _ = await file_service.read_file(file_path)
assert "New section 1 content" in file_content
assert "Original section 1 content" not in file_content
assert "Original section 2 content" in file_content # Other sections preserved
assert "Original section 2 content" not in file_content # Same-level sibling consumed
assert file_content.count("## Section 2") == 0 # Original sibling gone (replacement can re-author)


@pytest.mark.asyncio
Expand Down Expand Up @@ -1498,7 +1574,10 @@ async def test_edit_entity_replace_section_strips_duplicate_header(

assert "New content for testing section" in file_content
assert "Original content" not in file_content
assert "## Another Section" in file_content # Other sections preserved
# Directly-following same-level sibling is consumed so replacement content can
# re-author it without leaving the original duplicated (issue #1012).
assert "## Another Section" not in file_content
assert "Other content" not in file_content


# Insert before/after section tests
Expand Down