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(""):]