From 9eb0e2cb2b451b31ab9417a2e0424eaeaa4e51e8 Mon Sep 17 00:00:00 2001 From: ciregenz Date: Mon, 8 Jun 2026 23:47:26 -0700 Subject: [PATCH] [eric] browser: new gathered data resets the spin backstop, so a page-by-page gather runs to completion instead of getting cut with partial results --- backend/apps/agents/browser/browser_agent.py | 98 ++++++++++++------- backend/apps/agents/browser/browser_schema.py | 7 +- backend/tests/test_browser_agent_loop.py | 32 +++++- 3 files changed, 95 insertions(+), 42 deletions(-) diff --git a/backend/apps/agents/browser/browser_agent.py b/backend/apps/agents/browser/browser_agent.py index 061b056c..36a4b9c6 100644 --- a/backend/apps/agents/browser/browser_agent.py +++ b/backend/apps/agents/browser/browser_agent.py @@ -957,6 +957,9 @@ async def run_browser_agent( # model's half-finished sentence as the reply). We nudge it to wrap up ONCE, # so it summarizes what it has via Done; a second trip then stops for real. wrapup_nudged = False + # Distinct read results seen, so the backstop tells a productive page-by-page + # gather (new data each turn) from genuine spinning (re-reading the same thing). + seen_read_sigs: set[str] = set() # rows already shown to the model; attached state shrinks to the delta attached_state_seen: set[str] = set() # under-batching telemetry + nudge state @@ -1125,46 +1128,65 @@ async def run_browser_agent( f"batch={_turn_has_batch} streak={single_action_streak}" ) - # Post-send stall: the send already CONFIRMED, so a pure-perception turn - # now (no action, the model didn't finish) is wasted re-verification. Push - # hard to the OUTCOME; if it spins again, end (the confirm IS the proof, the - # completion gate re-checks the log). An action turn means real more-to-do, - # so reset and let it continue (multi-send stays safe). + # Progress = an action OR a read that returned content we haven't seen. + # A page-by-page gather (a fresh BrowserExtract each turn) is real progress, + # not spinning, so detecting new data here keeps the backstop from cutting + # it off with partial results. Re-reading the same page yields no new sig. + _novel_read = False + for _a in action_log: + if (_a.get("ok") and _a.get("tool") not in _BATCHABLE_ACTION_TOOLS + and _a.get("tool") not in ("ReportProgress", "Done")): + _sig = f"{_a.get('tool')}:{_a.get('result_summary') or ''}" + if _sig not in seen_read_sigs: + seen_read_sigs.add(_sig) + _novel_read = True + + # Out of turn budget with no answer yet: nudge a wrap-up so a long-running + # gather delivers what it has via Done at the cap, instead of the for-loop + # ending on the model's half-finished sentence. Same one-shot channel. + if turn >= MAX_TURNS - 4 and not wrapup_nudged and not done_called and not send_confirmed: + wrapup_nudged = True + wrapup_pending = True + logger.info(f"[browser-agent {session_id}] turn budget low ({turn}/{MAX_TURNS}); nudging wrap-up") + + # Spin backstop: a pure-perception turn that ISN'T gathering new data is + # wasted (re-verifying a send, or re-looking at the same page). Bound it. if _turn_actions == 0: - perception_stall += 1 - # The general backstop only applies AFTER the agent has actually done - # something (a real mutation, not a read), early pure-perception is - # legitimate orienting (a cold/slow page can need several look turns - # before the first action) and we must never cut that short. Reads - # land in action_log with ok=True too, so check the TOOL, not just ok. - _acted = any(a.get("ok") and a.get("tool") in (_BATCHABLE_ACTION_TOOLS | {"BrowserBatch"}) - for a in action_log) - _stall_limit = (_POST_SEND_STALL_LIMIT if send_confirmed - else (_PERCEPTION_STALL_LIMIT if _acted else 10 ** 9)) - if perception_stall >= _stall_limit: - if send_confirmed: - # the send registered: hand the parent a real done. The raw - # action-log proof (indices/coords) is machine-speak, kept out. - done_called = True - done_message = "All set, your message went through and it's showing in the conversation now." - logger.info(f"[browser-agent {session_id}] ending: {perception_stall} post-send perception turns") - break - if not wrapup_nudged: - # First trip on a non-send run: don't cut it off mid-thought. - # Ride a wrap-up nudge out on this turn's tool_results (appended - # below) so next turn the model answers from what it has via Done. - wrapup_nudged = True - wrapup_pending = True - perception_stall = 0 - logger.info(f"[browser-agent {session_id}] spin backstop: nudging to wrap up via Done") - else: - # Nudge already spent and it's still looping: stop with a clean - # line, never the model's half-finished sentence. - logger.info(f"[browser-agent {session_id}] ending: wrap-up nudge ignored, stopping") - if not done_called: + # gather tasks: new content IS the work, so it resets the stall. After + # a send confirmed, re-reading is just re-verification, never progress. + if _novel_read and not send_confirmed: + perception_stall = 0 + else: + perception_stall += 1 + # The general backstop only applies AFTER the agent has actually + # done something; early pure-perception is legitimate orienting on a + # cold/slow page, which we must never cut short. + _acted = any(a.get("ok") and a.get("tool") in (_BATCHABLE_ACTION_TOOLS | {"BrowserBatch"}) + for a in action_log) + _stall_limit = (_POST_SEND_STALL_LIMIT if send_confirmed + else (_PERCEPTION_STALL_LIMIT if _acted else 10 ** 9)) + if perception_stall >= _stall_limit: + if send_confirmed: + # the send registered: hand the parent a real done. The raw + # action-log proof (indices/coords) is machine-speak, kept out. done_called = True - done_message = "That's as far as I could get gathering this one." - break + done_message = "All set, your message went through and it's showing in the conversation now." + logger.info(f"[browser-agent {session_id}] ending: {perception_stall} post-send perception turns") + break + if not wrapup_nudged: + # don't cut it off mid-thought: ride a wrap-up nudge out on + # this turn's tool_results so next turn it answers via Done. + wrapup_nudged = True + wrapup_pending = True + perception_stall = 0 + logger.info(f"[browser-agent {session_id}] spin backstop: nudging to wrap up via Done") + else: + # Nudge already spent and still looping: stop on a clean line. + logger.info(f"[browser-agent {session_id}] ending: wrap-up nudge ignored, stopping") + if not done_called: + done_called = True + done_message = "That's as far as I could get gathering this one." + break else: perception_stall = 0 diff --git a/backend/apps/agents/browser/browser_schema.py b/backend/apps/agents/browser/browser_schema.py index f8518321..44a66c35 100644 --- a/backend/apps/agents/browser/browser_schema.py +++ b/backend/apps/agents/browser/browser_schema.py @@ -824,7 +824,12 @@ SYSTEM_PROMPT = ( "scraping burns turns and silently returns the wrong nodes. If one BrowserEvaluate read " "comes back empty or shaped wrong, STOP, that is your signal to switch to BrowserExtract, " "not to debug another selector. The answer must end up IN your Done message, so gather it " - "for real; never report done on a gather task you couldn't actually read.\n\n" + "for real; never report done on a gather task you couldn't actually read.\n" + "- Spanning MANY pages (get all N, every result)? Confirm the page shape ONCE, then COMMIT " + "to the sweep: don't re-verify each page works. The site usually exposes far fewer than a " + "round number asks (a '1000' is often ~15 pages); gather every page it does expose, then " + "Done with the full set and a one-line note on the real ceiling. Accumulate as you go so a " + "wrap-up nudge can always answer from what you already have.\n\n" "## When you genuinely cannot proceed\n" "Use RequestHumanIntervention for:\n" diff --git a/backend/tests/test_browser_agent_loop.py b/backend/tests/test_browser_agent_loop.py index f9a0f26f..98d6372a 100644 --- a/backend/tests/test_browser_agent_loop.py +++ b/backend/tests/test_browser_agent_loop.py @@ -97,6 +97,10 @@ def _install(monkeypatch, primary, aux): # a confirm/target probe embeds a non-empty `const spec="..."`; report it found found = "const spec=" in expr and 'const spec=""' not in expr return {"text": json.dumps({"ready": True, "quiet": 9999, "elems": 100, "found": found}), "url": DOC_URL} + # generic evaluate echoes its expression so distinct reads yield distinct + # results (lets a test exercise new-data-each-turn gather vs spinning) + if action == "evaluate": + return {"text": f"eval:{str(params.get('expression',''))[:120]}", "url": DOC_URL} if action == "list_interactives": # a non-irreversible label on purpose: Send/Submit-named steps are # refused by the replay send-gate, which has its own test below @@ -303,15 +307,37 @@ def test_send_shortcut_does_not_arm_on_a_gather_task(monkeypatch): assert "went through" not in result["summary"] +def test_gather_pulling_new_data_each_turn_is_not_nudged_early(monkeypatch): + # The Airbnb regression: a page-by-page gather (a fresh Extract returning NEW + # listings every turn) must NOT trip the spin backstop, gathering is the work, + # not spinning. Here 9 straight Extract turns each return distinct data; the run + # should keep going (no early wrap-up nudge) and finish on the model's own Done. + BH._browser_history.clear(); BH._domain_notes.clear() + primary = FakeLLM([ + # each turn reads a DIFFERENT page (distinct expression -> distinct result) + *[Resp([_rp(f"page {i}"), _tu("BrowserEvaluate", expression=f"parsePage({i})")]) for i in range(9)], + Resp([_tu("Done", message="Gathered all pages: 250 listings. Airbnb caps SF at ~15 pages.")]), + ]) + aux = FakeAux() + _install(monkeypatch, primary, aux) + result = asyncio.run(BA.run_browser_agent(task="find me all the airbnbs in sf", browser_id="b1", model="sonnet")) + # it ran the full gather (all 9 extract turns) and finished on its own Done, + # NOT cut short by a wrap-up nudge at turn 6 + assert primary.turn >= 9, f"gather cut short at turn {primary.turn} (new-data reads wrongly counted as spinning)" + assert "Gathered all pages" in result["summary"] + assert result.get("done") is True + + def test_spin_backstop_nudges_a_clean_wrapup_instead_of_a_midthought(monkeypatch): # The Airbnb mid-thought bug: a read-heavy run that trips the spin backstop must # get ONE wrap-up nudge to summarize via Done, not be cut off mid-sentence. The # final reply is the model's clean Done answer, and the nudge actually reached it. BH._browser_history.clear(); BH._domain_notes.clear() primary = FakeLLM([ - Resp([_rp("open the list"), _tu("BrowserClickIndex", index=3)]), # an action arms the backstop - *[Resp([_tu("BrowserScreenshot")]) for _ in range(6)], # 6 read turns trip it - Resp([_tu("Done", message="Here are the top repos: a, b, c")]), # obeys the wrap-up nudge + Resp([_rp("open the list"), _tu("BrowserClickIndex", index=3)]), # an action arms the backstop + # repeated identical screenshots (same result, no new data) = genuine spinning + *[Resp([_tu("BrowserScreenshot")]) for _ in range(10)], + Resp([_tu("Done", message="Here are the top repos: a, b, c")]), # obeys the wrap-up nudge ]) aux = FakeAux() _install(monkeypatch, primary, aux)