feat: add insert_before_section and insert_after_section edit operations

[phernandez]

Summary

Adds two new edit_note operations — insert_before_section and insert_after_section — that use section headings as positional anchors to insert content without consuming the heading.

• insert_before_section: inserts content above a section heading
• insert_after_section: inserts content below a section heading
• Both require unique section headers (error on duplicates) and raise ValueError if the section is not found
• Schema, service, and MCP tool layers all updated consistently
• Full test coverage: unit tests, MCP tool tests, and schema validation tests

This addresses a real problem where LLMs trained on str_replace patterns consistently eat section headings when using find_replace to insert content near them.

Based on the work by @Brennall in #647, addressing #646.

Changes

• src/basic_memory/schemas/request.py — new operations in Literal type, updated section validator
• src/basic_memory/services/entity_service.py — insert_relative_to_section() method + routing in apply_edit_operation()
• src/basic_memory/mcp/tools/edit_note.py — validation, response formatting, description updates
• tests/services/test_entity_service.py — 8 new unit tests
• tests/mcp/test_tool_edit_note.py — 4 new MCP tool tests
• tests/schemas/test_schemas.py — 3 new schema validation tests

Test plan

• pytest tests/services/test_entity_service.py -v -k "insert" — 8 unit tests pass
• pytest tests/mcp/test_tool_edit_note.py -v -k "insert" — 4 MCP tool tests pass
• pytest tests/schemas/test_schemas.py -v — all schema tests pass
• pytest tests/mcp/test_tool_edit_note.py tests/schemas/test_schemas.py -v — all 79 tests pass
• Lint and format clean
• CI: full SQLite + Postgres test suite

🤖 Generated with Claude Code

[github-actions]

Claude finished @phernandez's task in 2m 25s —— View job

---

PR Review: feat/insert-before-after-section

Tasks:

• Gather context and understand the request
• Read changed files
• Review against team checklist
• Post comprehensive review

---

Summary

This is a clean, well-motivated feature that addresses a real problem (LLMs eating section headings with find_replace). The implementation is consistent across schema, service, and MCP tool layers, and the test coverage is solid. I found a few minor issues worth addressing before merge.

---

Code Quality & Standards

• Follows Basic Memory's coding conventions in CLAUDE.md
• Python 3.12+ type annotations and async patterns (minor issue below)
• SQLAlchemy 2.0 best practices — N/A
• FastAPI and Typer conventions followed
• 100-character line length limit maintained
• No commented-out code blocks

Testing & Documentation

• Unit tests for new functions/methods (8 unit tests)
• Integration tests for new MCP tools (4 MCP tool tests)
• [~] Test coverage for edge cases — see gaps below
• 100% test coverage likely maintained
• [~] Documentation — minor stale docstrings (see below)
• CLAUDE.md unchanged — appropriate

Basic Memory Architecture

• MCP tools follow atomic, composable design
• No database changes — no migration needed
• Preserves local-first architecture principles
• Knowledge graph operations consistent
• Markdown file handling preserves integrity

Security & Performance

• No hardcoded secrets or credentials
• Input validation present at all layers (schema, service, MCP)
• Proper error handling and logging
• Performance fine (linear scan, no DB overhead)
• No sensitive data in logs

Compatibility

• No file path comparisons added
• No emojis/unicode in console or log output

---

Issues Found

1. Blank line asymmetry in insert_after_section (behavioral concern)

File: src/basic_memory/services/entity_service.py:1046-1051

The before branch adds a blank line separator between inserted content and the section heading, but the after branch does not:

if position == "before":
    insert_lines = new_content.rstrip("\n").split("\n")
    if before and before[-1].strip() != "":
        insert_lines = [""] + insert_lines
    return "\n".join(before + insert_lines + [""] + after)  # blank line before heading ✓
else:
    insert_lines = new_content.rstrip("\n").split("\n")
    return "\n".join(before + insert_lines + after)  # no blank line after heading ✗

