Skip to content

Commit f1b2b1e

Browse files
Brian KrafftCopilot
andcommitted
feat(5.6): extract formatting helpers, complete evolver.py facade
P5a capstone: evolver.py is now a pure delegation shell. evolution/formatting.py (~85 lines): - format_skill_dir_content: multi-file skill directory → prompt text - format_analysis_context: execution analyses → prompt context block evolver.py: 1,498 → 303 lines (80% reduction, zero implementation logic) All methods are either __init__, thin delegates, or staticmethod bindings. Unused imports removed (collect_skill_snapshot, SKILL_FILENAME, _ANALYSIS_CONTEXT_MAX). 15 new tests; 1,577 total pass, 127 skipped. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 3eb4529 commit f1b2b1e

3 files changed

Lines changed: 282 additions & 68 deletions

File tree

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
"""Formatting helpers for evolution prompts.
2+
3+
Pure functions that transform skill snapshots and execution analyses
4+
into text blocks suitable for LLM prompt inclusion.
5+
"""
6+
7+
from __future__ import annotations
8+
9+
from typing import List
10+
11+
from openspace.utils.logging import Logger
12+
13+
from ..patch import SKILL_FILENAME, collect_skill_snapshot
14+
from ..types import ExecutionAnalysis
15+
from .triggers import _ANALYSIS_CONTEXT_MAX
16+
17+
logger = Logger.get_logger("openspace.skill_engine.evolver")
18+
19+
20+
def format_skill_dir_content(skill_dir) -> str:
21+
"""Format all text files in a skill directory for prompt inclusion.
22+
23+
Returns a multi-file listing if there are auxiliary files beyond
24+
SKILL.md, or just the SKILL.md content for single-file skills.
25+
"""
26+
files = collect_skill_snapshot(skill_dir)
27+
if not files:
28+
return ""
29+
30+
# Single-file skill: return just the content
31+
if len(files) == 1 and SKILL_FILENAME in files:
32+
return files[SKILL_FILENAME]
33+
34+
# Multi-file: format as directory listing
35+
parts: list[str] = []
36+
# SKILL.md first
37+
if SKILL_FILENAME in files:
38+
parts.append(f"### File: {SKILL_FILENAME}\n```markdown\n{files[SKILL_FILENAME]}\n```")
39+
for name, content in sorted(files.items()):
40+
if name == SKILL_FILENAME:
41+
continue
42+
parts.append(f"### File: {name}\n```\n{content}\n```")
43+
44+
return "\n\n".join(parts)
45+
46+
47+
def format_analysis_context(analyses: List[ExecutionAnalysis]) -> str:
48+
"""Format recent analyses into a concise context block for prompts."""
49+
_ANALYSIS_NOTE_MAX_CHARS = 500 # Per-analysis note truncation
50+
51+
if not analyses:
52+
return "(no execution history available)"
53+
54+
parts: List[str] = []
55+
for a in analyses[:_ANALYSIS_CONTEXT_MAX]:
56+
completed = "completed" if a.task_completed else "failed"
57+
58+
# Per-skill notes
59+
skill_notes = []
60+
for j in a.skill_judgments:
61+
applied = "applied" if j.skill_applied else "NOT applied"
62+
note = f" - {j.skill_id}: {applied}"
63+
if j.note:
64+
note += f" — {j.note[:_ANALYSIS_NOTE_MAX_CHARS]}"
65+
skill_notes.append(note)
66+
67+
# Tool issues
68+
tool_lines = []
69+
for issue in a.tool_issues[:3]:
70+
tool_lines.append(f" - {issue[:200]}")
71+
72+
block = f"### Task: {a.task_id} ({completed})\n"
73+
if a.execution_note:
74+
block += f"{a.execution_note[:_ANALYSIS_NOTE_MAX_CHARS]}\n"
75+
if skill_notes:
76+
block += "Skills:\n" + "\n".join(skill_notes) + "\n"
77+
if tool_lines:
78+
block += "Tool issues:\n" + "\n".join(tool_lines) + "\n"
79+
parts.append(block)
80+
81+
return "\n".join(parts)

