fix(edit): consume same-level sibling sections in replace_section (#1012)#1039
fix(edit): consume same-level sibling sections in replace_section (#1012)#1039sanjibani wants to merge 1 commit into
Conversation
…sicmachines-co#1012) The consume run after a target section header used to stop at any '#' heading, which left a same-level sibling in place even when the replacement content re-authored that sibling from scratch. Result: silent content duplication -- the replacement's new '## Description' followed by the original '## Description'. Stop the consume run only when we hit a strictly deeper heading (a subsection). Same-level/higher-level headings that appear directly after the target are now consumed, so replacement content can authoritatively re-define them. A same-level sibling that appears only after one or more preserved subsections is left untouched (it belongs to the next top-level block). Updated two existing tests that asserted the old buggy behavior ('## Another Section' / '## Section 2' preserved when their previous section was replaced). Added regression tests covering both the duplicate-sibling case and the subsection-preservation case.
|
|
![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: 2fb2be544b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| 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 |
There was a problem hiding this comment.
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 👍 / 👎.
|
recheck |
2 participants
Closes #1012.
The consume run after the target section header used to stop at any
#heading, which left a directly-following same-level sibling in place even when the replacement content re-authored that sibling from scratch. The reproduction in the issue shows the result: a replacement that includes a new## Descriptionis followed by the original## Description(and its body) below, silently duplicating content.The fix stops the consume run only when we hit a strictly deeper heading (a subsection). Same-level and higher-level headings that appear directly after the target are now consumed, so replacement content can authoritatively re-define them. A same-level sibling that appears only after one or more preserved subsections is left untouched — it belongs to the next top-level block, not to the section being replaced.
Two existing tests (
test_edit_entity_replace_section,test_edit_entity_replace_section_strips_duplicate_header) asserted the old buggy behavior (## Section 2/## Another Sectionpreserved when their previous section was replaced). Updated to assert the new contract. Added two new regression tests:test_replace_section_consumes_same_level_siblings: directly-following## Descriptionis consumed when## Observationsis replaced with content that re-authors it.test_replace_section_preserves_deeper_subsections:### Detail A/### Detail Bunder the target are still preserved.Verified locally: new tests fail without the source change and pass with it. Full
replace_sectiontest set (11 tests) passes.