With input ## Section 1\nExisting content and inserted text New content, insert_after_section produces:

## Section 1
New content
Existing content

This may be the intent, but it diverges from how insert_before_section pads with blank lines. Consider whether insert_after_section should also add a blank line after the heading before content (i.e., before + [""] + insert_lines + after). Fix this →

2. Stale validator docstring

File: src/basic_memory/schemas/request.py:83

@field_validator("section")
@classmethod
def validate_section_for_replace_section(cls, v, info):
    """Ensure section is provided for replace_section operation."""  # ← stale

The docstring and method name only mention replace_section but the validator now also covers insert_before_section and insert_after_section. Fix this →

3. Incomplete section param docstring in MCP tool

File: src/basic_memory/mcp/tools/edit_note.py:198

section: For replace_section operation - the markdown header to replace content under...

This no longer accurately describes section — it's also required for insert_before_section and insert_after_section. Fix this →

4. position parameter type is str not Literal

File: src/basic_memory/services/entity_service.py:994

def insert_relative_to_section(
    self,
    current_content: str,
    section_header: str,
    new_content: str,
    position: str,  # ← could be Literal["before", "after"]
) -> str:

Using Literal["before", "after"] would make the contract explicit and enable type-checking at call sites. Fix this →

5. Missing error-response handling for new ops in _format_error_response

File: src/basic_memory/mcp/tools/edit_note.py:111

replace_section gets specific error guidance for "Multiple sections" — but insert_before_section and insert_after_section can raise the same error and will fall through to the generic handler. The specific guidance ("read the note first to see document structure") is especially relevant for these insert operations. Fix this →

6. Test coverage gap: no duplicate-section test for insert_after_section

File: tests/services/test_entity_service.py

test_edit_entity_insert_before_section_multiple_sections_error tests duplicate handling only for insert_before_section. An equivalent test for insert_after_section is missing. Both ops go through the same code path, but coverage ensures that stays true. Fix this →

---

Minor Notes

