From edc7b9b63b0d5f2ad8ca5b2bf184ed46c600850f Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 24 May 2026 11:09:17 +0000 Subject: [PATCH] fix: _apply_links now overwrites related_skills regardless of current value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously the `related_skills:` frontmatter was only rewritten when its current value was a placeholder sentinel (`none`, `PLACEHOLDER`, or `[PLACEHOLDER]`). A second `link-ingest` run — e.g. after the Phase 2 updater triggers a re-link — silently left the frontmatter stale while still rewriting the body Related Skills table, making frontmatter and body inconsistent without any error or warning. Fix: replace the placeholder-only regex with `.*` so any existing value is overwritten. Guard with `if joined:` so an empty link list doesn't blank out the field. Adds `test_existing_real_value_overwritten` to pin the correct behaviour. Co-Authored-By: Claude Sonnet 4.6 --- tests/test_link.py | 13 +++++++++++++ tools/skill_generator/link.py | 20 +++++++++++++------- 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/tests/test_link.py b/tests/test_link.py index c18c35e..0af04f8 100644 --- a/tests/test_link.py +++ b/tests/test_link.py @@ -243,6 +243,19 @@ def test_duplicate_targets_deduplicated(self): ) self.assertEqual(fm_line.count("invoice-compare"), 1) + def test_existing_real_value_overwritten(self): + """Regression: a second link-ingest run must overwrite an existing real + related_skills value, not leave it stale. Previously only placeholder + sentinels (none / PLACEHOLDER / [PLACEHOLDER]) were matched, so this + silently skipped the frontmatter update while still rewriting the body + table — leaving frontmatter and body inconsistent.""" + text = _make_skill("file-delivery", related_skills_value="invoice-compare") + links = [{"to": "payment-method", "type": "calls", "reason": "needs payment"}] + result = _apply_links(text, links) + fm_line = next(l for l in result.splitlines() if l.startswith("related_skills:")) + self.assertEqual(fm_line, "related_skills: payment-method", + "existing related_skills value must be replaced, not left stale") + class TestApplyLinksRelatedSkillsTable(unittest.TestCase): def test_table_row_inserted(self): diff --git a/tools/skill_generator/link.py b/tools/skill_generator/link.py index cfde6ec..ab658c9 100644 --- a/tools/skill_generator/link.py +++ b/tools/skill_generator/link.py @@ -118,13 +118,19 @@ def _apply_links(text: str, links: list) -> str: """Update frontmatter `related_skills:` and the body Related Skills table.""" targets = [ln["to"] for ln in links] joined = ", ".join(sorted(set(targets))) - text = re.sub( - r"^related_skills:\s*(?:PLACEHOLDER|none|\[PLACEHOLDER\])\s*$", - f"related_skills: {joined}", - text, - count=1, - flags=re.MULTILINE, - ) + # Update the frontmatter field for any existing value — not just placeholder + # sentinels. Prior to this fix, a second link-ingest run (e.g., after a + # Phase 2 update) left the frontmatter stale while rewriting the body table, + # creating a schema inconsistency. Guard against empty joined so a caller + # with no links doesn't blank out an existing value. + if joined: + text = re.sub( + r"^related_skills:\s*.*$", + f"related_skills: {joined}", + text, + count=1, + flags=re.MULTILINE, + ) table_rows = "\n".join( f"| {ln['to']} | {ln.get('type', 'calls')} | {ln.get('reason', '')} |" for ln in links