Repository navigation
fix(core): keep appended setext underlines from turning paragraphs into headings - #1698
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2456c79791
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 443df92577
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a07f286053
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d0a6a7117f
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cbde5398b7
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3c66684c31
ℹ️ 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".
…to headings edit_note joined appended and prepended text on a single newline. When a paragraph line ended up directly above a `---` or `===` line, CommonMark read the pair as a setext heading, so the existing paragraph became an H1/H2 and the section parser saw a heading nobody wrote. Insert a blank line only at that boundary. Every other join keeps the single newline, so appended list items and observations stay in a tight list. Fixes #1585. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01APFUk2bjEwMptMQqpRhjea Signed-off-by: phernandez <paul@basicmachines.co>
…ristic The one-line paragraph check misread block context. `paragraph\n2. item` is lazy paragraph continuation, so appending `---` still made an H2. A note ending inside an open fence or HTML block got a blank line inside the code or the block. Keep the cheap setext-underline prefilter, then parse the single-newline join with the section parser and add the blank line only when a setext heading would end on the appended underline. Applies to append and both prepend joins. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01APFUk2bjEwMptMQqpRhjea Signed-off-by: phernandez <paul@basicmachines.co>
The setext prefilter split on "\n" only, so `---\r\n` or `---\r` kept a trailing "\r" and was rejected, even though markdown-it treats both as line ends and still underlines the paragraph above. Match the underline line against all three terminators instead of splitting. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01APFUk2bjEwMptMQqpRhjea Signed-off-by: phernandez <paul@basicmachines.co>
…dings The setext check parsed the whole note, frontmatter included. A literal block scalar such as `meta: |` followed by a fence or `<div>` line read as an open block running to the end, so appending `---` after a body paragraph still produced an H2. Drop valid frontmatter before parsing, as EntityParser does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01APFUk2bjEwMptMQqpRhjea Signed-off-by: phernandez <paul@basicmachines.co>
…plicitly The setext check stripped frontmatter from whatever text preceded the join. For a prepend into a note that already has frontmatter, that text is a body fragment, so a `---` block inside it is Markdown; stripping it hid an open HTML block and the inserted blank line closed it. The check no longer guesses. Each caller passes the body text that ends at the join: the note body for append, the fragment itself when prepending into an existing body, and the fragment's body when it opens a note with no frontmatter. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01APFUk2bjEwMptMQqpRhjea Signed-off-by: phernandez <paul@basicmachines.co>
…ilter The setext-underline prefilter only recognized top-level underlines, so `> ---` after `> paragraph`, or an underline indented into a list item, skipped the parse and still produced a heading. Each round of review found another input variant the line pattern missed. Drop the prefilter and its regex. The join check always parses the joined body and adds a blank line only when a setext heading would end on the first line of the text after the join. Plain list item and observation appends still join on a single newline. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01APFUk2bjEwMptMQqpRhjea Signed-off-by: phernandez <paul@basicmachines.co>
3c66684 to
67b6a23
Compare
Entries for #1697, #1698, #1699, #1700, #1701, #1702, #1704 and #1705, which landed without changelog edits so the parallel PRs would not conflict. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01APFUk2bjEwMptMQqpRhjea Signed-off-by: phernandez <paul@basicmachines.co>
Entries for #1697, #1698, #1699, #1700, #1701, #1702, #1704 and #1705, which landed without changelog edits so the parallel PRs would not conflict. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01APFUk2bjEwMptMQqpRhjea Signed-off-by: phernandez <paul@basicmachines.co>
1 participant
Fixes #1585.
What was wrong
edit_notejoins appended and prepended text with a single newline. CommonMark reads a paragraph line followed directly by a run of-or=(up to 3 spaces of indent, optional trailing spaces) as a setext heading. So appending---after a paragraph turned that existing paragraph into an H2 (===made an H1). The edit changed text that was already in the note, and Basic Memory's section parser then saw a heading nobody wrote.Fix
_joins_into_setext_headinginsrc/basic_memory/services/note_preparation.pyparses the single-newline join with the section parser's markdown-it instance (setext_heading_underlined_atinsrc/basic_memory/markdown/sections.py) and reports whether a setext heading would end on the first line of the text after the join. Only then does_edit_join_separatoradd a blank line. Callers pass the Markdown body that ends at the join explicitly (body_before): the note body without frontmatter for append, the fragment itself when prepending into an existing body. The parser handles block context no line pattern captures:paragraph\n2. itemlazy continuation, open fences and HTML blocks, CRLF/CR line ends,> ---inside quotes, and underlines indented into list items. It is used by:append: existing content, then appended content.prepend(with and without frontmatter): prepended content whose last line is a paragraph, then an existing body that starts with---/===.replace_sectionand the insert operations were checked and need no change. Their boundaries are always next to an ATX heading line, which is never a setext underline and is never claimed as setext heading text. The insert operations already add blank lines around the heading.Why not always add a blank line
The suggested fix was to join every append with a blank line. That was not adopted. The common append is a
- [category] factobservation or a list item under existing ones. A blank line there turns a tight list into a loose list, which changes rendering on every such edit. A## headingdirectly after a paragraph is still a valid ATX heading, so the missing blank line before the next heading afterreplace_sectionis cosmetic and is left alone.Out of scope here, left as a possible follow-up: prepending paragraph text before a body that starts with paragraph text (including a multi-line setext heading, or
2.ordered lists via lazy continuation) still merges into that paragraph. That is the same "join text into the adjacent paragraph" behavior as appending plain text, not the setext underline case.Tests
tests/services/test_entity_service_prepare.pyasserts bytes and parses with markdown-it-py:---appended after a paragraph (with and without a trailing newline) gives\n\n---, renders an<hr />, no setext heading.===appended after a paragraph keeps the paragraph.---after a list item, after a blank line, and plain text after a paragraph are unchanged.paragraph\n2. item+---gets the blank line; an open fence or open<div>+---does not.> paragraph+> ---, and a list-indented underline.---, with and without frontmatter; prepending an ATX heading stays a single newline.Verification
just fast-check: passes (ruff, format, ty).BASIC_MEMORY_TESTMON_SELECT_FLAGS="--import-mode=importlib" just fast-test tests/services/test_entity_service_prepare.py tests/services/test_entity_service.py tests/mcp/test_tool_edit_note.py test-int/mcp/test_edit_note_integration.py tests/api/v2/test_knowledge_router.py tests/services/test_upsert_entity_optimization.py tests/markdown: 657 passed (after the review fixes).🤖 Generated with Claude Code
https://claude.ai/code/session_01APFUk2bjEwMptMQqpRhjea