fix: normalize Pro action targets and bound no-progress recovery
This commit is contained in:
1 parent
2ec563c079
commit
9d6833f092
7 files changed
+225
-20
No files matched your search
@@ -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):
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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
|
||||
@@ -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))
|
||||
|
||||
Reference in new issue
Block a user