fix: give final acceptance a shared retry budget and diagnostics
This commit is contained in:
1 parent
50638d4130
commit
32441e79c4
18 files changed
+779
-40
No files matched your search
@@ -111,16 +111,36 @@ async def test_flash_reuses_checker_with_original_prompt_and_final_report(
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_acceptance_timeout_finishes_with_unknown_verdict(tmp_path, monkeypatch):
|
||||
from artemis.llm import final_review
|
||||
|
||||
async def hanging_check(*args, **kwargs):
|
||||
await asyncio.Event().wait()
|
||||
|
||||
monkeypatch.setattr(flash, "run_final_check", hanging_check)
|
||||
monkeypatch.setattr(final_review, "MAX_FINAL_REVIEW_SECONDS", 0.01)
|
||||
data = await flash.settle_flash_outcome(
|
||||
context(tmp_path, checkpoint_timeout=0.01), "任务", {"status": "completed"}
|
||||
)
|
||||
assert data["task_status"] == "completed"
|
||||
assert data["test_result"]["status"] == "inconclusive"
|
||||
assert "0.01 秒" in data["test_result"]["reason"]
|
||||
assert "总预算已用尽(0.01 秒)" in data["test_result"]["reason"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_flash_final_review_can_outlast_midway_checkpoint_timeout(tmp_path, monkeypatch):
|
||||
async def slower_review(*args, **kwargs):
|
||||
await asyncio.sleep(0.02)
|
||||
return CheckReport(
|
||||
test_result=CaseTestResult(
|
||||
status="passed", reason="完整核对实际证据", evidence=["步骤 1 截图"]
|
||||
)
|
||||
)
|
||||
|
||||
monkeypatch.setattr(flash, "run_final_check", slower_review)
|
||||
data = await flash.settle_flash_outcome(
|
||||
context(tmp_path, checkpoint_timeout=0.001), "任务", {"status": "completed"}
|
||||
)
|
||||
assert data["test_result"]["status"] == "passed"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
||||
@@ -125,6 +125,46 @@ async def test_review_stops_capture_before_waiting_and_seals_exploration(explora
|
||||
ctx.on_execution_resume.assert_not_called()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_exploration_guidance_can_interrupt_shared_final_review_budget(exploration_exit):
|
||||
from types import SimpleNamespace
|
||||
|
||||
ctx, _, _, _ = exploration_exit
|
||||
started = asyncio.Event()
|
||||
closed = asyncio.Event()
|
||||
|
||||
async def review(*args, **kwargs):
|
||||
started.set()
|
||||
try:
|
||||
await asyncio.Event().wait()
|
||||
finally:
|
||||
closed.set()
|
||||
|
||||
async def interrupt(operation, **kwargs):
|
||||
task = asyncio.create_task(operation)
|
||||
await started.wait()
|
||||
task.cancel()
|
||||
with pytest.raises(asyncio.CancelledError):
|
||||
await task
|
||||
return True, None
|
||||
|
||||
ctx.task_control = SimpleNamespace(
|
||||
run_interruptibly=interrupt, directory=ctx.data_engine.base_dir / "control"
|
||||
)
|
||||
with patch("artemis.graph.graph.run_final_check", new=review):
|
||||
result = await exit_settlement_node(_make_state(execution_report="调查报告"), ctx)
|
||||
assert result == {"exit_settlement_route": "continue"}
|
||||
assert closed.is_set()
|
||||
assert ctx.final_check_attempts == 0
|
||||
assert not (ctx.data_engine.base_dir / "run_outcome.json").exists()
|
||||
diagnostics = [
|
||||
call.kwargs["payload"]
|
||||
for call in ctx.data_engine.record_trace.call_args_list
|
||||
if call.kwargs.get("type") == "final_review_diagnostic"
|
||||
]
|
||||
assert diagnostics[-1]["status"] == "cancelled"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_completed_exploration_waits_for_report_before_review(exploration_exit):
|
||||
ctx, repo, session, run = exploration_exit
|
||||
@@ -237,6 +277,32 @@ def _make_state(**overrides):
|
||||
return state
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_pro_final_review_has_its_own_budget_and_reports_exhaustion(tmp_path, monkeypatch):
|
||||
from artemis.llm import final_review
|
||||
from artemis.runtime.case_result import CaseTestResult
|
||||
|
||||
_write_plan(tmp_path, DONE_PLAN_NO_CHECKS)
|
||||
ctx = _make_ctx(tmp_path, checkpoint_timeout=0.001)
|
||||
|
||||
async def slow_review(*args, **kwargs):
|
||||
await asyncio.sleep(0.02)
|
||||
return CheckReport(
|
||||
test_result=CaseTestResult(status="passed", reason="证据齐全", evidence=["步骤 1 截图"])
|
||||
)
|
||||
|
||||
with patch("artemis.graph.graph.run_final_check", new=slow_review):
|
||||
result = await exit_settlement_node(_make_state(), ctx)
|
||||
assert result["run_outcome"]["test_result"]["status"] == "passed"
|
||||
|
||||
monkeypatch.setattr(final_review, "MAX_FINAL_REVIEW_SECONDS", 0.001)
|
||||
with patch("artemis.graph.graph.run_final_check", new=slow_review):
|
||||
result = await exit_settlement_node(_make_state(), ctx)
|
||||
assert result["exit_settlement_route"] == "end"
|
||||
assert result["run_outcome"]["test_result"]["status"] == "inconclusive"
|
||||
assert "总预算已用尽" in result["run_outcome"]["test_result"]["reason"]
|
||||
|
||||
|
||||
def _write_plan(tmp_path, content):
|
||||
notes = tmp_path / "notes"
|
||||
notes.mkdir(parents=True, exist_ok=True)
|
||||
|
||||
@@ -0,0 +1,245 @@
|
||||
import asyncio
|
||||
from copy import deepcopy
|
||||
import json
|
||||
from types import SimpleNamespace
|
||||
|
||||
from langchain_core.messages import AIMessageChunk, HumanMessage, ToolMessage
|
||||
from langchain_core.tools import tool
|
||||
import pytest
|
||||
|
||||
from artemis.agents.checker import checker
|
||||
from artemis.config.llm import LLM
|
||||
from artemis.llm import final_review
|
||||
from artemis.llm.diagnostics import ACTIVE_ATTEMPT
|
||||
from artemis.llm.final_review import (
|
||||
ACTIVE_FINAL_REVIEW,
|
||||
FinalReviewBudget,
|
||||
FinalReviewBudgetExceeded,
|
||||
)
|
||||
from artemis.llm.reliability import LLMExhaustedError
|
||||
from artemis.llm.router import ModelEndpoint
|
||||
from artemis.services import llm
|
||||
|
||||
|
||||
class RecordingEngine:
|
||||
current_session_id = "review-test"
|
||||
current_step_id = None
|
||||
|
||||
def __init__(self):
|
||||
self.records = []
|
||||
|
||||
def record_trace(self, **record):
|
||||
self.records.append(deepcopy(record))
|
||||
|
||||
def _publish(self, *_):
|
||||
pass
|
||||
|
||||
def latest(self, kind):
|
||||
return next(r["payload"] for r in reversed(self.records) if r["type"] == kind)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def engine(monkeypatch, tmp_path):
|
||||
engine = RecordingEngine()
|
||||
monkeypatch.setattr(llm, "_CURRENT_DATA_ENGINE", engine)
|
||||
monkeypatch.setattr(llm.settings, "TRACES_PATH", str(tmp_path))
|
||||
monkeypatch.setattr(llm, "PAUSE_FILE", tmp_path / "pause")
|
||||
monkeypatch.setattr(
|
||||
llm, "retry_policy_for", lambda _: SimpleNamespace(delay_for=lambda _: 0, max_attempts=2)
|
||||
)
|
||||
llm._ENDPOINT_BREAKER._states.clear()
|
||||
yield engine
|
||||
llm._ENDPOINT_BREAKER._states.clear()
|
||||
assert ACTIVE_FINAL_REVIEW.get() is None
|
||||
assert ACTIVE_ATTEMPT.get() is None
|
||||
|
||||
|
||||
def budget(engine, *, timeout=1, retries=1):
|
||||
return FinalReviewBudget(
|
||||
engine, attempt_id="final#1", request_timeout=timeout, max_retries=retries, retry_delay=0
|
||||
)
|
||||
|
||||
|
||||
def test_budget_uses_effective_checker_config_and_respects_overall_cap(monkeypatch):
|
||||
monkeypatch.setattr(llm.settings, "OPENAI_TIMEOUT_SECONDS", 300)
|
||||
monkeypatch.setattr(llm.settings, "OPENAI_MAX_RETRIES", 5)
|
||||
monkeypatch.setattr(llm.settings, "OPENAI_RETRY_INTERVAL_SECONDS", 10)
|
||||
monkeypatch.setattr(llm.settings, "OPENAI_RATE_LIMIT_RETRY_SECONDS", 60)
|
||||
cfg = LLM(model="test", timeout=20, max_retries=2)
|
||||
ctx = SimpleNamespace(llm_config=SimpleNamespace(get_agent=lambda name: cfg), data_engine=None)
|
||||
review = llm.final_review_budget(ctx, attempt_id="final#1")
|
||||
assert review.total_seconds == 280 # three normal calls + two retries + waits/tool reads
|
||||
assert review.data["request_timeout_seconds"] == 20
|
||||
assert review.data["max_retries"] == 2
|
||||
|
||||
cfg.timeout = 300
|
||||
cfg.max_retries = 100
|
||||
assert llm.final_review_budget(ctx, attempt_id="final#2").total_seconds == 3600
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_checker_retries_only_failed_request_with_existing_tool_evidence(engine, monkeypatch):
|
||||
calls = []
|
||||
evidence_reads = 0
|
||||
stream_closed = asyncio.Event()
|
||||
|
||||
@tool
|
||||
async def read_evidence() -> str:
|
||||
"""Read the already-recorded page evidence."""
|
||||
nonlocal evidence_reads
|
||||
evidence_reads += 1
|
||||
return "实际为中文搜索,预期为英文搜索"
|
||||
|
||||
class ScriptedModel:
|
||||
def bind_tools(self, *args, **_):
|
||||
return self
|
||||
|
||||
async def astream(self, messages, **_):
|
||||
calls.append(deepcopy(messages))
|
||||
if len(calls) == 1:
|
||||
yield AIMessageChunk(
|
||||
content="",
|
||||
tool_calls=[{"name": "read_evidence", "args": {}, "id": "read-1"}],
|
||||
)
|
||||
elif len(calls) == 2:
|
||||
try:
|
||||
yield AIMessageChunk(content='{"test_result":{"status":"passed","reason":"')
|
||||
await asyncio.Event().wait() # the *request* timer triggers a retry
|
||||
finally:
|
||||
stream_closed.set()
|
||||
else:
|
||||
yield AIMessageChunk(
|
||||
content=json.dumps(
|
||||
{
|
||||
"test_result": {
|
||||
"status": "failed",
|
||||
"reason": "中文搜索不满足英文预期",
|
||||
"evidence": ["read_evidence:实际为中文搜索"],
|
||||
}
|
||||
}
|
||||
)
|
||||
)
|
||||
|
||||
wrapper = llm.RobustChatModelWrapper(
|
||||
ScriptedModel(),
|
||||
endpoint=ModelEndpoint(model_name="test", timeout_seconds=0.02, max_retries=1),
|
||||
)
|
||||
monkeypatch.setattr(checker, "get_llm", lambda **_: wrapper)
|
||||
monkeypatch.setattr(checker, "trace_langchain_tool", lambda tool, ctx: tool)
|
||||
events = []
|
||||
monkeypatch.setattr(llm, "_record_llm_event", lambda *args, **kwargs: events.append(args))
|
||||
ctx = SimpleNamespace(execution_setup=SimpleNamespace(checker_max_iterations=4))
|
||||
review = budget(engine)
|
||||
report = await review.run(
|
||||
lambda: checker._run_check_loop(
|
||||
ctx, [HumanMessage(content="核实英文搜索")], [read_evidence], []
|
||||
)
|
||||
)
|
||||
|
||||
assert report.test_result.status == "failed" # partial `passed` was never a verdict
|
||||
assert evidence_reads == 1
|
||||
assert len(calls) == 3
|
||||
assert calls[1] == calls[2]
|
||||
assert any(isinstance(m, ToolMessage) and "中文搜索" in m.content for m in calls[2])
|
||||
assert stream_closed.is_set()
|
||||
assert any(e[0] == "llm_stream_reset" and e[1]["action"] == "discard" for e in events)
|
||||
stored = engine.latest("final_review_diagnostic")
|
||||
assert stored["status"] == "completed"
|
||||
assert stored["request_count"] == 2
|
||||
assert stored["retry_count"] == 1
|
||||
assert stored["last_failure_category"] == "timeout"
|
||||
assert engine.latest("llm_diagnostic")["final_review"]["review_id"] == stored["review_id"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize("phase", ["request", "retry_wait", "evidence"])
|
||||
async def test_total_budget_expiry_is_not_user_cancellation_or_a_new_retry(
|
||||
engine, monkeypatch, tmp_path, phase
|
||||
):
|
||||
monkeypatch.setattr(final_review, "MAX_FINAL_REVIEW_SECONDS", 0.02)
|
||||
monkeypatch.setattr(
|
||||
llm, "retry_policy_for", lambda _: SimpleNamespace(delay_for=lambda _: 60, max_attempts=6)
|
||||
)
|
||||
cancelled = asyncio.Event()
|
||||
calls = 0
|
||||
|
||||
async def request():
|
||||
nonlocal calls
|
||||
calls += 1
|
||||
if phase == "retry_wait":
|
||||
raise ConnectionError("connection reset")
|
||||
try:
|
||||
await asyncio.Event().wait()
|
||||
finally:
|
||||
cancelled.set()
|
||||
|
||||
review = budget(engine, retries=5)
|
||||
|
||||
async def operation():
|
||||
if phase == "evidence":
|
||||
return await request()
|
||||
return await llm._run_with_recovery(request, request_timeout=1, max_retries=5)
|
||||
|
||||
with pytest.raises(FinalReviewBudgetExceeded, match="总预算已用尽"):
|
||||
await review.run(operation)
|
||||
assert calls == 1
|
||||
assert not (tmp_path / "pause").exists()
|
||||
assert engine.latest("final_review_diagnostic")["status"] == "budget_exhausted"
|
||||
if phase != "retry_wait":
|
||||
assert cancelled.is_set()
|
||||
if phase != "evidence":
|
||||
stored = engine.latest("llm_diagnostic")
|
||||
assert stored["phase"] == "failed"
|
||||
assert stored["category"] == "final_review_budget_exhausted"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_exhausted_final_review_request_ends_without_hidden_pause(
|
||||
engine, monkeypatch, tmp_path
|
||||
):
|
||||
calls = 0
|
||||
|
||||
async def fails():
|
||||
nonlocal calls
|
||||
calls += 1
|
||||
raise TimeoutError()
|
||||
|
||||
pause = []
|
||||
monkeypatch.setattr(llm, "_handle_llm_pause_and_resume", lambda *_: pause.append(True))
|
||||
with pytest.raises(LLMExhaustedError):
|
||||
await budget(engine).run(
|
||||
lambda: llm._run_with_recovery(fails, request_timeout=1, max_retries=1)
|
||||
)
|
||||
assert calls == 2
|
||||
assert not pause
|
||||
assert not (tmp_path / "pause").exists()
|
||||
assert engine.latest("final_review_diagnostic")["status"] == "request_retries_exhausted"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_user_cancellation_closes_review_and_child_without_retry(engine):
|
||||
started = asyncio.Event()
|
||||
cancelled = asyncio.Event()
|
||||
calls = 0
|
||||
|
||||
async def request():
|
||||
nonlocal calls
|
||||
calls += 1
|
||||
started.set()
|
||||
try:
|
||||
await asyncio.Event().wait()
|
||||
finally:
|
||||
cancelled.set()
|
||||
|
||||
task = asyncio.create_task(
|
||||
budget(engine).run(
|
||||
lambda: llm._run_with_recovery(request, request_timeout=1, max_retries=5)
|
||||
)
|
||||
)
|
||||
await started.wait()
|
||||
task.cancel()
|
||||
with pytest.raises(asyncio.CancelledError):
|
||||
await task
|
||||
assert calls == 1 and cancelled.is_set()
|
||||
assert engine.latest("final_review_diagnostic")["status"] == "cancelled"
|
||||
assert engine.latest("llm_diagnostic")["category"] == "cancelled"
|
||||
@@ -251,7 +251,10 @@ def test_error_redaction_and_image_headers():
|
||||
assert error_text(RuntimeError()) == "RuntimeError"
|
||||
|
||||
|
||||
def test_snapshots_survive_out_of_order_writes_and_export(tmp_path):
|
||||
@pytest.mark.parametrize(
|
||||
"kind, key", [("llm_diagnostic", "requests"), ("final_review_diagnostic", "final_reviews")]
|
||||
)
|
||||
def test_snapshots_survive_out_of_order_writes_and_export(tmp_path, kind, key):
|
||||
from artemis.data_engine.models import TraceRecord
|
||||
from artemis.data_engine.storage import StorageManager
|
||||
from apps.admin_console.database.connection import db_session
|
||||
@@ -270,13 +273,14 @@ def test_snapshots_survive_out_of_order_writes_and_export(tmp_path):
|
||||
TraceRecord(
|
||||
trace_id=trace_id,
|
||||
session_id=session_id,
|
||||
type="llm_diagnostic",
|
||||
type=kind,
|
||||
name="llm_request",
|
||||
timestamp=10 + sequence,
|
||||
status="success" if sequence == 3 else "running",
|
||||
payload={
|
||||
"sequence": sequence,
|
||||
"phase": phase,
|
||||
"status": "completed" if sequence == 3 else "running",
|
||||
"request_id": "call-one",
|
||||
"started_at": 10,
|
||||
"ended_at": 13 if sequence == 3 else None,
|
||||
@@ -284,10 +288,51 @@ def test_snapshots_survive_out_of_order_writes_and_export(tmp_path):
|
||||
)
|
||||
)
|
||||
result = request_diagnostics(str(session_id), db_path=path)
|
||||
assert len(result["requests"]) == 1
|
||||
assert result["requests"][0]["phase"] == "completed"
|
||||
assert result["requests"][0]["active"] is False
|
||||
assert result["requests"][0]["timestamps_utc"]["started_at"].endswith("+00:00")
|
||||
assert len(result[key]) == 1
|
||||
assert result[key][0]["phase"] == "completed"
|
||||
assert result[key][0]["active"] is False
|
||||
assert result[key][0]["timestamps_utc"]["started_at"].endswith("+00:00")
|
||||
assert result["total"] == (1 if kind == "llm_diagnostic" else 0)
|
||||
|
||||
|
||||
def test_review_diagnostics_show_evidence_stage_without_a_live_model_request(tmp_path, monkeypatch):
|
||||
from artemis.data_engine.storage import StorageManager
|
||||
from apps.admin_console.database.connection import db_session
|
||||
from apps.admin_console.services import request_diagnostics as service
|
||||
|
||||
path = tmp_path / "data.db"
|
||||
StorageManager(db_path=path, base_trace_dir=tmp_path)
|
||||
monkeypatch.setattr(service.time, "time", lambda: 130)
|
||||
with db_session(path) as conn:
|
||||
conn.execute("INSERT INTO sessions(session_id,status) VALUES ('task', 'running')")
|
||||
value = {
|
||||
"review_id": "final-review",
|
||||
"status": "running",
|
||||
"stage": "reading_evidence",
|
||||
"started_at": 100,
|
||||
"deadline_at": 200,
|
||||
"total_seconds": 100,
|
||||
"updated_at": 110,
|
||||
}
|
||||
conn.execute(
|
||||
"INSERT INTO traces(trace_id,session_id,type,timestamp,payload) "
|
||||
"VALUES ('review', 'task', 'final_review_diagnostic', 100, ?)",
|
||||
(json.dumps(value),),
|
||||
)
|
||||
conn.commit()
|
||||
result = service.request_diagnostics("task", db_path=path, limit=100)
|
||||
assert result["requests"] == []
|
||||
review = result["final_reviews"][0]
|
||||
assert review["active"] is True
|
||||
assert review["stage"] == "reading_evidence"
|
||||
assert review["elapsed_seconds"] == 30
|
||||
assert review["remaining_seconds"] == 70
|
||||
assert review["updated_at"] == 110 # preserve the actual last persisted event time
|
||||
assert review["timestamps_utc"]["deadline_at"].endswith("+00:00")
|
||||
assert (
|
||||
service.request_diagnostics("task", db_path=path, offset=1)["final_reviews"][0]["active"]
|
||||
is False
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
||||
Reference in new issue
Block a user