• The insert_after_section test at end of document (test_edit_entity_insert_after_section_at_end_of_document) does not assert that the inserted content is after ## Only Section. It only asserts both strings are present. Consider adding a positional assertion (file_content.index("## Only Section") < file_content.index("Inserted after the last section heading")).
• The matching logic in insert_relative_to_section (line.strip() == section_header.strip()) is stricter than it appears — it requires the heading level (##, ###, etc.) to match exactly. This is the right behavior but is worth documenting in the docstring.

---

Overall

The core implementation is solid and the motivation is clear. Issues #1 (blank line behavior) and #5 (error response) have the most practical impact. The rest are polish. Happy to approve after those are addressed.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0b0c072f09

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Preserve paragraph boundaries after insert_after_section

When insert_after_section inserts plain text into a section that already starts with paragraph text, the current join logic places the old first line immediately after the inserted line with only a single newline, so Markdown treats them as one paragraph. This means the operation can unintentionally merge newly inserted text with existing section content instead of inserting a distinct block beneath the heading; adding explicit blank-line separation (as the before branch already does) avoids that formatting regression.

Useful? React with 👍 / 👎.

[github-actions]

Claude finished @phernandez's task in 1m 38s —— View job

---

PR Review: feat/insert-before-after-section

Tasks:

• Gather context and understand the request
• Read changed files
• Review against team checklist
• Post comprehensive review

---

Summary

The fix commit (d01ca0d) addresses the blank-line asymmetry raised in the previous review — insert_after_section now correctly emits a separating blank line so that inserted text doesn't merge with existing section content into a single Markdown paragraph. A dedicated test (test_edit_entity_insert_after_section_preserves_paragraph_separation) was added to lock in this behavior. Good fix.

Five issues from the prior review remain open.

---

Code Quality & Standards

• Follows Basic Memory's coding conventions in CLAUDE.md
• Python 3.12+ type annotations and async patterns (minor open issue — see Create/update db on startup  #4)
• SQLAlchemy 2.0 best practices — N/A
• FastAPI and Typer conventions followed
• 100-character line length limit maintained
• No commented-out code blocks

Testing & Documentation

• Unit tests for new functions/methods (9 unit tests, including new paragraph-separation test)
• Integration tests for new MCP tool operations
• [~] Test coverage for edge cases — one gap remains (see Alembic #6)
• 100% test coverage maintained
• [~] Documentation — stale docstrings remain (see Memory json import #2, fix: fix permalink uniqueness violations on create/update/sync #3)
• CLAUDE.md unchanged — appropriate

Basic Memory Architecture

• MCP tools follow atomic, composable design
• No database changes — no migration needed
• Preserves local-first architecture principles
• Knowledge graph operations consistent
• Markdown file handling preserves integrity

Security & Performance

• No hardcoded secrets or credentials
• Input validation present at all layers (schema, service, MCP)
• Proper error handling and logging
• Performance fine (linear scan, no DB overhead)
• No sensitive data in logs

Compatibility

• No file path comparisons added
• No emojis/unicode in console or log output

---

Fixed Since Last Review

• Blank line asymmetry in insert_after_section (Issue Pre release #1) — after branch now appends a blank line when the next line is non-empty, preventing paragraph merging. New test test_edit_entity_insert_after_section_preserves_paragraph_separation asserts exact "Inserted line\n\nExisting paragraph text" ordering.

---

Remaining Issues

1. Stale validator docstring

File: src/basic_memory/schemas/request.py:84

@field_validator("section")
@classmethod
def validate_section_for_replace_section(cls, v, info):
    """Ensure section is provided for replace_section operation."""  # ← stale

The validator now also covers insert_before_section and insert_after_section. The docstring and method name still only mention replace_section. Fix this →

2. Incomplete section param docstring in MCP tool

File: src/basic_memory/mcp/tools/edit_note.py:198

section: For replace_section operation - the markdown header to replace content under...

section is also required by insert_before_section and insert_after_section. Fix this →

3. position parameter type is str not Literal

File: src/basic_memory/services/entity_service.py:995

position: str,  # ← should be Literal["before", "after"]

Using Literal["before", "after"] makes the contract explicit and enables type checking at call sites. Fix this →

4. _format_error_response doesn't handle new ops for duplicate-section error

File: src/basic_memory/mcp/tools/edit_note.py:111

if operation == "replace_section" and "Multiple sections" in error_message:

insert_before_section and insert_after_section raise the same "Multiple sections found" and "Section not found" errors, but will fall through to the generic fallback handler rather than receiving targeted guidance. Fix this →

5. Missing duplicate-section test for insert_after_section

File: tests/services/test_entity_service.py

test_edit_entity_insert_before_section_multiple_sections_error tests duplicate handling for insert_before_section but there is no equivalent test for insert_after_section. Both operations go through the same code path, but a test ensures that stays true. Fix this →

---

Minor Notes

• test_edit_entity_insert_after_section_at_end_of_document still only checks presence (in file_content) rather than asserting ordering. Consider adding file_content.index("## Only Section") < file_content.index("Inserted after the last section heading").
• The header-level matching in insert_relative_to_section (line.strip() == section_header.strip()) requires an exact match including heading level (##, ###, etc.). This is the right behavior but worth a docstring note since it differs from replace_section_content's matching.

---

Overall

The blank-line fix is clean and the new test is correctly specific (\n\nExisting paragraph text). The remaining issues (#1–#5 above) are all low-risk but straightforward to address before merge.

[github-actions]

Claude finished @phernandez's task in 1m 44s —— View job

---

PR Review: feat/insert-before-after-section

Tasks:

• Gather context and read changed files
• Check remaining issues from prior reviews
• Review against team checklist
• Post comprehensive review

---

What Changed in the Latest Commit

aa4547f (fix(test): update integration test for new section-based error message) updates test-int/cli/test_cli_tool_edit_note_integration.py to match the updated error message wording for section-based operations. That's the only change — it doesn't address any of the five open issues from the prior review.

---

Code Quality & Standards

• Follows Basic Memory's coding conventions in CLAUDE.md
• Python 3.12+ type annotations and async patterns (open issue — see fix: fix permalink uniqueness violations on create/update/sync #3)
• SQLAlchemy 2.0 best practices — N/A
• FastAPI and Typer conventions followed
• 100-character line length limit maintained
• No commented-out code blocks

Testing & Documentation

• Unit tests for new functions/methods (9 unit tests)
• Integration tests for new MCP tool operations
• [~] Test coverage for edge cases — one gap remains (see Pre release fixups #5)
• 100% test coverage maintained
• [~] Documentation — stale docstrings remain (see Pre release #1, Memory json import #2)
• CLAUDE.md unchanged — appropriate

Basic Memory Architecture

• MCP tools follow atomic, composable design
• No database changes — no migration needed
• Preserves local-first architecture principles
• Knowledge graph operations consistent
• Markdown file handling preserves integrity

Security & Performance

• No hardcoded secrets or credentials
• Input validation present at all layers (schema, service, MCP)
• Proper error handling and logging
• Performance fine (linear scan, no DB overhead)
• No sensitive data in logs

Compatibility

• No file path comparisons added
• No emojis/unicode in console or log output

---

Remaining Issues (unchanged from prior review)

All five issues identified in the previous two reviews are still open.

1. Stale validator docstring

File: src/basic_memory/schemas/request.py:84

def validate_section_for_replace_section(cls, v, info):
    """Ensure section is provided for replace_section operation."""  # ← stale

The validator now covers insert_before_section and insert_after_section too, but the docstring (and method name) only mention replace_section. Fix this →

2. Incomplete section param docstring in MCP tool

File: src/basic_memory/mcp/tools/edit_note.py:198

section: For replace_section operation - the markdown header to replace content under...

section is now required by all three section-based operations. Fix this →

3. position parameter type is str not Literal

File: src/basic_memory/services/entity_service.py:995

position: str,  # ← should be Literal["before", "after"]

Using Literal["before", "after"] makes the contract explicit and catches typos at type-checking time. Literal is already imported in the file. Fix this →

4. _format_error_response doesn't handle new ops for duplicate/missing section errors

File: src/basic_memory/mcp/tools/edit_note.py:111

if operation == "replace_section" and "Multiple sections" in error_message:

insert_before_section and insert_after_section raise the same "Multiple sections found" and "Section not found" errors, but will fall through to the generic fallback rather than getting the targeted guidance (which explicitly recommends reading the note first to see document structure — the most useful advice for these insert operations). Fix this →

5. Missing duplicate-section test for insert_after_section

File: tests/services/test_entity_service.py

test_edit_entity_insert_before_section_multiple_sections_error covers insert_before_section but there is no equivalent test for insert_after_section. Both share the same code path in insert_relative_to_section, but a symmetric test ensures that stays true after future refactors. Fix this →

---

Minor Notes (unchanged)

• test_edit_entity_insert_after_section_at_end_of_document still checks only in file_content rather than ordering. Consider adding file_content.index("## Only Section") < file_content.index("Inserted after the last section heading").
• The heading-level matching in insert_relative_to_section (line.strip() == section_header.strip()) requires an exact match including ##/### level. This is correct behavior but worth noting in the docstring since it differs from replace_section_content.

---

Overall

The integration test fix in this commit is correct. The core feature implementation remains solid. The five open issues are all small — issues #1 and #2 are one-line docstring fixes, #3 is a one-word type annotation change, #4 extends one condition and adds a block, and #5 is a copy-paste test. None block correctness, but addressing them before merge keeps the codebase consistent with CLAUDE.md's literate-programming standards and prevents the error handler gap from going unnoticed.