fix(skills): make evolved skill text optimizer-visible and validate the full skill file

evolution/core/constraints.py

@@ -52,6 +52,31 @@ def validate_all(
 
         return results
 
+    def validate_skill(
+        self,
+        frontmatter: str,
+        body: str,
+        baseline_body: Optional[str] = None,
+    ) -> list[ConstraintResult]:
+        """Validate a skill from its parts, reassembling before structural checks.
+
+        Structural validation needs the *whole file* (frontmatter + body), while
+        size and growth are most meaningful against the body — the only part
+        evolution actually rewrites. Callers pass the parts and this method
+        applies each constraint to the right text, so a bare body can no longer
+        be handed to the frontmatter check by mistake.
+        """
+        from evolution.skills.skill_module import reassemble_skill
+
+        results = [
+            self._check_size(body, "skill"),
+            self._check_non_empty(body),
+            self._check_skill_structure(reassemble_skill(frontmatter, body)),
+        ]
+        if baseline_body is not None:
+            results.insert(1, self._check_growth(body, baseline_body, "skill"))
+        return results
+
     def run_test_suite(self, hermes_repo: Path) -> ConstraintResult:
         """Run the full hermes-agent test suite. Must pass 100%."""
         try:

evolution/skills/evolve_skill.py

@@ -118,7 +118,9 @@ def evolve(
     # ── 3. Validate constraints on baseline ─────────────────────────────
     console.print(f"\n[bold]Validating baseline constraints[/bold]")
     validator = ConstraintValidator(config)
-    baseline_constraints = validator.validate_all(skill["body"], "skill")
+    baseline_constraints = validator.validate_skill(
+        frontmatter=skill["frontmatter"], body=skill["body"]
+    )
     all_pass = True
     for c in baseline_constraints:
         icon = "✓" if c.passed else "✗"
@@ -179,13 +181,29 @@ def evolve(
     console.print(f"\n  Optimization completed in {elapsed:.1f}s")
 
     # ── 6. Extract evolved skill text ───────────────────────────────────
-    # The optimized module's instructions contain the evolved skill text
+    # The optimized module's signature instructions ARE the evolved skill text.
     evolved_body = optimized_module.skill_text
     evolved_full = reassemble_skill(skill["frontmatter"], evolved_body)
 
+    # Guard: if the optimizer produced no textual change, there is nothing to
+    # evaluate and any holdout delta would be measuring noise, not evolution.
+    if evolved_body.strip() == skill["body"].strip():
+        console.print(
+            "\n[red]✗ Optimizer returned an unchanged skill body — aborting.[/red]"
+        )
+        console.print(
+            "  No evolution occurred, so no improvement can be reported.\n"
+            "  Check that the optimizer ran and that skill text is optimizer-visible state."
+        )
+        return
+
     # ── 7. Validate evolved skill ───────────────────────────────────────
     console.print(f"\n[bold]Validating evolved skill[/bold]")
-    evolved_constraints = validator.validate_all(evolved_body, "skill", baseline_text=skill["body"])
+    evolved_constraints = validator.validate_skill(
+        frontmatter=skill["frontmatter"],
+        body=evolved_body,
+        baseline_body=skill["body"],
+    )
     all_pass = True
     for c in evolved_constraints:
         icon = "✓" if c.passed else "✗"

evolution/skills/skill_module.py

@@ -84,11 +84,13 @@ def find_skill(skill_name: str, hermes_agent_path: Path) -> Optional[Path]:
 class SkillModule(dspy.Module):
     """A DSPy module that wraps a skill file for optimization.
 
-    The skill text (body) is the parameter that GEPA optimizes.
-    On each forward pass, the module:
-    1. Uses the skill text as instructions
-    2. Processes the task input
-    3. Returns the agent's response
+    The skill text is held as the predictor's *signature instructions*, which is
+    the state DSPy optimizers (GEPA, MIPROv2) actually rewrite. Reading
+    `skill_text` back after `compile()` therefore returns the evolved text.
+
+    Holding it in a plain Python attribute instead — or passing it as an
+    InputField — leaves it invisible to every optimizer, so the "evolved" skill
+    written to disk is byte-identical to the baseline.
     """
 
     class TaskWithSkill(dspy.Signature):
@@ -97,20 +99,21 @@ class TaskWithSkill(dspy.Signature):
         You are an AI agent following specific skill instructions to complete a task.
         Read the skill instructions carefully and follow the procedure described.
         """
-        skill_instructions: str = dspy.InputField(desc="The skill instructions to follow")
         task_input: str = dspy.InputField(desc="The task to complete")
         output: str = dspy.OutputField(desc="Your response following the skill instructions")
 
     def __init__(self, skill_text: str):
         super().__init__()
-        self.skill_text = skill_text
-        self.predictor = dspy.ChainOfThought(self.TaskWithSkill)
+        signature = self.TaskWithSkill.with_instructions(skill_text)
+        self.predictor = dspy.ChainOfThought(signature)
+
+    @property
+    def skill_text(self) -> str:
+        """The current skill text, read from optimizer-owned state."""
+        return self.predictor.predict.signature.instructions
 
     def forward(self, task_input: str) -> dspy.Prediction:
-        result = self.predictor(
-            skill_instructions=self.skill_text,
-            task_input=task_input,
-        )
+        result = self.predictor(task_input=task_input)
         return dspy.Prediction(output=result.output)
 
 

tests/skills/test_evolution_e2e.py

@@ -0,0 +1,79 @@
+"""End-to-end proof that the write-back path works, using a fake optimizer.
+
+No network, no API keys: a stub optimizer stands in for GEPA and rewrites the
+signature instructions the way a real one does. This exercises the whole
+extract -> guard -> validate -> reassemble chain.
+"""
+
+import pytest
+
+from evolution.skills.skill_module import SkillModule, load_skill, reassemble_skill
+
+
+SAMPLE_SKILL = """---
+name: demo-skill
+description: Demo skill
+---
+
+# Demo
+
+## Procedure
+1. Baseline step
+"""
+
+
+class FakeOptimizer:
+    """Stands in for dspy.GEPA: rewrites predictor signature instructions."""
+
+    def __init__(self, new_instructions: str):
+        self.new_instructions = new_instructions
+
+    def compile(self, module, **_kwargs):
+        for _, predictor in module.named_predictors():
+            predictor.signature = predictor.signature.with_instructions(
+                self.new_instructions
+            )
+        return module
+
+
+def test_optimizer_rewrite_survives_extraction_and_reassembly(tmp_path):
+    skill_file = tmp_path / "SKILL.md"
+    skill_file.write_text(SAMPLE_SKILL)
+    skill = load_skill(skill_file)
+
+    module = SkillModule(skill["body"])
+    evolved_text = "# Demo\n\n## Procedure\n1. Evolved step\n2. Extra verification step"
+
+    optimized = FakeOptimizer(evolved_text).compile(module)
+
+    evolved_body = optimized.skill_text
+    assert evolved_body != skill["body"], "optimizer rewrite was lost"
+    assert "Evolved step" in evolved_body
+
+    evolved_full = reassemble_skill(skill["frontmatter"], evolved_body)
+    assert evolved_full.startswith("---")
+    assert "name: demo-skill" in evolved_full
+    assert "Evolved step" in evolved_full
+
+
+def test_noop_optimizer_is_detectable(tmp_path):
+    """SABOTAGE CHECK: an optimizer that changes nothing must be detectable.
+
+    This is the condition the pipeline's no-op guard aborts on. If this test
+    ever passes trivially, the guard is not measuring anything.
+    """
+    skill_file = tmp_path / "SKILL.md"
+    skill_file.write_text(SAMPLE_SKILL)
+    skill = load_skill(skill_file)
+
+    module = SkillModule(skill["body"])
+
+    class NoOpOptimizer:
+        def compile(self, module, **_kwargs):
+            return module  # the original bug's behaviour
+
+    optimized = NoOpOptimizer().compile(module)
+
+    assert optimized.skill_text.strip() == skill["body"].strip(), (
+        "no-op optimizer somehow changed the text; guard would not fire"
+    )

tests/skills/test_evolution_writeback.py

@@ -0,0 +1,135 @@
+"""Regression tests for the Phase 1 skill-evolution write-back path.
+
+Two defects made `evolve_skill` unable to ever emit a genuinely evolved skill:
+
+1. `SkillModule` carried the skill text in a plain Python attribute and passed it
+   to the signature as an InputField, so no DSPy optimizer could rewrite it.
+   `evolve_skill.py` then read that same untouched attribute back as the
+   "evolved" body.
+
+2. The constraint validator was handed `skill["body"]`, but `_check_skill_structure`
+   requires YAML frontmatter — which `load_skill` has already stripped out of the
+   body. Every evolved candidate therefore failed structural validation and was
+   rejected before the holdout comparison ran.
+
+These tests pin both behaviours so they cannot silently regress.
+"""
+
+import pytest
+
+from evolution.core.config import EvolutionConfig
+from evolution.core.constraints import ConstraintValidator
+from evolution.skills.skill_module import (
+    SkillModule,
+    load_skill,
+    reassemble_skill,
+)
+
+
+SAMPLE_SKILL = """---
+name: test-skill
+description: A skill for testing things
+version: 1.0.0
+---
+
+# Test Skill — Testing Things
+
+## Procedure
+1. First, do the thing
+2. Then, verify it worked
+"""
+
+
+@pytest.fixture
+def config(tmp_path):
+    return EvolutionConfig(hermes_agent_path=tmp_path)
+
+
+class TestSkillTextIsOptimizable:
+    """Defect 1: the skill text must be optimizer-visible state, not a bare attribute."""
+
+    def test_skill_text_is_exposed_as_predictor_instructions(self):
+        module = SkillModule("# Baseline procedure\n1. Do the thing")
+
+        instructions = [
+            predictor.signature.instructions
+            for _, predictor in module.named_predictors()
+        ]
+
+        assert instructions, "module exposes no predictors for the optimizer to touch"
+        assert any(
+            "Baseline procedure" in text for text in instructions
+        ), "skill text is not reachable through any predictor's signature instructions"
+
+    def test_evolved_instructions_are_read_back(self):
+        """What the optimizer rewrites must be what we serialize."""
+        module = SkillModule("# Baseline procedure")
+
+        # Simulate what a DSPy optimizer does: rewrite signature instructions.
+        for _, predictor in module.named_predictors():
+            predictor.signature = predictor.signature.with_instructions(
+                "# EVOLVED procedure\n1. Do the better thing"
+            )
+
+        assert "EVOLVED procedure" in module.skill_text
+        assert "Baseline procedure" not in module.skill_text
+
+
+class TestStructureValidationTarget:
+    """Defect 2: structural validation must run against the reassembled file."""
+
+    def test_bare_body_has_no_frontmatter(self, tmp_path):
+        skill_file = tmp_path / "SKILL.md"
+        skill_file.write_text(SAMPLE_SKILL)
+        skill = load_skill(skill_file)
+
+        # Precondition for the bug: load_skill strips frontmatter out of the body.
+        assert not skill["body"].lstrip().startswith("---")
+
+    def test_bare_body_fails_structure_check(self, tmp_path, config):
+        """Pins WHY passing the body was wrong — it can never pass."""
+        skill_file = tmp_path / "SKILL.md"
+        skill_file.write_text(SAMPLE_SKILL)
+        skill = load_skill(skill_file)
+
+        result = ConstraintValidator(config)._check_skill_structure(skill["body"])
+        assert not result.passed
+
+    def test_reassembled_skill_passes_structure_check(self, tmp_path, config):
+        skill_file = tmp_path / "SKILL.md"
+        skill_file.write_text(SAMPLE_SKILL)
+        skill = load_skill(skill_file)
+
+        evolved_full = reassemble_skill(skill["frontmatter"], skill["body"])
+        result = ConstraintValidator(config)._check_skill_structure(evolved_full)
+
+        assert result.passed, result.message
+
+    def test_validate_skill_reassembles_internally(self, tmp_path, config):
+        """The call site passes frontmatter + body; the validator owns assembly.
+
+        This is what makes the original defect unrepresentable: callers can no
+        longer hand a bare body to skill validation by mistake.
+        """
+        skill_file = tmp_path / "SKILL.md"
+        skill_file.write_text(SAMPLE_SKILL)
+        skill = load_skill(skill_file)
+
+        results = ConstraintValidator(config).validate_skill(
+            frontmatter=skill["frontmatter"],
+            body=skill["body"],
+            baseline_body=skill["body"],
+        )
+
+        failed = [r for r in results if not r.passed]
+        assert not failed, f"unexpected failures: {[(r.constraint_name, r.message) for r in failed]}"
+
+    def test_validate_skill_still_catches_broken_frontmatter(self, tmp_path, config):
+        """Reassembly must not paper over genuinely malformed frontmatter."""
+        results = ConstraintValidator(config).validate_skill(
+            frontmatter="version: 1.0.0",  # no name, no description
+            body="# Body",
+        )
+
+        structure = [r for r in results if r.constraint_name == "skill_structure"]
+        assert structure and not structure[0].passed