mirror of
https://github.com/openswarm-ai/openswarm.git
synced 2026-09-02 14:28:59 +02:00
[eric] browser: a failed dispatch says whether anything changed, so a caller can branch instead of guessing (ENG-402)
This commit is contained in:
@@ -34,6 +34,7 @@ from backend.apps.agents.browser.browser_history import (
|
||||
refusal_shaped_summary,
|
||||
PAGE_STATE_MARKER,
|
||||
)
|
||||
from backend.apps.agents.browser.effect_disposition import disposition_line, effect_disposition
|
||||
from backend.apps.agents.browser.browser_loop import (
|
||||
LOOP_DETECTION_EXCLUDED_TOOLS,
|
||||
LOOP_HARD_CAP,
|
||||
@@ -3261,7 +3262,12 @@ async def run_browser_agent(
|
||||
summary=summary)
|
||||
final_status = "completed" if honest else "error"
|
||||
if not honest:
|
||||
summary = f"I was not able to complete this task ({dishonest_reason})."
|
||||
# The caller has to be able to DECIDE, and prose does not let it: "may not have gone
|
||||
# out" leaves a parent choosing between stalling and a double-write (ENG-402). Say which
|
||||
# of the three states this is, and what is safe to do next.
|
||||
p_effect = effect_disposition(action_log, send_confirmed=send_confirmed)
|
||||
summary = (f"I was not able to complete this task ({dishonest_reason}). "
|
||||
f"{disposition_line(p_effect)}")
|
||||
logger.warning(
|
||||
f"[browser-agent {session_id}] completion gate caught a ghost: "
|
||||
f"model declared done but {dishonest_reason}; reporting as error"
|
||||
|
||||
@@ -222,7 +222,7 @@ def stagnation_exhausted(streak: int) -> bool:
|
||||
# --- completion honesty gate ---------------------------------------------- A model that ends its turn is NOT proof the goal happened. The worst ghost we measured: multi-minute runs where every tool errored, still reported "completed". This deterministic gate reality-checks the run before we let the status say "done", so a fake success is reported as the failure it actually is.
|
||||
|
||||
# State-changing tools: a task that needed to DO something must land one of these.
|
||||
P_PRODUCTIVE_TOOLS = {
|
||||
STATE_CHANGING_TOOLS = {
|
||||
"BrowserClick", "BrowserClickIndex", "BrowserType", "BrowserNavigate",
|
||||
"BrowserPressKey", "BrowserScroll", "BrowserBatch", "BrowserActVerified",
|
||||
# Enumerated against the live dispatcher 2026-08-13 (ENG-297): these seven change state and were
|
||||
@@ -232,7 +232,7 @@ P_PRODUCTIVE_TOOLS = {
|
||||
"BrowserUploadFile", "BrowserSaveData", "BrowserRepeatFlow",
|
||||
}
|
||||
# Read/extract tools: a look-only task's evidence is that a read returned content.
|
||||
P_READ_TOOLS = {
|
||||
READ_ONLY_TOOLS = {
|
||||
"BrowserGetText", "BrowserGetElements", "BrowserListInteractives",
|
||||
"BrowserListRoutes", "BrowserReplayRoute", "BrowserScreenshot", "BrowserEvaluate",
|
||||
}
|
||||
@@ -426,14 +426,14 @@ def outcome_facts(action_log: list[dict]) -> dict:
|
||||
which is the failure mode this exists to remove.
|
||||
"""
|
||||
log = action_log or []
|
||||
mutations = [a for a in log if a.get("tool") in P_PRODUCTIVE_TOOLS]
|
||||
mutations = [a for a in log if a.get("tool") in STATE_CHANGING_TOOLS]
|
||||
return {
|
||||
"calls": len(log),
|
||||
"mutations_attempted": len(mutations),
|
||||
"mutations_succeeded": len([a for a in mutations if a.get("ok")]),
|
||||
"reads_with_content": len([
|
||||
a for a in log
|
||||
if a.get("tool") in P_READ_TOOLS and a.get("ok")
|
||||
if a.get("tool") in READ_ONLY_TOOLS and a.get("ok")
|
||||
and str(a.get("result_summary") or "").strip()
|
||||
]),
|
||||
}
|
||||
@@ -484,7 +484,7 @@ def completion_is_honest(
|
||||
"check the page before trusting this")
|
||||
if not action_log:
|
||||
return False, "declared done without taking a single action"
|
||||
actions = [a for a in action_log if a.get("tool") in P_PRODUCTIVE_TOOLS]
|
||||
actions = [a for a in action_log if a.get("tool") in STATE_CHANGING_TOOLS]
|
||||
actions_ok = [a for a in actions if a.get("ok")]
|
||||
# Prestage seeds two reads into the log before the model ever runs. They are real page content,
|
||||
# so a model that ANSWERS from them did honest work (a read task needs no further tools). But a
|
||||
@@ -494,7 +494,7 @@ def completion_is_honest(
|
||||
p_answered = bool(str(summary or "").strip())
|
||||
reads_ok = [
|
||||
a for a in action_log
|
||||
if a.get("tool") in P_READ_TOOLS and a.get("ok")
|
||||
if a.get("tool") in READ_ONLY_TOOLS and a.get("ok")
|
||||
and (p_answered or not a.get("seeded"))
|
||||
and str(a.get("result_summary") or "").strip()
|
||||
]
|
||||
|
||||
@@ -0,0 +1,51 @@
|
||||
"""Did the run change anything out there? Answered as a value the caller can branch on.
|
||||
|
||||
Haik, production 1.7.9, on a playlist edit: "Zero-confidence outcomes on a write path is the worst
|
||||
possible spot to leave an agent in: I'm forced to choose between doing nothing and risking a
|
||||
double-write." Our honesty gate was right to refuse to claim the send happened; what it gave back
|
||||
was prose, so a parent agent could not tell "definitely did not happen" from "cannot tell" and had
|
||||
no basis to decide between stalling and retrying (ENG-402).
|
||||
|
||||
Fail-safe by construction: the read-only set is an ALLOWLIST, and anything not on it is assumed to
|
||||
have changed something. A browser tool added tomorrow reads as UNKNOWN rather than as harmless.
|
||||
"""
|
||||
|
||||
from typing import Dict, List, Literal
|
||||
|
||||
from typeguard import typechecked
|
||||
|
||||
from backend.apps.agents.browser.browser_loop import READ_ONLY_TOOLS
|
||||
|
||||
Disposition = Literal["applied", "none", "unknown"]
|
||||
|
||||
NOTHING_HAPPENED = "none"
|
||||
MAY_HAVE_HAPPENED = "unknown"
|
||||
CONFIRMED = "applied"
|
||||
|
||||
|
||||
@typechecked
|
||||
def effect_disposition(action_log: List[Dict], send_confirmed: bool = False) -> Disposition:
|
||||
"""Whether anything left this machine, given what the run actually called.
|
||||
|
||||
"applied" needs positive proof, not a tool that returned ok: a click reporting success is not
|
||||
evidence the write landed, which is the precise gap that told a user "Done, I sent it for you"
|
||||
while the post never arrived.
|
||||
"""
|
||||
p_changing = [a for a in action_log or [] if str(a.get("tool") or "") not in READ_ONLY_TOOLS]
|
||||
if not p_changing:
|
||||
return NOTHING_HAPPENED
|
||||
if send_confirmed:
|
||||
return CONFIRMED
|
||||
return MAY_HAVE_HAPPENED
|
||||
|
||||
|
||||
@typechecked
|
||||
def disposition_line(disposition: Disposition) -> str:
|
||||
"""One sentence telling the caller what it is safe to do next."""
|
||||
if disposition == NOTHING_HAPPENED:
|
||||
return ("Nothing was changed on the page, so this is safe to retry.")
|
||||
if disposition == CONFIRMED:
|
||||
return ("The change was confirmed on the page, so do not repeat it.")
|
||||
return ("A change was attempted and could not be confirmed, so this may or may not have gone "
|
||||
"through. Read the page and check before retrying; repeating it blind risks doing it "
|
||||
"twice.")
|
||||
@@ -0,0 +1,58 @@
|
||||
"""A failed browser dispatch must resolve to a state the caller can branch on.
|
||||
|
||||
Haik, production 1.7.9, on a Spotify playlist edit: "Zero-confidence outcomes on a write path is
|
||||
the worst possible spot to leave an agent in: I'm forced to choose between doing nothing and
|
||||
risking a double-write." The honesty gate was right to refuse to claim the send happened. What it
|
||||
returned was prose, so a parent agent could not tell "definitely did not happen" from "cannot
|
||||
tell", and a naive retry duplicates tracks (ENG-402).
|
||||
"""
|
||||
|
||||
from backend.apps.agents.browser.browser_loop import STATE_CHANGING_TOOLS, READ_ONLY_TOOLS
|
||||
from backend.apps.agents.browser.effect_disposition import (
|
||||
CONFIRMED, MAY_HAVE_HAPPENED, NOTHING_HAPPENED, disposition_line, effect_disposition,
|
||||
)
|
||||
|
||||
|
||||
def p_log(*tools):
|
||||
return [{"tool": t, "ok": True} for t in tools]
|
||||
|
||||
|
||||
def test_a_run_that_only_looked_around_changed_nothing():
|
||||
assert effect_disposition([]) == NOTHING_HAPPENED
|
||||
assert effect_disposition(p_log(*sorted(READ_ONLY_TOOLS))) == NOTHING_HAPPENED
|
||||
|
||||
|
||||
def test_an_attempted_write_with_no_proof_is_unknown_not_failed():
|
||||
# This is the Spotify case: every click returned ok and the send was never confirmed.
|
||||
assert effect_disposition(p_log("BrowserType", "BrowserClick")) == MAY_HAVE_HAPPENED
|
||||
|
||||
|
||||
def test_ok_on_a_click_is_not_proof_the_write_landed():
|
||||
# A click reporting success is how a user got told "Done, I sent it for you" on a post that
|
||||
# never arrived. Only the run's own confirmation signal earns "applied".
|
||||
assert effect_disposition([{"tool": "BrowserClick", "ok": True}]) != CONFIRMED
|
||||
assert effect_disposition([{"tool": "BrowserClick", "ok": True}], send_confirmed=True) == CONFIRMED
|
||||
|
||||
|
||||
def test_an_unrecognised_tool_is_assumed_to_have_changed_something():
|
||||
# The read-only set is an ALLOWLIST. A browser tool added tomorrow must read as UNKNOWN, not as
|
||||
# harmless: ENG-297 shipped seven state-changing tools that no classifier knew about.
|
||||
assert effect_disposition(p_log("BrowserSomethingShippedNextWeek")) == MAY_HAVE_HAPPENED
|
||||
for tool in STATE_CHANGING_TOOLS:
|
||||
assert effect_disposition(p_log(tool)) == MAY_HAVE_HAPPENED, tool
|
||||
|
||||
|
||||
def test_every_disposition_tells_the_caller_what_to_do_next():
|
||||
assert "safe to retry" in disposition_line(NOTHING_HAPPENED)
|
||||
assert "do not repeat" in disposition_line(CONFIRMED)
|
||||
unknown = disposition_line(MAY_HAVE_HAPPENED)
|
||||
assert "check before retrying" in unknown and "twice" in unknown
|
||||
assert "may or may not" in unknown, "the ambiguity has to be stated, not implied"
|
||||
|
||||
|
||||
def test_the_failed_run_carries_the_disposition_home():
|
||||
src = open("backend/apps/agents/browser/browser_agent.py").read()
|
||||
i = src.index("I was not able to complete this task")
|
||||
body = src[i - 400:i + 300]
|
||||
assert "effect_disposition(action_log" in body
|
||||
assert "disposition_line(p_effect)" in body, "the sentence must reach the caller's summary"
|
||||
Reference in New Issue
Block a user