From 42938741e987a5ea98a838e4b127e000eb1bd8e8 Mon Sep 17 00:00:00 2001 From: 1AbdullahButt <168034895+1AbdullahButt@users.noreply.github.com> Date: Thu, 12 Nov 2026 17:01:16 +0500 Subject: [PATCH] fix(optimizer): insert_after a target that ends in a newline right after it insert_after finds the end of the target's line by searching for the next newline from the end of the target. When the target already ends with "\n", that search found the end of the following line, so the content landed one line too late: under the next heading, or inside a protected region whose start marker follows directly. Insert at the end of the target when it already ends its line. Fixes #407 Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 3 ++ skillopt/optimizer/skill.py | 9 +++-- tests/test_insert_after_newline_target.py | 44 +++++++++++++++++++++++ 3 files changed, 54 insertions(+), 2 deletions(-) create mode 100644 tests/test_insert_after_newline_target.py diff --git a/CHANGELOG.md b/CHANGELOG.md index ef83812d..64d1e776 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -129,6 +129,9 @@ All notable changes to SkillOpt are documented here. This project adheres to value is rejected with a clear error. ### Fixed +- `insert_after` with a target that ends in a newline now inserts right + after that line instead of after the next one, which could file the new + text under the following heading or inside a protected region (#407). - Close four ways invalid evidence could reach the SkillOpt-Sleep gate: recall no longer brings a content twin of tonight's held-out task into training, a measured tool route no longer credits a self-reported `TOOL_CALL:` marker, diff --git a/skillopt/optimizer/skill.py b/skillopt/optimizer/skill.py index 69229f94..26ad5625 100644 --- a/skillopt/optimizer/skill.py +++ b/skillopt/optimizer/skill.py @@ -119,8 +119,13 @@ def _apply_edit_with_report(skill: str, edit: EditType | dict) -> tuple[str, dic report["status"] = "applied_insert_after_fallback_append" return skill.rstrip() + "\n\n" + content + "\n", report idx = skill.index(target) + len(target) - newline = skill.find("\n", idx) - insert_at = newline + 1 if newline != -1 else len(skill) + if target.endswith("\n"): + # The target already ends its line: insert right here, not after + # the line that follows it. + insert_at = idx + else: + newline = skill.find("\n", idx) + insert_at = newline + 1 if newline != -1 else len(skill) report["status"] = "applied_insert_after" return skill[:insert_at] + "\n" + content + "\n" + skill[insert_at:], report diff --git a/tests/test_insert_after_newline_target.py b/tests/test_insert_after_newline_target.py new file mode 100644 index 00000000..a5503ed0 --- /dev/null +++ b/tests/test_insert_after_newline_target.py @@ -0,0 +1,44 @@ +"""Regression: ``insert_after`` with a target that ends in a newline. + +The insertion point is the end of the target's line. When the target already +ends with ``\\n``, the search for the next newline found the end of the +*following* line, so the content landed one line too late: under the next +heading, or inside a protected region whose start marker follows directly. +""" +from __future__ import annotations + +from skillopt.optimizer.skill import apply_patch_with_report + + +def _insert_after(skill: str, target: str) -> tuple[str, str]: + new, rep = apply_patch_with_report( + skill, + {"edits": [{"op": "insert_after", "target": target, "content": "- Cite the source."}]}, + ) + return new, rep[0]["status"] + + +def test_target_ending_in_newline_inserts_right_after_it() -> None: + skill = "# QA Skill\n\n## Rules\n## Output format\n- One line.\n" + new, status = _insert_after(skill, "## Rules\n") + assert status == "applied_insert_after" + assert new == "# QA Skill\n\n## Rules\n\n- Cite the source.\n## Output format\n- One line.\n" + + +def test_target_ending_in_newline_matches_target_without_it() -> None: + skill = "# QA Skill\n\n## Rules\n## Output format\n- One line.\n" + with_newline, _ = _insert_after(skill, "## Rules\n") + without_newline, _ = _insert_after(skill, "## Rules") + assert with_newline == without_newline + + +def test_insert_does_not_land_inside_following_protected_region() -> None: + skill = ( + "# QA Skill\n- Answer briefly.\n" + "\nPrefer exact spans.\n\n" + ) + new, status = _insert_after(skill, "- Answer briefly.\n") + assert status == "applied_insert_after" + start = new.index("") + assert new.index("- Cite the source.") < start + assert new[start:] == skill[skill.index(""):]