Skip to content

Commit b102295

Browse files
Brian KrafftCopilot
andcommitted
feat(evolution): extract confirmation logic (Epic 5.4)
Extract _llm_confirm_evolution and _parse_confirmation from evolver.py to evolution/confirmation.py (~160 lines). - llm_confirm_evolution: LLM-based candidate validation with recording - parse_confirmation: JSON + keyword response parser - _SKILL_CONTENT_MAX_CHARS constant moved to confirmation.py - MRO preserved: calls evolver._format_analysis_context and evolver._parse_confirmation through instance - 28 new tests: JSON/keyword parsing, async flow, error path, MRO seams - 1,498 total pass, 127 skipped Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent fc743e1 commit b102295

3 files changed

Lines changed: 460 additions & 95 deletions

File tree

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
"""Evolution confirmation — LLM-based candidate validation.
2+
3+
Functions extracted from ``SkillEvolver`` (Epic 5.4):
4+
- ``llm_confirm_evolution`` — Ask LLM to confirm a rule-based candidate
5+
- ``parse_confirmation`` — Parse LLM yes/no response (JSON or keyword)
6+
"""
7+
8+
from __future__ import annotations
9+
10+
import copy
11+
import json
12+
import re
13+
from typing import TYPE_CHECKING, List
14+
15+
from openspace.prompts import SkillEnginePrompts
16+
from openspace.skill_engine.skill_utils import truncate
17+
from openspace.utils.logging import Logger
18+
19+
if TYPE_CHECKING:
20+
from openspace.skill_engine.types import (
21+
EvolutionType,
22+
ExecutionAnalysis,
23+
SkillRecord,
24+
)
25+
26+
logger = Logger.get_logger(__name__)
27+
28+
# ---------------------------------------------------------------------------
29+
# Constants
30+
# ---------------------------------------------------------------------------
31+
32+
_SKILL_CONTENT_MAX_CHARS = 12_000 # Max chars of SKILL.md in evolution prompt
33+
34+
35+
# ---------------------------------------------------------------------------
36+
# LLM confirmation
37+
# ---------------------------------------------------------------------------
38+
39+
async def llm_confirm_evolution(
40+
evolver,
41+
*,
42+
skill_record: "SkillRecord",
43+
skill_content: str,
44+
proposed_type: "EvolutionType",
45+
proposed_direction: str,
46+
trigger_context: str,
47+
recent_analyses: List["ExecutionAnalysis"],
48+
) -> bool:
49+
"""Ask LLM to confirm whether a rule-based evolution candidate
50+
truly needs evolution.
51+
52+
Returns True if LLM agrees, False otherwise.
53+
This prevents false positives from rigid threshold-based rules.
54+
55+
The confirmation prompt and response are recorded to
56+
``conversations.jsonl`` under agent_name="SkillEvolver.confirm".
57+
"""
58+
from openspace.recording import RecordingManager
59+
60+
analysis_ctx = evolver._format_analysis_context(recent_analyses)
61+
62+
prompt = SkillEnginePrompts.evolution_confirm(
63+
skill_id=skill_record.skill_id,
64+
skill_content=truncate(skill_content, _SKILL_CONTENT_MAX_CHARS // 2),
65+
proposed_type=proposed_type.value,
66+
proposed_direction=proposed_direction,
67+
trigger_context=trigger_context,
68+
recent_analyses=analysis_ctx,
69+
)
70+
71+
confirm_messages = [{"role": "user", "content": prompt}]
72+
73+
# Record confirmation setup
74+
await RecordingManager.record_conversation_setup(
75+
setup_messages=copy.deepcopy(confirm_messages),
76+
agent_name="SkillEvolver.confirm",
77+
extra={
78+
"skill_id": skill_record.skill_id,
79+
"proposed_type": proposed_type.value,
80+
"trigger_context": trigger_context[:200],
81+
},
82+
)
83+
84+
model = evolver._model or evolver._llm_client.model
85+
try:
86+
result = await evolver._llm_client.complete(
87+
messages=confirm_messages,
88+
model=model,
89+
)
90+
content = result["message"].get("content", "").strip().lower()
91+
confirmed = evolver._parse_confirmation(content)
92+
93+
# Record confirmation response
94+
await RecordingManager.record_iteration_context(
95+
iteration=1,
96+
delta_messages=[{"role": "assistant", "content": content}],
97+
response_metadata={
98+
"has_tool_calls": False,
99+
"confirmed": confirmed,
100+
},
101+
agent_name="SkillEvolver.confirm",
102+
)
103+
104+
return confirmed
105+
except Exception as e:
106+
logger.warning("LLM confirmation failed, defaulting to skip: %s", e)
107+
return False
108+
109+
110+
# ---------------------------------------------------------------------------
111+
# Response parsing
112+
# ---------------------------------------------------------------------------
113+
114+
def parse_confirmation(response: str) -> bool:
115+
"""Parse LLM confirmation response (expects JSON with 'proceed' field).
116+
117+
Parsing order:
118+
1. JSON ``{"proceed": true/false}`` (with markdown-fence stripping)
119+
2. Keyword matching (yes/confirm → True, no/reject/skip → False)
120+
3. Default: False (ambiguous → skip costly evolution)
121+
"""
122+
# Try JSON parse first
123+
try:
124+
cleaned = response.strip()
125+
if cleaned.startswith("```"):
126+
cleaned = re.sub(r"^```(?:json)?\s*\n?", "", cleaned)
127+
cleaned = re.sub(r"\n?```\s*$", "", cleaned)
128+
data = json.loads(cleaned)
129+
if isinstance(data, dict):
130+
return bool(data.get("proceed", False))
131+
except (json.JSONDecodeError, ValueError):
132+
pass
133+
134+
# Fallback: keyword matching.
135+
# - yes/no use strict word boundaries to avoid false positives
136+
# (e.g. "know" matching "no").
137+
# - confirm/reject/skip use stem-style matching so that common
138+
# LLM variants like "confirmed", "rejected", "skipping" still
139+
# parse correctly.
140+
_wb = re.search # shorthand
141+
if (
142+
any(w in response for w in ('"proceed": true', "proceed: true"))
143+
or _wb(r"\byes\b", response)
144+
or _wb(r"\bconfirm\w*\b", response)
145+
):
146+
return True
147+
if (
148+
any(w in response for w in ('"proceed": false', "proceed: false"))
149+
or _wb(r"\bno\b", response)
150+
or _wb(r"\breject\w*\b", response)
151+
or _wb(r"\bskip\w*\b", response)
152+
):
153+
return False
154+
155+
# Default: skip — ambiguous response should not trigger costly evolution
156+
logger.debug("LLM confirmation response was ambiguous, defaulting to skip")
157+
return False

openspace/skill_engine/evolver.py

Lines changed: 13 additions & 95 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,11 @@
3939
log_background_result as _log_background_result_impl,
4040
schedule_background as _schedule_background_impl,
4141
)
42+
from .evolution.confirmation import (
43+
_SKILL_CONTENT_MAX_CHARS,
44+
llm_confirm_evolution as _llm_confirm_evolution_impl,
45+
parse_confirmation as _parse_confirmation_impl,
46+
)
4247
from .evolution.triggers import (
4348
_ANALYSIS_CONTEXT_MAX,
4449
build_context_from_analysis as _build_context_from_analysis_impl,
@@ -99,8 +104,6 @@
99104
EVOLUTION_COMPLETE = SkillEnginePrompts.EVOLUTION_COMPLETE
100105
EVOLUTION_FAILED = SkillEnginePrompts.EVOLUTION_FAILED
101106

102-
_SKILL_CONTENT_MAX_CHARS = 12_000 # Max chars of SKILL.md in evolution prompt
103-
104107
# Agent loop / retry constants
105108
_MAX_EVOLUTION_ITERATIONS = 5 # Max tool-calling rounds for evolution agent
106109
_MAX_EVOLUTION_ATTEMPTS = 3 # Max apply-retry attempts per evolution
@@ -252,103 +255,18 @@ async def _llm_confirm_evolution(
252255
recent_analyses: List[ExecutionAnalysis],
253256
) -> bool:
254257
"""Ask LLM to confirm whether a rule-based evolution candidate
255-
truly needs evolution.
256-
257-
Returns True if LLM agrees, False otherwise.
258-
This prevents false positives from rigid threshold-based rules.
259-
260-
The confirmation prompt and response are recorded to
261-
``conversations.jsonl`` under agent_name="SkillEvolver.confirm".
262-
"""
263-
from openspace.recording import RecordingManager
264-
265-
analysis_ctx = self._format_analysis_context(recent_analyses)
266-
267-
prompt = SkillEnginePrompts.evolution_confirm(
268-
skill_id=skill_record.skill_id,
269-
skill_content=_truncate(skill_content, _SKILL_CONTENT_MAX_CHARS // 2),
270-
proposed_type=proposed_type.value,
258+
truly needs evolution."""
259+
return await _llm_confirm_evolution_impl(
260+
self,
261+
skill_record=skill_record,
262+
skill_content=skill_content,
263+
proposed_type=proposed_type,
271264
proposed_direction=proposed_direction,
272265
trigger_context=trigger_context,
273-
recent_analyses=analysis_ctx,
274-
)
275-
276-
confirm_messages = [{"role": "user", "content": prompt}]
277-
278-
# Record confirmation setup
279-
await RecordingManager.record_conversation_setup(
280-
setup_messages=copy.deepcopy(confirm_messages),
281-
agent_name="SkillEvolver.confirm",
282-
extra={
283-
"skill_id": skill_record.skill_id,
284-
"proposed_type": proposed_type.value,
285-
"trigger_context": trigger_context[:200],
286-
},
266+
recent_analyses=recent_analyses,
287267
)
288268

289-
model = self._model or self._llm_client.model
290-
try:
291-
result = await self._llm_client.complete(
292-
messages=confirm_messages,
293-
model=model,
294-
)
295-
content = result["message"].get("content", "").strip().lower()
296-
confirmed = self._parse_confirmation(content)
297-
298-
# Record confirmation response
299-
await RecordingManager.record_iteration_context(
300-
iteration=1,
301-
delta_messages=[{"role": "assistant", "content": content}],
302-
response_metadata={
303-
"has_tool_calls": False,
304-
"confirmed": confirmed,
305-
},
306-
agent_name="SkillEvolver.confirm",
307-
)
308-
309-
return confirmed
310-
except Exception as e:
311-
logger.warning(f"LLM confirmation failed, defaulting to skip: {e}")
312-
return False
313-
314-
@staticmethod
315-
def _parse_confirmation(response: str) -> bool:
316-
"""Parse LLM confirmation response (expects JSON with 'proceed' field)."""
317-
# Try JSON parse first
318-
try:
319-
# Strip markdown fences
320-
cleaned = response.strip()
321-
if cleaned.startswith("```"):
322-
cleaned = re.sub(r"^```(?:json)?\s*\n?", "", cleaned)
323-
cleaned = re.sub(r"\n?```\s*$", "", cleaned)
324-
data = json.loads(cleaned)
325-
if isinstance(data, dict):
326-
return bool(data.get("proceed", False))
327-
except (json.JSONDecodeError, ValueError):
328-
pass
329-
# Fallback: look for keywords.
330-
# - yes/no use strict word boundaries to avoid false positives
331-
# (e.g. "know" matching "no").
332-
# - confirm/reject/skip use stem-style matching so that common
333-
# LLM variants like "confirmed", "rejected", "skipping" still
334-
# parse correctly.
335-
_wb = re.search # shorthand
336-
if (
337-
any(w in response for w in ('"proceed": true', "proceed: true"))
338-
or _wb(r"\byes\b", response)
339-
or _wb(r"\bconfirm\w*\b", response)
340-
):
341-
return True
342-
if (
343-
any(w in response for w in ('"proceed": false', "proceed: false"))
344-
or _wb(r"\bno\b", response)
345-
or _wb(r"\breject\w*\b", response)
346-
or _wb(r"\bskip\w*\b", response)
347-
):
348-
return False
349-
# Default: skip — ambiguous response should not trigger costly evolution
350-
logger.debug("LLM confirmation response was ambiguous, defaulting to skip")
351-
return False
269+
_parse_confirmation = staticmethod(_parse_confirmation_impl)
352270

353271
async def _evolve_fix(self, ctx: EvolutionContext) -> Optional[SkillRecord]:
354272
"""In-place fix: same name, same directory, new version record.

0 commit comments

Comments
 (0)