[eric] browser: record the navigation prefix instead of discarding the whole run

backend/apps/agents/browser/browser_skills.py

@@ -226,6 +226,7 @@ def distill_steps(action_log: list[dict]) -> list[dict]:
     or [] if it can't be made safely replayable."""
     steps: list[dict] = []
     productive_count = 0
+    p_truncated = False
 
     def p_emit_simple(tool, inp):
         nonlocal productive_count
@@ -269,12 +270,19 @@ def distill_steps(action_log: list[dict]) -> list[dict]:
                 if st == "click_index":
                     name = (r or {}).get("clicked_name")
                     if not name:
-                        return []  # index clicks need a re-resolvable identity
+                        # Same truncation as the un-batched path below, but two loops deep, so it
+                        # needs a flag: a bare break here would only leave the sub-action loop and
+                        # the outer scan would carry on past the step we could not name.
+                        p_truncated = True
+                        break
                     steps.append({"tool": "BrowserClickByName", "params": {"role": (r or {}).get("clicked_role", ""), "name": name}})
                     productive_count += 1
                     continue
                 if not p_emit_simple(st, sp):
-                    return []
+                    p_truncated = True
+                    break
+            if p_truncated:
+                break
             continue
         if tool == "BrowserNavigate" and inp.get("url"):
             steps.append({"tool": "BrowserNavigate", "params": {"url": inp["url"]}})
@@ -284,7 +292,16 @@ def distill_steps(action_log: list[dict]) -> list[dict]:
         elif tool == "BrowserClickIndex":
             name = a.get("clicked_name")
             if not name:
-                return []
+                # STOP here and keep what came before, rather than throwing the run away. Discarding
+                # everything is why this layer recorded 0 skills in 55 attempts across 7MB of real
+                # logs: browser_send_script appends its clicks without `clicked_name` (it puts the
+                # element name in result_summary as prose), and it runs on every fast-path write, so
+                # one un-nameable click deleted the navigate and the type that reached the page.
+                # What survives is a navigation PREFIX, which is the half worth replaying anyway:
+                # the write itself is the send script's job and its payload must never be baked into
+                # a skill, or replay re-posts last week's text.
+                p_truncated = True
+                break
             steps.append({"tool": "BrowserClickByName", "params": {"role": a.get("clicked_role", ""), "name": name}})
             productive_count += 1
         elif tool == "BrowserClick" and inp.get("selector"):
@@ -296,6 +313,13 @@ def distill_steps(action_log: list[dict]) -> list[dict]:
         elif tool == "BrowserScroll":
             steps.append({"tool": "BrowserScroll", "params": {k: inp[k] for k in ("direction", "amount") if k in inp}})
             productive_count += 1
+    if p_truncated:
+        # A truncated run is a NAVIGATION macro and nothing else. Whatever text was typed before the
+        # un-nameable click is the payload of that one run, and replaying it would re-post last
+        # week's message; getting to the page is the reusable part, the write belongs to the send
+        # script every time. Dropping the fill is what makes recording a prefix safe at all.
+        steps = [s for s in steps if s["tool"] not in ("BrowserType", "BrowserPressKey")]
+        productive_count = sum(s["tool"] != "BrowserNavigate" for s in steps)
     if productive_count == 0:
         return []
     return p_prune_detours(steps)

backend/tests/test_browser_skills.py

@@ -60,8 +60,8 @@ def test_distill_refuses_click_without_resolved_name():
     assert sk.distill_steps(log) == []
 
 
-def test_one_unnamed_click_throws_away_every_other_step_in_the_run():
-    """Why the skill layer has recorded exactly zero skills in production.
+def test_an_unnameable_click_truncates_the_run_into_a_navigation_prefix():
+    """Why the skill layer recorded exactly zero skills in production, and what it does instead now.
 
     The rule above is right on its own, but its blast radius is the whole run, and the clicks that
     browser_send_script appends to the action log carry no `clicked_name`: it puts the element's
@@ -71,21 +71,29 @@ def test_one_unnamed_click_throws_away_every_other_step_in_the_run():
 
     Measured across 104 sweep logs / 7MB of real runs: 55 record attempts, 43 of them holding a
     productive action, 0 skills recorded, 0 replays, 244 lookups that found nothing.
+
+    So recording now STOPS at the un-nameable click and keeps what came before, which is a pure
+    navigation macro. That split is the point: getting to the page is reusable, the write is not.
     """
     log = [
         {"tool": "BrowserNavigate", "input": {"url": "https://x.com/home"}, "ok": True},
+        {"tool": "BrowserClickIndex", "input": {"index": 3}, "ok": True,
+         "clicked_name": "Post", "clicked_role": "button"},
         {"tool": "BrowserType", "input": {"selector": "#c", "text": "hi"}, "ok": True},
         {"tool": "BrowserClickIndex", "input": {"index": 7, "text": "hi"}, "ok": True,
          "result_summary": "script fill into 'Post text'"},
     ]
-    assert sk.distill_steps(log) == [], "if this ever records, read the payload warning below"
-    # The trap for whoever fixes it: a click carrying TEXT distills into BrowserClickByName, which
-    # has no text parameter, so a replayed send would click the composer, type nothing, and submit.
-    named = dict(log[-1], clicked_name="Post text", clicked_role="textbox")
-    steps = sk.distill_steps(log[:2] + [named])
-    assert "hi" not in str(steps[-1]["params"]), (
-        "the payload is dropped on the way into a replayable step; naming the click without fixing "
-        "this is how a skill replays an empty post")
+    steps = sk.distill_steps(log)
+    assert [s["tool"] for s in steps] == ["BrowserNavigate", "BrowserClickByName"], steps
+
+    # THE SAFETY PROPERTY, and the reason a prefix is safe to record where a whole run was not: the
+    # typed payload must never survive into a skill. It is one run's text; replaying it re-posts
+    # last week's message, and a BrowserClickByName carrying it would type nothing and still submit.
+    assert "hi" not in str(steps), "a recorded skill must never carry the payload of the run"
+
+    # And a run whose ONLY productive step is the un-nameable click still records nothing, because
+    # a bare navigate is not a skill worth replaying.
+    assert sk.distill_steps(log[:1] + log[2:]) == []
 
 
 def test_distill_skips_navigate_only():
@@ -117,7 +125,10 @@ def test_distill_flattens_browser_batch():
 
 
 def test_distill_bails_on_batched_click_index():
-    # a batched click_index can't be made robust (resolved name not recoverable)
+    # A batched click_index can't be made robust (resolved name not recoverable), so recording stops
+    # there. Nothing usable precedes it here (only the fill, which a prefix must never carry), so the
+    # result is still empty; what changed is the REASON, from "discard the run" to "keep the prefix,
+    # and this run has no prefix".
     log = [
         {"tool": "BrowserBatch", "ok": True, "input": {"actions": [
             {"type": "type", "params": {"selector": "#m", "text": "x"}},