fix(agent): resolve skill section headings that differ only by case (#1797)
Signed-off-by: Lanre Shittu <136805224+Shizoqua@users.noreply.github.com>
This commit is contained in:
1 parent
20f2869ccc
commit
cf19d98b96
2 files changed
+62
-1
No files matched your search
@@ -342,7 +342,8 @@ def find_sections(sections: List[SkillSection], wanted: str) -> List[SkillSectio
|
||||
parts = [part for part in parts if part]
|
||||
if not parts:
|
||||
return []
|
||||
key = _normalize_title(parts[-1])
|
||||
leaf_exact = " ".join(parts[-1].replace("#", " ").split())
|
||||
key = leaf_exact.casefold()
|
||||
wanted_ancestors = [_normalize_title(part) for part in parts[:-1]]
|
||||
if not key:
|
||||
return []
|
||||
@@ -355,6 +356,13 @@ def find_sections(sections: List[SkillSection], wanted: str) -> List[SkillSectio
|
||||
return not remaining
|
||||
|
||||
for predicate in (
|
||||
# A case-exact match comes first so that submitting the exact
|
||||
# qualified path a prior ambiguous-match error suggested actually
|
||||
# disambiguates, instead of casefolding straight back into the same
|
||||
# tie -- two headings differing only by case (e.g. "Setup"/"setup")
|
||||
# would otherwise report as ambiguous no matter which suggested
|
||||
# path the caller resubmits.
|
||||
lambda title: " ".join(title.replace("#", " ").split()) == leaf_exact,
|
||||
lambda title: _normalize_title(title) == key,
|
||||
lambda title: _normalize_title(title).startswith(key),
|
||||
):
|
||||
|
||||
@@ -799,6 +799,59 @@ class TestRepeatedHeadingsAreAddressable:
|
||||
assert "- Mode 1 (" in payload["outline"]
|
||||
|
||||
|
||||
_CASE_REPEATED_TITLE_SKILL = (
|
||||
"---\nname: case-repeat-skill\ndescription: Repeats a heading in different case.\n---\n"
|
||||
"# Case Repeat Skill\n\n"
|
||||
"## Setup\n\nfirst setup body\n\n"
|
||||
"## setup\n\nsecond setup body\n"
|
||||
)
|
||||
|
||||
|
||||
class TestCaseDifferingHeadingsAreAddressable:
|
||||
"""Two headings differing only by case must resolve, not loop forever.
|
||||
|
||||
The ambiguous-match error suggests a qualified path for each match,
|
||||
built from the section's own stored title. When the only difference
|
||||
between two matches is case, casefolded comparison used for the
|
||||
suggested path's own lookup threw that distinction away, so
|
||||
resubmitting exactly what the error suggested reproduced the same
|
||||
ambiguous error with no way out.
|
||||
"""
|
||||
|
||||
@pytest.fixture()
|
||||
def case_repeat(self, tmp_path: Path) -> LoadSkillTool:
|
||||
directory = tmp_path / "case-repeat-skill"
|
||||
directory.mkdir()
|
||||
(directory / "SKILL.md").write_text(_CASE_REPEATED_TITLE_SKILL, encoding="utf-8")
|
||||
return LoadSkillTool(SkillsLoader(skills_dir=tmp_path, user_skills_dir=tmp_path / "absent"))
|
||||
|
||||
def test_the_suggested_qualified_path_actually_resolves(self, case_repeat: LoadSkillTool) -> None:
|
||||
bare = json.loads(case_repeat.execute(name="case-repeat-skill", section="SETUP"))
|
||||
assert bare["status"] == "error"
|
||||
assert "Case Repeat Skill > Setup" in bare["content"]
|
||||
assert "Case Repeat Skill > setup" in bare["content"]
|
||||
|
||||
first = json.loads(
|
||||
case_repeat.execute(name="case-repeat-skill", section="Case Repeat Skill > Setup")
|
||||
)
|
||||
second = json.loads(
|
||||
case_repeat.execute(name="case-repeat-skill", section="Case Repeat Skill > setup")
|
||||
)
|
||||
|
||||
assert "first setup body" in first["content"]
|
||||
assert "second setup body" not in first["content"]
|
||||
assert "second setup body" in second["content"]
|
||||
assert "first setup body" not in second["content"]
|
||||
|
||||
def test_an_exact_case_bare_title_resolves_without_qualifying(
|
||||
self, case_repeat: LoadSkillTool
|
||||
) -> None:
|
||||
payload = json.loads(case_repeat.execute(name="case-repeat-skill", section="setup"))
|
||||
|
||||
assert payload["status"] == "ok"
|
||||
assert "second setup body" in payload["content"]
|
||||
|
||||
|
||||
class TestEverySectionOfEveryBundledSkillRoundTrips:
|
||||
"""Whatever the outline offers, asking for it must return exactly that span."""
|
||||
|
||||
|
||||
Reference in new issue
Block a user