From 9d6833f09293352800a3764d7795503c3a069ec6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=99=88=E9=93=AD=E8=BD=A9?= Date: Wed, 23 Sep 2026 01:34:10 +0800 Subject: [PATCH] fix: normalize Pro action targets and bound no-progress recovery --- .env.example | 2 + artemis/agents/operator/operator.py | 48 +++++- artemis/config/settings.py | 2 + tests/unit/agents/test_operator.py | 24 ++- .../agents/test_operator_prompt_estimate.py | 10 +- tests/unit/agents/test_operator_recovery.py | 137 ++++++++++++++++++ tests/unit/agents/test_operator_transcript.py | 22 ++- 7 files changed, 225 insertions(+), 20 deletions(-) create mode 100644 tests/unit/agents/test_operator_recovery.py diff --git a/.env.example b/.env.example index 0f99ff5..7becb93 100644 --- a/.env.example +++ b/.env.example @@ -16,6 +16,8 @@ OPENAI_REASONING_EFFORT=high ARTEMIS_VIDEO_ENABLED=false ARTEMIS_OUTPUTTER_ENABLED=true ARTEMIS_STEP_SUMMARIZER_ENABLED=true +# Pro 连续无工具调用或参数校验失败的最大次数；耗尽后明确失败，避免空转。 +ARTEMIS_OPERATOR_MAX_NO_PROGRESS_ATTEMPTS=3 # 视频分段、重试与单次分析超时（秒） ARTEMIS_VIDEO_CHUNK_SECONDS=60 diff --git a/artemis/agents/operator/operator.py b/artemis/agents/operator/operator.py index aece8a2..0bc09f8 100644 --- a/artemis/agents/operator/operator.py +++ b/artemis/agents/operator/operator.py @@ -19,8 +19,10 @@ from uuid import uuid4 from langchain_core.messages import SystemMessage, ToolMessage from langchain_core.tools import BaseTool -from artemis.core.tool_failure import is_tool_failure +from artemis.agents.validator.tool_declarations import normalize_coordinate_target +from artemis.config.settings import settings from artemis.context import ArtemisContext +from artemis.core.tool_failure import is_tool_failure from artemis.data_engine.trace import ( TraceSpan, trace, @@ -57,6 +59,11 @@ from artemis.utils.visualization import format_minimal_list_with_elements logger = get_logger(__name__) + +class OperatorNoProgressError(RuntimeError): + """The model repeatedly returned no usable tool call; stop without device actions.""" + + DEFERRING_TOOLS = { "ask_diagnoser", "video_analyzer", @@ -485,6 +492,8 @@ class OperatorNode: native_thoughts = [] tool_limit_exceeded = False plan_gate_bounced = False + no_progress_attempts = 0 + no_progress_limit = settings.ARTEMIS_OPERATOR_MAX_NO_PROGRESS_ATTEMPTS for iteration in range(max_iterations): if iteration == max_iterations - 1: @@ -568,8 +577,27 @@ class OperatorNode: logger.error(f"Failed to parse function_call fallback: {e}") if not response.tool_calls: - logger.warning("LLM stopped without calling any tool. Encouraging action.") - break + no_progress_attempts += 1 + if no_progress_attempts >= no_progress_limit: + raise OperatorNoProgressError( + f"模型连续 {no_progress_attempts} 次未提供可执行的工具调用，" + "已停止任务以避免空转；未执行新的设备动作。" + ) + logger.warning("LLM returned no tool call; requesting a concrete tool call.") + current_messages.append(response) + current_messages.append( + SystemMessage( + content=( + "No tool call was returned, so no device action occurred. " + "Use the available tool interface now, not a prose promise or JSON " + "inside text. Use an index from the current element list or " + "normalized coordinates with target_description. If blocked, " + "use an observation/diagnostic tool; do not invent a click or " + "claim success. This recovery has a limited retry budget." + ) + ) + ) + continue current_messages.append(response) @@ -840,9 +868,20 @@ class OperatorNode: break if validation_errors: + no_progress_attempts += 1 + if no_progress_attempts >= no_progress_limit: + raise OperatorNoProgressError( + f"模型连续 {no_progress_attempts} 次未提供可执行的工具调用，" + "工具参数校验仍失败；已停止任务，未执行被拒绝的设备动作。" + ) logger.warning("Some action calls failed validation. Feeding back to LLM.") continue + no_progress_attempts = 0 else: + if no_progress_attempts: + raise OperatorNoProgressError( + "模型在本轮工具调用预算内未恢复有效调用，已停止任务以避免空转。" + ) tool_limit_exceeded = True raw_thinking = "\n".join(raw_thoughts) if raw_thoughts else None @@ -1122,6 +1161,9 @@ class OperatorNode: straight from the indexed list); a coordinate pair carries only the model's own ``target_description``. The two never mix. """ + target = normalize_coordinate_target(target) + if isinstance(target, bool): + return None, "Error: Boolean values are not valid target indices." if isinstance(target, (int, float)): try: target_int = int(target) diff --git a/artemis/config/settings.py b/artemis/config/settings.py index 06d1b9e..f763d3c 100644 --- a/artemis/config/settings.py +++ b/artemis/config/settings.py @@ -111,6 +111,8 @@ class Settings(BaseSettings): ARTEMIS_VIDEO_ENABLED: bool = False ARTEMIS_OUTPUTTER_ENABLED: bool = True ARTEMIS_STEP_SUMMARIZER_ENABLED: bool = True + # Consecutive Pro responses without a valid tool call before failing the task. + ARTEMIS_OPERATOR_MAX_NO_PROGRESS_ATTEMPTS: int = Field(default=3, ge=1, le=20) # Android ADB Connectivity ADB_HOST: str | None = Field(default=DEFAULT_ADB_HOST) diff --git a/tests/unit/agents/test_operator.py b/tests/unit/agents/test_operator.py index 5fcfb7c..fb5be99 100644 --- a/tests/unit/agents/test_operator.py +++ b/tests/unit/agents/test_operator.py @@ -410,7 +410,7 @@ async def test_operator_node_multiple_actions(): @pytest.mark.asyncio async def test_operator_node_no_record_step(): - from artemis.agents.operator.operator import OperatorNode + from artemis.agents.operator.operator import OperatorNode, OperatorNoProgressError from artemis.context import ArtemisContext from unittest.mock import AsyncMock, MagicMock, patch @@ -437,13 +437,11 @@ async def test_operator_node_no_record_step(): with patch("artemis.agents.operator.operator.get_llm", return_value=mock_llm): node = OperatorNode(mock_ctx, transcript_config=LEGACY_TRANSCRIPT) - node_update = await node(mock_state) + with pytest.raises(OperatorNoProgressError): + await node(mock_state) mock_ctx.data_engine.record_step.assert_not_called() - update_dict = node_update - assert "structured_decisions" in update_dict - @pytest.mark.asyncio async def test_operator_dynamic_prompt(): @@ -472,7 +470,13 @@ async def test_operator_dynamic_prompt(): mock_llm = MagicMock() mock_response = MagicMock() - mock_response.tool_calls = [] + mock_response.tool_calls = [ + { + "name": "click", + "args": {"target": [50, 50], "target_description": "button"}, + "id": "call_click", + } + ] async def mock_ainvoke(*args, **kwargs): messages = args[0] @@ -574,7 +578,13 @@ async def test_operator_history_compression(): mock_llm = MagicMock() mock_response = MagicMock() - mock_response.tool_calls = [] + mock_response.tool_calls = [ + { + "name": "click", + "args": {"target": [50, 50], "target_description": "button"}, + "id": "call_click", + } + ] # Smarter file reading mock def mock_read_text(path_self, encoding=None): diff --git a/tests/unit/agents/test_operator_prompt_estimate.py b/tests/unit/agents/test_operator_prompt_estimate.py index 5332d49..7367320 100644 --- a/tests/unit/agents/test_operator_prompt_estimate.py +++ b/tests/unit/agents/test_operator_prompt_estimate.py @@ -69,8 +69,14 @@ def _state(): def _llm(captured: list, usage_metadata=None): mock_llm = MagicMock() response = MagicMock() - response.tool_calls = [] - response.content = "no action this turn" + response.tool_calls = [ + { + "name": "click", + "args": {"target": [50, 50], "target_description": "button"}, + "id": "call_click", + } + ] + response.content = "Click the button." # usage_from_message only trusts a non-empty dict; a MagicMock attribute # (the default) reads as "no usage reported". if usage_metadata is not None: diff --git a/tests/unit/agents/test_operator_recovery.py b/tests/unit/agents/test_operator_recovery.py new file mode 100644 index 0000000..93f7902 --- /dev/null +++ b/tests/unit/agents/test_operator_recovery.py @@ -0,0 +1,137 @@ +"""Regression coverage for quoted click targets and prose-only recovery loops.""" + +from types import SimpleNamespace +from unittest.mock import AsyncMock, Mock + +from langchain_core.messages import AIMessage, SystemMessage, ToolMessage +import pytest + +from artemis.agents.operator.operator import OperatorNode, OperatorNoProgressError +from artemis.config.settings import settings + + +@pytest.fixture +def operator(monkeypatch): + node = object.__new__(OperatorNode) + node.ctx = SimpleNamespace(transcript_ledger=None) + node._available_device_actions = lambda: {"click", "long_press", "input_text"} + node._plan_changed_this_turn = lambda: True + node._max_burst_actions = lambda: 4 + state = SimpleNamespace( + operator_raw_data={"width": 1280, "height": 2856}, + indexed_elements=[{"center": [310, 2610], "text": "model menu", "resource_id": "model"}] + * 13, + open_incident=None, + ) + monkeypatch.setattr(settings, "ARTEMIS_OPERATOR_MAX_NO_PROGRESS_ATTEMPTS", 3) + return node, state + + +@pytest.mark.parametrize("name", ["click", "long_press", "input_text"]) +@pytest.mark.parametrize("target", ["13", "[240, 914]", "240, 914"]) +def test_quoted_targets_keep_same_action_and_grounding(operator, name, target): + node, state = operator + expected_target = 13 if target == "13" else [240, 914] + args = {"target": target, "target_description": "model menu", "text": "test"} + actions, error = node._translate_and_validate_tool({"name": name, "args": args}, state) + expected, _ = node._translate_and_validate_tool( + {"name": name, "args": {**args, "target": expected_target}}, state + ) + assert error is None + assert actions == expected + if target == "13": + assert actions[0]["target_resource_id"] == "model" + else: + assert actions[0]["target_description"] == "model menu" + + +@pytest.mark.parametrize("target", ["0", "14", "model menu", True, "true"]) +def test_invalid_index_is_not_guessed_or_dispatched(operator, target): + node, state = operator + actions, error = node._translate_and_validate_tool( + {"name": "click", "args": {"target": target}}, state + ) + assert not actions + assert error + + +def test_quoted_coordinates_still_need_description(operator): + node, state = operator + actions, error = node._translate_and_validate_tool( + {"name": "click", "args": {"target": "[240, 914]"}}, state + ) + assert not actions + assert "target_description" in error + + +def click(target="13"): + return AIMessage( + content="Open the model menu.", + tool_calls=[{"name": "click", "args": {"target": target}, "id": "call_click"}], + ) + + +async def run_loop(operator, monkeypatch, responses): + node, state = operator + invoke = AsyncMock(side_effect=responses) + monkeypatch.setattr("artemis.agents.operator.operator.acomplete", invoke) + messages = [] + result = await node._invoke_llm_loop(Mock(), messages, [], [], state) + return result, messages, invoke + + +@pytest.mark.asyncio +async def test_prose_retry_keeps_context_and_recovers_once(operator, monkeypatch): + prose = AIMessage(content="I will click the model menu.") + result, messages, invoke = await run_loop(operator, monkeypatch, [prose, click()]) + assert invoke.await_count == 2 + assert len(result[0]) == 1 + assert result[0][0]["coordinates"] == [310, 2610] + assert prose in messages + assert any(isinstance(m, SystemMessage) and "No tool call" in m.content for m in messages) + + +@pytest.mark.asyncio +async def test_invalid_target_can_be_corrected_before_limit(operator, monkeypatch): + result, messages, invoke = await run_loop(operator, monkeypatch, [click("14"), click("13")]) + assert invoke.await_count == 2 + assert len(result[0]) == 1 + assert result[0][0]["coordinates"] == [310, 2610] + outputs = [m for m in messages if isinstance(m, ToolMessage)] + assert [m.status for m in outputs] == ["error", "success"] + + +@pytest.mark.asyncio +@pytest.mark.parametrize("limit", [1, 3]) +async def test_empty_responses_fail_at_configured_limit(operator, monkeypatch, limit): + monkeypatch.setattr(settings, "ARTEMIS_OPERATOR_MAX_NO_PROGRESS_ATTEMPTS", limit) + invoke = AsyncMock(return_value=AIMessage(content="I will click now.")) + monkeypatch.setattr("artemis.agents.operator.operator.acomplete", invoke) + node, state = operator + with pytest.raises(OperatorNoProgressError, match="未提供可执行的工具调用"): + await node._invoke_llm_loop(Mock(), [], [], [], state) + assert invoke.await_count == limit + + +@pytest.mark.asyncio +async def test_invalid_calls_and_prose_share_bounded_recovery(operator, monkeypatch): + invoke = AsyncMock(side_effect=[click("missing"), AIMessage(content="Retry."), click("14")]) + monkeypatch.setattr("artemis.agents.operator.operator.acomplete", invoke) + node, state = operator + messages = [] + with pytest.raises(OperatorNoProgressError, match="校验仍失败"): + await node._invoke_llm_loop(Mock(), messages, [], [], state) + assert invoke.await_count == 3 + assert any(isinstance(m, ToolMessage) and m.status == "error" for m in messages) + assert not any(isinstance(m, ToolMessage) and m.content == "Action Recorded" for m in messages) + + +@pytest.mark.asyncio +async def test_iteration_cap_cannot_restart_no_progress_next_turn(operator, monkeypatch): + monkeypatch.setattr("artemis.agents.operator.operator.OPERATOR_MAX_TOOL_ITERATIONS", 2) + invoke = AsyncMock(return_value=AIMessage(content="I will click now.")) + monkeypatch.setattr("artemis.agents.operator.operator.acomplete", invoke) + node, state = operator + with pytest.raises(OperatorNoProgressError, match="预算内未恢复"): + await node._invoke_llm_loop(Mock(), [], [], [], state) + assert invoke.await_count == 2 diff --git a/tests/unit/agents/test_operator_transcript.py b/tests/unit/agents/test_operator_transcript.py index dc363b5..d389143 100644 --- a/tests/unit/agents/test_operator_transcript.py +++ b/tests/unit/agents/test_operator_transcript.py @@ -141,11 +141,17 @@ def _transcript_state(**overrides): return state -def _no_action_llm(captured: list): +def _action_llm(captured: list): mock_llm = MagicMock() mock_response = MagicMock() - mock_response.tool_calls = [] - mock_response.content = "no action this turn" + mock_response.tool_calls = [ + { + "name": "click", + "args": {"target": [50, 50], "target_description": "button"}, + "id": "call_click", + } + ] + mock_response.content = "Click the button." async def mock_ainvoke(*args, **kwargs): captured.append(list(args[0])) @@ -166,7 +172,7 @@ async def test_transcript_mode_two_turns_build_four_regions(tmp_path): ctx.data_engine.global_base_dir = str(tmp_path) ctx.data_engine.get_agent_friendly_steps.return_value = [] captured: list = [] - mock_llm = _no_action_llm(captured) + mock_llm = _action_llm(captured) with patch("artemis.agents.operator.operator.get_llm", return_value=mock_llm): node = OperatorNode(ctx, transcript_config=MemoryTranscriptConfig(enabled=True)) @@ -237,7 +243,7 @@ async def test_transcript_tail_marks_per_turn_notices_ephemeral(): ctx = _transcript_ctx() captured: list = [] - mock_llm = _no_action_llm(captured) + mock_llm = _action_llm(captured) with patch("artemis.agents.operator.operator.get_llm", return_value=mock_llm): node = OperatorNode(ctx, transcript_config=MemoryTranscriptConfig(enabled=True)) await node( @@ -278,7 +284,7 @@ async def test_transcript_tail_marks_per_turn_notices_ephemeral(): async def test_transcript_actionless_turn_commits_without_validator_message(): ctx = _transcript_ctx() captured: list = [] - mock_llm = _no_action_llm(captured) + mock_llm = _action_llm(captured) with patch("artemis.agents.operator.operator.get_llm", return_value=mock_llm): node = OperatorNode(ctx, transcript_config=MemoryTranscriptConfig(enabled=True)) @@ -315,7 +321,7 @@ async def test_transcript_cold_start_builds_restored_history_block(tmp_path): ] captured: list = [] - mock_llm = _no_action_llm(captured) + mock_llm = _action_llm(captured) with patch("artemis.agents.operator.operator.get_llm", return_value=mock_llm): node = OperatorNode(ctx, transcript_config=MemoryTranscriptConfig(enabled=True)) @@ -338,7 +344,7 @@ async def test_transcript_cold_start_builds_restored_history_block(tmp_path): async def test_flag_off_keeps_legacy_two_message_build_and_no_ledger(): ctx = _transcript_ctx() captured: list = [] - mock_llm = _no_action_llm(captured) + mock_llm = _action_llm(captured) with patch("artemis.agents.operator.operator.get_llm", return_value=mock_llm): node = OperatorNode(ctx, transcript_config=MemoryTranscriptConfig(enabled=False))