Skip to content

Commit ddcbc8c

Browse files
DeepfreezechillBrian KrafftCopilot
authored
Epic 5.10: ResultFormatter + Telemetry — grounding/ package (#48)
* Epic 5.10: Extract results/telemetry to grounding/results.py Extract 8 methods (lines 188-451) from grounding_agent.py into openspace/agents/grounding/results.py: Pure functions (no agent param): - build_iteration_feedback - remove_previous_guidance - format_tool_executions - check_task_completion - extract_last_assistant_message Agent-dependent functions: - generate_final_summary (uses agent._llm_client) - build_final_result (routes through agent for MRO) - record_agent_execution (uses agent._recording_manager) grounding_agent.py reduced from 451 to 237 lines. Pure functions bound as staticmethod; async functions use thin delegates. Removed now-unused imports (copy, json, GroundingAgentPrompts). 38 new tests in test_grounding_results.py covering all functions plus delegation seam verification. Full suite: 1721 passed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: address review findings — strip str(e) leak, add MRO adversarial test, clean _FakeAgent - Remove str(e) from error return in generate_final_summary (security) - Add MRO adversarial regression test with subclass override sentinel - Simplify _FakeAgent staticmethod bindings to bare function assignment - Update exception test to verify str(e) does NOT leak Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Brian Krafft <bkrafft@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 024c21c commit ddcbc8c

3 files changed

Lines changed: 854 additions & 245 deletions

File tree

Lines changed: 285 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,285 @@
1+
"""Results, telemetry, and iteration helpers for GroundingAgent.
2+
3+
Pure functions and agent-delegating helpers for building iteration
4+
feedback, final summaries, task-completion checks, and recording.
5+
Extracted from grounding_agent.py (Epic 5.10).
6+
"""
7+
8+
from __future__ import annotations
9+
10+
import copy
11+
import json
12+
from typing import Any, Dict, List, Optional
13+
14+
from openspace.prompts import GroundingAgentPrompts
15+
from openspace.utils.logging import Logger
16+
17+
logger = Logger.get_logger("openspace.agents.grounding_agent")
18+
19+
20+
# ── Pure functions (no agent parameter) ────────────────────────────
21+
22+
23+
def build_iteration_feedback(
24+
iteration: int,
25+
llm_summary: Optional[str] = None,
26+
add_guidance: bool = True,
27+
) -> Optional[Dict[str, str]]:
28+
"""Build feedback message to add to next iteration.
29+
30+
Returns ``None`` when *llm_summary* is falsy.
31+
"""
32+
if not llm_summary:
33+
return None
34+
35+
feedback_content = GroundingAgentPrompts.iteration_feedback(
36+
iteration=iteration, llm_summary=llm_summary, add_guidance=add_guidance
37+
)
38+
39+
return {"role": "system", "content": feedback_content}
40+
41+
42+
def remove_previous_guidance(messages: List[Dict[str, Any]]) -> None:
43+
"""Remove guidance section from previous iteration feedback messages.
44+
45+
Mutates *messages* in-place.
46+
"""
47+
for msg in messages:
48+
if msg.get("role") == "system":
49+
content = msg.get("content", "")
50+
# Check if this is an iteration feedback message with guidance
51+
if "## Iteration" in content and "Summary" in content and "---" in content:
52+
# Remove everything from "---" onwards (the guidance part)
53+
summary_only = content.split("---")[0].strip()
54+
msg["content"] = summary_only
55+
56+
57+
def format_tool_executions(all_tool_results: List[Dict]) -> List[Dict]:
58+
"""Format raw tool-result dicts into a serialisable execution list."""
59+
executions = []
60+
for tr in all_tool_results:
61+
tool_result_obj = tr.get("result")
62+
tool_call = tr.get("tool_call")
63+
64+
status = "unknown"
65+
if hasattr(tool_result_obj, "status"):
66+
status_obj = tool_result_obj.status
67+
status = getattr(status_obj, "value", status_obj)
68+
69+
# Extract tool_name and arguments from tool_call object (litellm format)
70+
tool_name = "unknown"
71+
arguments: dict = {}
72+
if tool_call is not None:
73+
if hasattr(tool_call, "function"):
74+
# tool_call is an object with .function attribute
75+
tool_name = getattr(tool_call.function, "name", "unknown")
76+
args_raw = getattr(tool_call.function, "arguments", "{}")
77+
if isinstance(args_raw, str):
78+
try:
79+
arguments = json.loads(args_raw) if args_raw.strip() else {}
80+
except json.JSONDecodeError:
81+
arguments = {}
82+
else:
83+
arguments = args_raw if isinstance(args_raw, dict) else {}
84+
elif isinstance(tool_call, dict):
85+
# Fallback: tool_call is a dict
86+
func = tool_call.get("function", {})
87+
tool_name = func.get("name", "unknown")
88+
args_raw = func.get("arguments", "{}")
89+
if isinstance(args_raw, str):
90+
try:
91+
arguments = json.loads(args_raw) if args_raw.strip() else {}
92+
except json.JSONDecodeError:
93+
arguments = {}
94+
else:
95+
arguments = args_raw if isinstance(args_raw, dict) else {}
96+
97+
executions.append(
98+
{
99+
"tool_name": tool_name,
100+
"arguments": arguments,
101+
"backend": tr.get("backend"),
102+
"server_name": tr.get("server_name"),
103+
"status": status,
104+
"content": tool_result_obj.content if hasattr(tool_result_obj, "content") else None,
105+
"error": tool_result_obj.error if hasattr(tool_result_obj, "error") else None,
106+
"execution_time": tool_result_obj.execution_time
107+
if hasattr(tool_result_obj, "execution_time")
108+
else None,
109+
"metadata": tool_result_obj.metadata if hasattr(tool_result_obj, "metadata") else {},
110+
}
111+
)
112+
return executions
113+
114+
115+
def check_task_completion(messages: List[Dict]) -> bool:
116+
"""Return True if the last assistant message contains the COMPLETE token."""
117+
for msg in reversed(messages):
118+
if msg.get("role") == "assistant":
119+
content = msg.get("content", "")
120+
return GroundingAgentPrompts.TASK_COMPLETE in content
121+
return False
122+
123+
124+
def extract_last_assistant_message(messages: List[Dict]) -> str:
125+
"""Return content of the last assistant message, or ``""``."""
126+
for msg in reversed(messages):
127+
if msg.get("role") == "assistant":
128+
return msg.get("content", "")
129+
return ""
130+
131+
132+
# ── Agent-dependent functions ──────────────────────────────────────
133+
134+
135+
async def generate_final_summary(
136+
agent,
137+
instruction: str,
138+
messages: List[Dict],
139+
iterations: int,
140+
) -> tuple[str, bool, List[Dict]]:
141+
"""Generate final summary across all iterations for reporting to upper layer.
142+
143+
Returns:
144+
tuple[str, bool, List[Dict]]: (summary_text, success_flag, context_used)
145+
"""
146+
final_summary_prompt = {
147+
"role": "user",
148+
"content": GroundingAgentPrompts.final_summary(instruction=instruction, iterations=iterations),
149+
}
150+
151+
clean_messages = []
152+
for msg in messages:
153+
# Skip tool result messages
154+
if msg.get("role") == "tool":
155+
continue
156+
# Copy message and remove tool_calls if present
157+
clean_msg = msg.copy()
158+
if "tool_calls" in clean_msg:
159+
del clean_msg["tool_calls"]
160+
clean_messages.append(clean_msg)
161+
162+
clean_messages.append(final_summary_prompt)
163+
164+
# Save context for return
165+
context_for_return = copy.deepcopy(clean_messages)
166+
167+
try:
168+
# Call LLMClient to generate final summary (without tools)
169+
summary_response = await agent._llm_client.complete(
170+
messages=clean_messages, tools=None, execute_tools=False
171+
)
172+
173+
final_summary = summary_response.get("message", {}).get("content", "")
174+
175+
if final_summary:
176+
logger.info(f"Generated final summary: {final_summary[:200]}...")
177+
return final_summary, True, context_for_return
178+
else:
179+
logger.warning("LLM returned empty final summary")
180+
return (
181+
f"Task completed after {iterations} iteration(s). Check execution history for details.",
182+
True,
183+
context_for_return,
184+
)
185+
186+
except Exception as e:
187+
logger.error(f"Error generating final summary: {e}")
188+
return (
189+
f"Task completed after {iterations} iteration(s), but failed to generate summary.",
190+
False,
191+
context_for_return,
192+
)
193+
194+
195+
async def build_final_result(
196+
agent,
197+
instruction: str,
198+
messages: List[Dict],
199+
all_tool_results: List[Dict],
200+
iterations: int,
201+
max_iterations: int,
202+
iteration_contexts: List[Dict] = None,
203+
retrieved_tools_list: List[Dict] = None,
204+
search_debug_info: Dict[str, Any] = None,
205+
) -> Dict[str, Any]:
206+
"""Build the final execution result dict.
207+
208+
Routes ``_check_task_completion``, ``_format_tool_executions``, and
209+
``_extract_last_assistant_message`` through *agent* to preserve MRO.
210+
"""
211+
is_complete = agent._check_task_completion(messages)
212+
213+
tool_executions = agent._format_tool_executions(all_tool_results)
214+
215+
result = {
216+
"instruction": instruction,
217+
"step": agent.step,
218+
"iterations": iterations,
219+
"tool_executions": tool_executions,
220+
"messages": messages,
221+
"iteration_contexts": iteration_contexts or [],
222+
"retrieved_tools_list": retrieved_tools_list or [],
223+
"search_debug_info": search_debug_info,
224+
"active_skills": list(agent._active_skill_ids),
225+
"keep_session": True,
226+
}
227+
228+
if is_complete:
229+
logger.info("Task completed with <COMPLETE> marker")
230+
last_response = agent._extract_last_assistant_message(messages)
231+
result["response"] = last_response.replace(GroundingAgentPrompts.TASK_COMPLETE, "").strip()
232+
result["status"] = "success"
233+
else:
234+
result["response"] = agent._extract_last_assistant_message(messages)
235+
result["status"] = "incomplete"
236+
result["warning"] = (
237+
f"Task reached max iterations ({max_iterations}) without completion. "
238+
f"This may indicate the task needs more steps or clarification."
239+
)
240+
241+
return result
242+
243+
244+
async def record_agent_execution(
245+
agent,
246+
result: Dict[str, Any],
247+
instruction: str,
248+
) -> None:
249+
"""Record agent execution to recording manager.
250+
251+
No-op when ``agent._recording_manager`` is falsy.
252+
"""
253+
if not agent._recording_manager:
254+
return
255+
256+
# Extract tool execution summary
257+
tool_summary = []
258+
if result.get("tool_executions"):
259+
for exec_info in result["tool_executions"]:
260+
tool_summary.append(
261+
{
262+
"tool": exec_info.get("tool_name", "unknown"),
263+
"backend": exec_info.get("backend", "unknown"),
264+
"status": exec_info.get("status", "unknown"),
265+
}
266+
)
267+
268+
await agent._recording_manager.record_agent_action(
269+
agent_name=agent.name,
270+
action_type="execute",
271+
input_data={"instruction": instruction},
272+
reasoning={
273+
"response": result.get("response", ""),
274+
"tools_selected": tool_summary,
275+
},
276+
output_data={
277+
"status": result.get("status", "unknown"),
278+
"iterations": result.get("iterations", 0),
279+
"num_tool_executions": len(result.get("tool_executions", [])),
280+
},
281+
metadata={
282+
"step": agent.step,
283+
"instruction": instruction,
284+
},
285+
)

0 commit comments

Comments
 (0)