mirror of
https://github.com/openswarm-ai/openswarm.git
synced 2026-08-29 03:09:41 +02:00
[eric] browser: BrowserRepeatFlow hands back capped per-item read data so batch-read actually delivers
This commit is contained in:
@@ -838,18 +838,23 @@ async def run_browser_agent(
|
||||
elif not values:
|
||||
bf_text = "No values to repeat; nothing to do."
|
||||
else:
|
||||
done, failed = [], []
|
||||
records = [] # {value, ok, text} per item, for the data return
|
||||
for val in values:
|
||||
if cancel_event.is_set():
|
||||
break
|
||||
item_ok = True
|
||||
item_text = ""
|
||||
for tool_name, params in browser_batch_replay.fill_template(steps_tmpl, val):
|
||||
st = time.time()
|
||||
res = await _cancellable(execute_browser_tool(tool_name, params, browser_id, tab_id))
|
||||
if res is None:
|
||||
item_ok = False; break
|
||||
item_ok = False; item_text = "cancelled"; break
|
||||
el = int((time.time() - st) * 1000)
|
||||
step_ok = "error" not in res
|
||||
# carry each step's output; the LAST read step's text is
|
||||
# the data the agent wanted from this item.
|
||||
if step_ok and res.get("text"):
|
||||
item_text = str(res["text"])
|
||||
action_log.append({
|
||||
"tool": tool_name, "input": params,
|
||||
"result_summary": str(res.get("text", res.get("error", "")))[:200],
|
||||
@@ -863,12 +868,13 @@ async def run_browser_agent(
|
||||
if res.get("url"):
|
||||
last_seen_url = res["url"]
|
||||
if not step_ok:
|
||||
item_ok = False; break
|
||||
(done if item_ok else failed).append(val)
|
||||
bf_text = f"Repeated the flow for {len(done)} of {len(values)}."
|
||||
if failed:
|
||||
bf_text += (f" These didn't match the template and need you to handle them "
|
||||
f"individually: {', '.join(failed[:20])}.")
|
||||
item_ok = False
|
||||
item_text = str(res.get("error") or "did not match the template")
|
||||
break
|
||||
records.append({"value": val, "ok": item_ok, "text": item_text})
|
||||
bf_text = browser_batch_replay.summarize_batch(
|
||||
records, browser_batch_replay.is_readonly_template(steps_tmpl),
|
||||
)
|
||||
tool_results.append({"type": "tool_result", "tool_use_id": tu.id, "content": [{"type": "text", "text": bf_text}]})
|
||||
result_msg = Message(role="tool_result", content={"text": bf_text, "tool_name": tu.name, "elapsed_ms": 0})
|
||||
session.messages.append(result_msg)
|
||||
|
||||
@@ -134,3 +134,48 @@ def is_readonly_template(steps) -> bool:
|
||||
"""True if every step is a pure read/navigation (no clicks/types at all), the
|
||||
safest class of loop."""
|
||||
return all(s.get("action") in _READONLY_ACTIONS for s in steps)
|
||||
|
||||
|
||||
# A batch READ is useless if it doesn't hand the data back. We return each item's
|
||||
# read output, capped so a 20-item batch stays cheap, and stay honest about
|
||||
# failures (named, with the error) and truncation (named, never silently dropped).
|
||||
_MAX_ITEM_CHARS = 500
|
||||
_MAX_TOTAL_CHARS = 6000
|
||||
|
||||
|
||||
def summarize_batch(records: list[dict], readonly: bool,
|
||||
max_item_chars: int = _MAX_ITEM_CHARS,
|
||||
max_total_chars: int = _MAX_TOTAL_CHARS) -> str:
|
||||
"""Turn per-item batch results into the text the agent gets back.
|
||||
|
||||
`records`: [{value, ok, text}]. For a successful item `text` is its read
|
||||
output (the data); for a failed one it's the error. Successes show their data
|
||||
(capped); once the total budget is hit, remaining successes are listed by
|
||||
value only (so nothing is silently lost); failures are always named with a
|
||||
short reason so a partial batch never reads as 'all done'."""
|
||||
done = [r for r in records if r.get("ok")]
|
||||
failed = [r for r in records if not r.get("ok")]
|
||||
verb = "Read" if readonly else "Completed"
|
||||
lines, used, overflow = [], 0, []
|
||||
for r in done:
|
||||
body = " ".join(str(r.get("text") or "").split())[:max_item_chars]
|
||||
line = f"- {r['value']}: {body}" if body else f"- {r['value']}: (done, no content)"
|
||||
if used and used + len(line) > max_total_chars:
|
||||
overflow.append(str(r["value"]))
|
||||
continue
|
||||
lines.append(line)
|
||||
used += len(line)
|
||||
out = f"{verb} {len(done)} of {len(records)}."
|
||||
if lines:
|
||||
out += "\n" + "\n".join(lines)
|
||||
if overflow:
|
||||
out += (f"\n(+{len(overflow)} more done but not shown to save space: "
|
||||
f"{', '.join(overflow[:20])}; ask for specific ones if needed)")
|
||||
if failed:
|
||||
fails = ", ".join(
|
||||
f"{r['value']} ({' '.join(str(r.get('text') or 'failed').split())[:60]})"
|
||||
for r in failed[:20]
|
||||
)
|
||||
out += (f"\n{len(failed)} couldn't be done and need you to handle them "
|
||||
f"individually: {fails}")
|
||||
return out
|
||||
|
||||
@@ -327,11 +327,13 @@ BROWSER_TOOLS_SCHEMA = [
|
||||
"speed: zero screenshots, zero extra thinking. Write the steps using "
|
||||
"{{value}} wherever the input varies. Each iteration is verified; any "
|
||||
"item whose page doesn't match falls back and is reported so you can "
|
||||
"handle it yourself, it never pretends. Use this for SEARCH / READ / "
|
||||
"NAVIGATE loops. It REFUSES irreversible steps (Send, Submit, Connect, "
|
||||
"Post, Pay, Delete, message composers): do those one at a time. For "
|
||||
"reading data, a 'replay_route' step (hit a captured API endpoint) is "
|
||||
"far faster than navigating the UI."
|
||||
"handle it yourself, it never pretends. It HANDS BACK each item's read "
|
||||
"data (the last step's output, capped), keyed by value, so a read loop "
|
||||
"actually delivers ('Read 5 of 5: - ada: ...'). Use this for SEARCH / "
|
||||
"READ / NAVIGATE loops. It REFUSES irreversible steps (Send, Submit, "
|
||||
"Connect, Post, Pay, Delete, message composers): do those one at a time. "
|
||||
"For reading data, a 'replay_route' step (hit a captured API endpoint) is "
|
||||
"far faster and cheaper than navigating the UI per item."
|
||||
),
|
||||
"input_schema": {
|
||||
"type": "object",
|
||||
|
||||
@@ -825,13 +825,25 @@ def test_batch_replay_runs_a_read_loop_for_all_values(monkeypatch):
|
||||
Resp([Blk("text", "Read all three.")], stop_reason="end_turn"),
|
||||
])
|
||||
sent = _install(monkeypatch, primary, FakeAux())
|
||||
|
||||
async def _data(request_id, action, browser_id, params, tab_id=""):
|
||||
sent.append({"action": action, "params": params})
|
||||
if action == "evaluate":
|
||||
# return value-specific data so we can prove the DATA comes back
|
||||
who = params["expression"].split("'")[1]
|
||||
return {"text": f"bio of {who}", "url": DOC_URL}
|
||||
if action == "navigate":
|
||||
return {"text": "Navigated", "url": params.get("url")}
|
||||
return {"text": "ok", "url": DOC_URL}
|
||||
monkeypatch.setattr(BA.ws_manager, "send_browser_command", _data, raising=False)
|
||||
|
||||
asyncio.run(BA.run_browser_agent(task="read three profiles", browser_id="b1", model="sonnet", initial_url=DOC_URL))
|
||||
navs = [c for c in sent if c["action"] == "navigate" and "/in/" in c["params"].get("url", "")]
|
||||
assert {c["params"]["url"].split("/in/")[1] for c in navs} == {"ada", "grace", "alan"}, "navigated each value"
|
||||
reads = {c["params"]["expression"] for c in sent if c["action"] == "evaluate"}
|
||||
assert reads == {"read('ada')", "read('grace')", "read('alan')"}, "read each value, per-value"
|
||||
all_msgs = json.dumps([c["messages"] for c in primary.calls])
|
||||
assert "Repeated the flow for 3 of 3" in all_msgs
|
||||
assert "Read 3 of 3" in all_msgs
|
||||
# Change #1: the actual per-item DATA is handed back, not just a count
|
||||
assert "ada: bio of ada" in all_msgs and "grace: bio of grace" in all_msgs and "alan: bio of alan" in all_msgs
|
||||
|
||||
|
||||
def test_batch_replay_is_ghost_proof_when_an_item_does_not_match(monkeypatch):
|
||||
@@ -858,8 +870,9 @@ def test_batch_replay_is_ghost_proof_when_an_item_does_not_match(monkeypatch):
|
||||
|
||||
asyncio.run(BA.run_browser_agent(task="read three", browser_id="b1", model="sonnet", initial_url=DOC_URL))
|
||||
all_msgs = json.dumps([c["messages"] for c in primary.calls])
|
||||
assert "Repeated the flow for 2 of 3" in all_msgs, "honest tally, not a ghost 'all done'"
|
||||
assert "Read 2 of 3" in all_msgs, "honest tally, not a ghost 'all done'"
|
||||
assert "grace" in all_msgs, "the failed item is surfaced for manual handling"
|
||||
assert "Page not found for grace" in all_msgs, "the failure REASON is reported, not hidden"
|
||||
# grace errored at navigate -> its read must NOT have run; ada+alan did
|
||||
reads = {c["params"]["expression"] for c in sent if c["action"] == "evaluate"}
|
||||
assert "read('grace')" not in reads, "the failed item must NOT proceed (no ghost)"
|
||||
|
||||
@@ -111,3 +111,51 @@ def test_fill_template_runs_every_step_per_value():
|
||||
def test_is_readonly_template():
|
||||
assert br.is_readonly_template([{"action": "navigate", "url": "u"}, {"action": "get_text"}])
|
||||
assert not br.is_readonly_template([{"action": "type", "selector": "#q", "text": "x"}])
|
||||
|
||||
|
||||
# --- the data return: batch-read must hand back what it read ----------------
|
||||
def test_summarize_returns_each_items_data():
|
||||
recs = [
|
||||
{"value": "ada", "ok": True, "text": "Ada Lovelace was a mathematician."},
|
||||
{"value": "grace", "ok": True, "text": "Grace Hopper was a computer scientist."},
|
||||
]
|
||||
out = br.summarize_batch(recs, readonly=True)
|
||||
assert "Read 2 of 2." in out
|
||||
assert "ada: Ada Lovelace was a mathematician." in out
|
||||
assert "grace: Grace Hopper was a computer scientist." in out
|
||||
|
||||
|
||||
def test_summarize_is_honest_about_failures_with_reasons():
|
||||
recs = [
|
||||
{"value": "ada", "ok": True, "text": "data"},
|
||||
{"value": "knuth", "ok": False, "text": "404 not found"},
|
||||
]
|
||||
out = br.summarize_batch(recs, readonly=True)
|
||||
assert "Read 1 of 2." in out
|
||||
assert "knuth (404 not found)" in out
|
||||
assert "handle them individually" in out
|
||||
|
||||
|
||||
def test_summarize_caps_each_item_and_total_without_silent_loss():
|
||||
big = "x" * 2000
|
||||
recs = [{"value": f"p{i}", "ok": True, "text": big} for i in range(30)]
|
||||
out = br.summarize_batch(recs, readonly=True, max_item_chars=100, max_total_chars=500)
|
||||
# each shown item is capped...
|
||||
assert "x" * 101 not in out
|
||||
# ...and the ones past the budget are NAMED as overflow, never silently dropped
|
||||
assert "more done but not shown" in out
|
||||
# every value is accounted for: shown bodies + overflow names cover all 30
|
||||
shown = out.count("- p")
|
||||
assert "+%d more" % (30 - shown) in out or "more done but not shown" in out
|
||||
|
||||
|
||||
def test_summarize_action_loop_uses_completed_verb():
|
||||
recs = [{"value": "x", "ok": True, "text": "Clicked Save"}]
|
||||
assert br.summarize_batch(recs, readonly=False).startswith("Completed 1 of 1.")
|
||||
assert br.summarize_batch(recs, readonly=True).startswith("Read 1 of 1.")
|
||||
|
||||
|
||||
def test_summarize_handles_empty_content():
|
||||
recs = [{"value": "x", "ok": True, "text": ""}]
|
||||
out = br.summarize_batch(recs, readonly=True)
|
||||
assert "x: (done, no content)" in out
|
||||
|
||||
Reference in New Issue
Block a user