openspace/skill_engine/evolver.py

Lines changed: 7 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,10 @@
3636
llm_confirm_evolution as _llm_confirm_evolution_impl,
3737
parse_confirmation as _parse_confirmation_impl,
3838
)
39+
from .evolution.formatting import (
40+
format_analysis_context as _format_analysis_context_impl,
41+
format_skill_dir_content as _format_skill_dir_content_impl,
42+
)
3943
from .evolution.loop import (
4044
EVOLUTION_COMPLETE,
4145
EVOLUTION_FAILED,
@@ -51,19 +55,14 @@
5155
evolve_fix as _evolve_fix_impl,
5256
)
5357
from .evolution.triggers import (
54-
_ANALYSIS_CONTEXT_MAX,
5558
build_context_from_analysis as _build_context_from_analysis_impl,
5659
diagnose_skill_health as _diagnose_skill_health_impl,
5760
load_skill_content as _load_skill_content_impl,
5861
process_analysis as _process_analysis_impl,
5962
process_metric_check as _process_metric_check_impl,
6063
process_tool_degradation as _process_tool_degradation_impl,
6164
)
62-
from .patch import (
63-
SKILL_FILENAME,
64-
SkillEditResult,
65-
collect_skill_snapshot,
66-
)
65+
from .patch import SkillEditResult
6766
from .store import SkillStore
6867
from .types import (
6968
EvolutionSuggestion,
@@ -297,68 +296,8 @@ def _load_skill_content(self, record: SkillRecord) -> str:
297296
"""Load SKILL.md content from disk via registry or direct read."""
298297
return _load_skill_content_impl(self, record)
299298

300-
@staticmethod
301-
def _format_skill_dir_content(skill_dir: Path) -> str:
302-
"""Format all text files in a skill directory for prompt inclusion.
299+
_format_skill_dir_content = staticmethod(_format_skill_dir_content_impl)
303300

304-
Returns a multi-file listing if there are auxiliary files beyond
305-
SKILL.md, or just the SKILL.md content for single-file skills.
306-
"""
307-
files = collect_skill_snapshot(skill_dir)
308-
if not files:
309-
return ""
310-
311-
# Single-file skill: return just the content
312-
if len(files) == 1 and SKILL_FILENAME in files:
313-
return files[SKILL_FILENAME]
314-
315-
# Multi-file: format as directory listing
316-
parts: list[str] = []
317-
# SKILL.md first
318-
if SKILL_FILENAME in files:
319-
parts.append(f"### File: {SKILL_FILENAME}\n```markdown\n{files[SKILL_FILENAME]}\n```")
320-
for name, content in sorted(files.items()):
321-
if name == SKILL_FILENAME:
322-
continue
323-
parts.append(f"### File: {name}\n```\n{content}\n```")
324-
325-
return "\n\n".join(parts)
326-
327-
@staticmethod
328-
def _format_analysis_context(analyses: List[ExecutionAnalysis]) -> str:
329-
"""Format recent analyses into a concise context block for prompts."""
330-
_ANALYSIS_NOTE_MAX_CHARS = 500 # Per-analysis note truncation
331-
332-
if not analyses:
333-
return "(no execution history available)"
334-
335-
parts: List[str] = []
336-
for a in analyses[:_ANALYSIS_CONTEXT_MAX]:
337-
completed = "completed" if a.task_completed else "failed"
338-
339-
# Per-skill notes
340-
skill_notes = []
341-
for j in a.skill_judgments:
342-
applied = "applied" if j.skill_applied else "NOT applied"
343-
note = f" - {j.skill_id}: {applied}"
344-
if j.note:
345-
note += f" — {j.note[:_ANALYSIS_NOTE_MAX_CHARS]}"
346-
skill_notes.append(note)
347-
348-
# Tool issues
349-
tool_lines = []
350-
for issue in a.tool_issues[:3]:
351-
tool_lines.append(f" - {issue[:200]}")
352-
353-
block = f"### Task: {a.task_id} ({completed})\n"
354-
if a.execution_note:
355-
block += f"{a.execution_note[:_ANALYSIS_NOTE_MAX_CHARS]}\n"
356-
if skill_notes:
357-
block += "Skills:\n" + "\n".join(skill_notes) + "\n"
358-
if tool_lines:
359-
block += "Tool issues:\n" + "\n".join(tool_lines) + "\n"
360-
parts.append(block)
361-
362-
return "\n".join(parts)
301+
_format_analysis_context = staticmethod(_format_analysis_context_impl)
363302

364303
_diagnose_skill_health = staticmethod(_diagnose_skill_health_impl)

tests/test_evolution_formatting.py

Lines changed: 194 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,194 @@
1+
"""Tests for openspace.skill_engine.evolution.formatting (Epic 5.6).
2+
3+
Verifies:
4+
1. format_skill_dir_content — single-file, multi-file, empty
5+
2. format_analysis_context — with analyses, empty, truncation
6+
3. Backward compat: SkillEvolver delegates to formatting functions
7+
4. Logger namespace
8+
"""
9+
10+
from __future__ import annotations
11+
12+
from pathlib import Path
13+
from unittest.mock import MagicMock, patch
14+
15+
import pytest
16+
17+
from openspace.skill_engine.evolution.formatting import (
18+
format_analysis_context,
19+
format_skill_dir_content,
20+
)
21+
22+
23+
# ---------------------------------------------------------------------------
24+
# format_skill_dir_content
25+
# ---------------------------------------------------------------------------
26+
27+
class TestFormatSkillDirContent:
28+
def test_empty_dir(self):
29+
"""Empty directory → empty string."""
30+
with patch("openspace.skill_engine.evolution.formatting.collect_skill_snapshot", return_value={}):
31+
result = format_skill_dir_content(Path("/fake"))
32+
assert result == ""
33+
34+
def test_single_file_skill(self):
35+
"""Single SKILL.md → return content directly (no formatting)."""
36+
with patch(
37+
"openspace.skill_engine.evolution.formatting.collect_skill_snapshot",
38+
return_value={"SKILL.md": "---\nname: test\n---\ncontent"},
39+
):
40+
result = format_skill_dir_content(Path("/fake"))
41+
assert result == "---\nname: test\n---\ncontent"
42+
43+
def test_multi_file_skill(self):
44+
"""Multiple files → formatted listing with SKILL.md first."""
45+
files = {
46+
"SKILL.md": "skill content",
47+
"helper.py": "def foo(): pass",
48+
"README.md": "readme text",
49+
}
50+
with patch(
51+
"openspace.skill_engine.evolution.formatting.collect_skill_snapshot",
52+
return_value=files,
53+
):
54+
result = format_skill_dir_content(Path("/fake"))
55+
56+
# SKILL.md appears first
57+
assert result.index("### File: SKILL.md") < result.index("### File: README.md")
58+
assert result.index("### File: SKILL.md") < result.index("### File: helper.py")
59+
# All files present
60+
assert "skill content" in result
61+
assert "def foo(): pass" in result
62+
assert "readme text" in result
63+
64+
def test_multi_file_without_skill_md(self):
65+
"""Multi-file with no SKILL.md → still formats all files."""
66+
files = {"helper.py": "code", "config.yaml": "key: val"}
67+
with patch(
68+
"openspace.skill_engine.evolution.formatting.collect_skill_snapshot",
69+
return_value=files,
70+
):
71+
result = format_skill_dir_content(Path("/fake"))
72+
assert "### File: config.yaml" in result
73+
assert "### File: helper.py" in result
74+
75+
76+
# ---------------------------------------------------------------------------
77+
# format_analysis_context
78+
# ---------------------------------------------------------------------------
79+
80+
class _FakeSkillJudgment:
81+
def __init__(self, skill_id, applied=True, note=""):
82+
self.skill_id = skill_id
83+
self.skill_applied = applied
84+
self.note = note
85+
86+
87+
class _FakeAnalysis:
88+
def __init__(self, task_id, completed=True, note="", judgments=None, tool_issues=None):
89+
self.task_id = task_id
90+
self.task_completed = completed
91+
self.execution_note = note
92+
self.skill_judgments = judgments or []
93+
self.tool_issues = tool_issues or []
94+
95+
96+
class TestFormatAnalysisContext:
97+
def test_empty_analyses(self):
98+
"""No analyses → fallback message."""
99+
result = format_analysis_context([])
100+
assert "no execution history" in result.lower()
101+
102+
def test_single_analysis(self):
103+
"""Single analysis → formatted block."""
104+
analysis = _FakeAnalysis(
105+
task_id="task-1",
106+
completed=True,
107+
note="Skill worked well",
108+
judgments=[_FakeSkillJudgment("skill-a", applied=True, note="Good result")],
109+
)
110+
result = format_analysis_context([analysis])
111+
assert "task-1" in result
112+
assert "completed" in result
113+
assert "skill-a" in result
114+
assert "applied" in result
115+
assert "Good result" in result
116+
117+
def test_failed_task(self):
118+
"""Failed task shows 'failed' label."""
119+
analysis = _FakeAnalysis(task_id="task-2", completed=False)
120+
result = format_analysis_context([analysis])
121+
assert "failed" in result
122+
123+
def test_tool_issues_included(self):
124+
"""Tool issues appear in output."""
125+
analysis = _FakeAnalysis(
126+
task_id="task-3",
127+
tool_issues=["Tool X timed out", "Tool Y returned error"],
128+
)
129+
result = format_analysis_context([analysis])
130+
assert "Tool X timed out" in result
131+
assert "Tool Y returned error" in result
132+
133+
def test_note_truncation(self):
134+
"""Execution notes are truncated to 500 chars."""
135+
long_note = "x" * 1000
136+
analysis = _FakeAnalysis(task_id="task-4", note=long_note)
137+
result = format_analysis_context([analysis])
138+
# The note should be truncated — full 1000 chars should NOT appear
139+
assert "x" * 501 not in result
140+
141+
def test_not_applied_skill(self):
142+
"""Skill not applied shows 'NOT applied'."""
143+
analysis = _FakeAnalysis(
144+
task_id="task-5",
145+
judgments=[_FakeSkillJudgment("skill-b", applied=False, note="Didn't match")],
146+
)
147+
result = format_analysis_context([analysis])
148+
assert "NOT applied" in result
149+
150+
151+
# ---------------------------------------------------------------------------
152+
# Backward compat
153+
# ---------------------------------------------------------------------------
154+
155+
class TestDelegationSeam:
156+
def test_format_skill_dir_is_staticmethod(self):
157+
from openspace.skill_engine.evolver import SkillEvolver
158+
assert isinstance(
159+
SkillEvolver.__dict__["_format_skill_dir_content"],
160+
staticmethod,
161+
)
162+
163+
def test_format_analysis_is_staticmethod(self):
164+
from openspace.skill_engine.evolver import SkillEvolver
165+
assert isinstance(
166+
SkillEvolver.__dict__["_format_analysis_context"],
167+
staticmethod,
168+
)
169+
170+
def test_format_skill_dir_callable(self):
171+
from openspace.skill_engine.evolver import SkillEvolver
172+
with patch(
173+
"openspace.skill_engine.evolution.formatting.collect_skill_snapshot",
174+
return_value={"SKILL.md": "test"},
175+
):
176+
result = SkillEvolver._format_skill_dir_content(Path("/fake"))
177+
assert result == "test"
178+
179+
180+
# ---------------------------------------------------------------------------
181+
# Constants / logger
182+
# ---------------------------------------------------------------------------
183+
184+
class TestMeta:
185+
def test_logger_uses_evolver_namespace(self):
186+
from openspace.skill_engine.evolution.formatting import logger
187+
assert "evolver" in logger.name
188+
189+
def test_module_size(self):
190+
"""formatting.py should stay under 120 lines."""
191+
import openspace.skill_engine.evolution.formatting as mod
192+
src = Path(mod.__file__)
193+
lines = src.read_text(encoding="utf-8").splitlines()
194+
assert len(lines) <= 120, f"formatting.py has {len(lines)} lines (limit 120)"

0 commit comments

Comments
 (0)