diff --git a/backend/apps/agents/browser/browser_agent.py b/backend/apps/agents/browser/browser_agent.py index d285ed30..f144589b 100644 --- a/backend/apps/agents/browser/browser_agent.py +++ b/backend/apps/agents/browser/browser_agent.py @@ -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) diff --git a/backend/apps/agents/browser/browser_batch_replay.py b/backend/apps/agents/browser/browser_batch_replay.py index 0c99b13f..4176d40b 100644 --- a/backend/apps/agents/browser/browser_batch_replay.py +++ b/backend/apps/agents/browser/browser_batch_replay.py @@ -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 diff --git a/backend/apps/agents/browser/browser_schema.py b/backend/apps/agents/browser/browser_schema.py index 5d152a32..e04e747d 100644 --- a/backend/apps/agents/browser/browser_schema.py +++ b/backend/apps/agents/browser/browser_schema.py @@ -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", diff --git a/backend/tests/test_browser_agent_loop.py b/backend/tests/test_browser_agent_loop.py index 2438d847..9c433db9 100644 --- a/backend/tests/test_browser_agent_loop.py +++ b/backend/tests/test_browser_agent_loop.py @@ -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)" diff --git a/backend/tests/test_browser_batch_replay.py b/backend/tests/test_browser_batch_replay.py index 9c683a4b..0559281e 100644 --- a/backend/tests/test_browser_batch_replay.py +++ b/backend/tests/test_browser_batch_replay.py @@ -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