Skip to content

Commit 731809d

Browse files
fix(tools): merge subagents metrics (TaskToolSet) (#2222)
Co-authored-by: openhands <openhands@all-hands.dev>
1 parent 8b61cd0 commit 731809d

2 files changed

Lines changed: 122 additions & 0 deletions

File tree

openhands-tools/openhands/tools/task/manager.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,9 @@ def _get_sub_agent(self, subagent_type: str) -> Agent:
243243

244244
llm_updates: dict = {"stream": False}
245245
sub_agent_llm = parent_llm.model_copy(update=llm_updates)
246+
# Reset metrics such that the sub-agent has its own
247+
# Metrics object
248+
sub_agent_llm.reset_metrics()
246249

247250
return factory.factory_func(sub_agent_llm)
248251

@@ -266,10 +269,21 @@ def _run_task(self, task: Task, prompt: str) -> Task:
266269
task.set_error(str(e))
267270
logger.warning(f"Task {task.id} failed with error: {e}")
268271
finally:
272+
self._update_parent_metrics(parent, task)
269273
self._evict_task(task)
270274

271275
return task
272276

277+
def _update_parent_metrics(self, parent: LocalConversation, task: Task) -> None:
278+
"""
279+
Sync sub-agent metrics into parent before eviction destroys the conversation.
280+
Replace (not merge) because sub-agent metrics are cumulative across resumes.
281+
"""
282+
if task.conversation is not None:
283+
parent.conversation_stats.usage_to_metrics[f"task:{task.id}"] = (
284+
task.conversation.conversation_stats.get_combined_metrics()
285+
)
286+
273287
def close(self) -> None:
274288
"""Clean up tmp directory and remove all created tasks."""
275289
if self._tmp_dir.exists():

tests/tools/task/test_task_manager.py

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -531,3 +531,111 @@ def test_start_task_resume_unknown_raises(self, tmp_path):
531531
resume="task_nonexistent",
532532
conversation=parent,
533533
)
534+
535+
536+
class TestTaskMetrics:
537+
"""Tests for sub-agent metrics isolation and merge-back."""
538+
539+
def setup_method(self):
540+
_reset_registry_for_tests()
541+
542+
def teardown_method(self):
543+
_reset_registry_for_tests()
544+
545+
def test_sub_agent_has_independent_metrics(self, tmp_path):
546+
"""Sub-agent LLM must not share the parent's Metrics object."""
547+
manager, parent = _manager_with_parent(tmp_path)
548+
register_builtins_agents()
549+
550+
parent_llm = parent.agent.llm
551+
sub_agent = manager._get_sub_agent("default")
552+
553+
assert sub_agent.llm.metrics is not parent_llm.metrics
554+
555+
before = parent_llm.metrics.accumulated_cost
556+
sub_agent.llm.metrics.add_cost(1.00)
557+
assert parent_llm.metrics.accumulated_cost == before
558+
559+
def test_run_task_merges_metrics_into_parent(self, tmp_path):
560+
"""After _run_task, sub-agent metrics appear in parent stats."""
561+
manager, parent = _manager_with_parent(tmp_path)
562+
register_builtins_agents()
563+
564+
task = manager._create_task(
565+
subagent_type="default",
566+
description="test",
567+
max_turns=3,
568+
)
569+
570+
# Wire LLM into sub-conv stats (simulates what _ensure_agent_ready does)
571+
sub_conv = task.conversation
572+
assert sub_conv is not None
573+
sub_llm = sub_conv.agent.llm
574+
sub_conv.conversation_stats.usage_to_metrics[sub_llm.usage_id] = sub_llm.metrics
575+
576+
# Simulate sub-agent LLM usage
577+
sub_llm.metrics.add_cost(1.50)
578+
sub_llm.metrics.add_token_usage(
579+
prompt_tokens=100,
580+
completion_tokens=50,
581+
cache_read_tokens=0,
582+
cache_write_tokens=0,
583+
context_window=128000,
584+
response_id="r1",
585+
)
586+
587+
with (
588+
patch.object(sub_conv, "send_message"),
589+
patch.object(sub_conv, "run"),
590+
patch(
591+
"openhands.tools.task.manager.get_agent_final_response",
592+
return_value="done",
593+
),
594+
):
595+
manager._run_task(task=task, prompt="do something")
596+
597+
# Metrics synced to parent under task:<id> key
598+
parent_stats = parent.conversation_stats
599+
assert f"task:{task.id}" in parent_stats.usage_to_metrics
600+
task_metrics = parent_stats.usage_to_metrics[f"task:{task.id}"]
601+
assert task_metrics.accumulated_cost == 1.50
602+
accumulated_token_usage = task_metrics.accumulated_token_usage
603+
assert accumulated_token_usage is not None
604+
assert accumulated_token_usage.prompt_tokens == 100
605+
606+
def test_multiple_tasks_have_separate_metrics(self, tmp_path):
607+
"""Each task gets its own metrics entry in parent stats."""
608+
manager, parent = _manager_with_parent(tmp_path)
609+
register_builtins_agents()
610+
611+
for cost in (1.00, 2.00):
612+
task = manager._create_task(
613+
subagent_type="default",
614+
description="test",
615+
max_turns=3,
616+
)
617+
sub_conv = task.conversation
618+
assert sub_conv is not None
619+
sub_llm = sub_conv.agent.llm
620+
sub_conv.conversation_stats.usage_to_metrics[sub_llm.usage_id] = (
621+
sub_llm.metrics
622+
)
623+
sub_llm.metrics.add_cost(cost)
624+
625+
with (
626+
patch.object(sub_conv, "send_message"),
627+
patch.object(sub_conv, "run"),
628+
patch(
629+
"openhands.tools.task.manager.get_agent_final_response",
630+
return_value="done",
631+
),
632+
):
633+
manager._run_task(task=task, prompt="work")
634+
635+
parent_stats = parent.conversation_stats
636+
assert (
637+
parent_stats.usage_to_metrics["task:task_00000001"].accumulated_cost == 1.00
638+
)
639+
assert (
640+
parent_stats.usage_to_metrics["task:task_00000002"].accumulated_cost == 2.00
641+
)

0 commit comments

Comments
 (0)