diff --git a/changes/402.fixed.md b/changes/402.fixed.md new file mode 100644 index 00000000..745ce2bc --- /dev/null +++ b/changes/402.fixed.md @@ -0,0 +1,3 @@ +**Browser click claims now stay grounded across a chat.** Sentient does not correct a user or another person's +click, or a sentence that already says no click happened. When it needs to correct its own unsupported browser-click +claim, it says plainly that nothing changed on the page and keeps the latest click outcome from the chat in context. diff --git a/sentient/agent/loop.py b/sentient/agent/loop.py index 6baa2505..d58e6dd7 100644 --- a/sentient/agent/loop.py +++ b/sentient/agent/loop.py @@ -297,6 +297,49 @@ def history_to_openai(rows: list[dict]) -> list[dict]: return fixed +def browser_click_history(rows: list[dict]) -> list[dict]: + """Completed browser-click outcomes in stored chat history, oldest first (#402).""" + calls: dict[str, dict] = {} + out: list[dict] = [] + for row in rows: + if row.get("role") == "assistant": + for call in _normalize_tool_calls(row.get("tool_calls")): + if call["function"]["name"] == "browser_click": + calls[call["id"]] = { + "name": "browser_click", + "target": _arguments(call["function"].get("arguments")).get("ref", ""), + } + continue + if row.get("role") != "tool": + continue + call = calls.pop(str(row.get("tool_call_id") or ""), None) + if call is None: + continue + try: + res = json.loads(str(row.get("content") or "")) + except (TypeError, ValueError): + # A short result may end after the stable leading fields. Preserve + # those fields rather than treating a recorded success as a failed + # click solely because its trailing URL was cut for model context. + content = str(row.get("content") or "") + res = {"ok": True} if re.search(r'"ok"\s*:\s*true\b', content) else {} + clicked = re.search(r'"clicked"\s*:\s*"((?:[^"\\]|\\.)*)"', content) + if clicked is not None: + with contextlib.suppress(ValueError): + res["clicked"] = json.loads(f'"{clicked.group(1)}"') + declined = isinstance(res, dict) and bool(res.get("declined")) + err = _error_text(res) + call.update( + declined=declined, + ok=isinstance(res, dict) and res.get("ok") is True and not err and outcomes.result_problem(res) is None, + uncertain=not declined and bool(err), + ) + if isinstance(res, dict) and res.get("clicked"): + call["target"] = str(res["clicked"]) + out.append(call) + return out + + def is_tool_format_error(exc: Exception) -> bool: """True when the provider rejected the request because the model cannot do tool calling (e.g. an old Ollama model template emitting malformed tool calls).""" @@ -392,6 +435,10 @@ class LoopResult: handed_off: list[dict] = field(default_factory=list) # the outcome checks of the calls that ran (#165, ``sentient.agent.outcomes``), in order; written by code only checks: list[dict] = field(default_factory=list) + # browser clicks attempted in this run, recorded by code so a final reply cannot invent one (#402) + browser_clicks: list[dict] = field(default_factory=list) + # completed browser clicks from earlier chat turns, consulted only for an explicit reference to one (#402) + browser_click_history: list[dict] = field(default_factory=list) def _hosts(raw: Any) -> set[str]: @@ -922,6 +969,12 @@ def interrupted() -> bool: result.from_findings = True log.info("%s: the model gave no answer after its tool calls; answering from the results", source) yield TextDelta(text=text_acc, **ev) + if note := outcomes.unmade_browser_click_note( + text_acc, result.browser_clicks, result.browser_click_history + ): + correction = f"\n\nCorrection: {note}" + text_acc += correction + yield TextDelta(text=correction, **ev) result.text = text_acc result.messages = messages return @@ -1054,6 +1107,12 @@ def interrupted() -> bool: result.hit_step_limit = True result.text = text_acc or "I reached the step limit before finishing. Tell me how to continue." + if note := outcomes.unmade_browser_click_note( + result.text, result.browser_clicks, result.browser_click_history + ): + correction = f"\n\nCorrection: {note}" + result.text += correction + yield TextDelta(text=correction, **ev) result.messages = messages async def _model_stream( @@ -1466,6 +1525,14 @@ async def _record( if ran and p.host: ctx.visited.add(p.host) err = _error_text(res) + if tc.name == "browser_click": + result.browser_clicks.append({ + "name": tc.name, + "declined": declined, + "ok": ran and not is_error and not err and outcomes.result_problem(res) is None, + "uncertain": ran and is_error, + "target": str(res.get("clicked") or tc.arguments.get("ref", "")) if isinstance(res, dict) else tc.arguments.get("ref", ""), + }) if ran and p.tool is not None and not ctx.untrusted: if brings_untrusted(p.tool): ctx.untrusted = untrusted_source(p.tool, self.registry) @@ -1726,6 +1793,7 @@ def release() -> list[str]: system, sources, notes = await self._system_prompt_with_sources(user_text, channel, session) history = await self.store.recent_messages(session_id, self.config.chat.history_window) convo = self.mark_outside_messages(history_to_openai(history)) + result.browser_click_history.extend(browser_click_history(history)) # Replace the just-stored user message with the rich version (attachments inlined). # Keep the stored user message unchanged. Add the current time to the diff --git a/sentient/agent/outcomes.py b/sentient/agent/outcomes.py index 3c359a18..4f973685 100644 --- a/sentient/agent/outcomes.py +++ b/sentient/agent/outcomes.py @@ -50,6 +50,20 @@ "home": re.compile(r"\b(turned|switched|set|done|ran)\b", re.I), } _CLAIM_ANY = re.compile(r"\b(done|completed|finished|success(?:fully)?|sent|saved|created|updated)\b", re.I) +_BROWSER_CLICK_CLAIM = re.compile(r"\b(?:clicked|was clicked)\b", re.I) +_NEGATED_BROWSER_CLICK = re.compile( + r"\b(?:not|didn't|hasn't|wasn't|haven't|never|nothing)\b[^.!?]{0,50}\bclicked\b", re.I +) +_OTHER_PERSON_BROWSER_CLICK = re.compile( + r"\b(?:you|he|she|they|someone|anyone|(?:my|your|his|her|their)\s+\w+)\s+" + r"(?:have\s+|had\s+|just\s+)?clicked\b", re.I +) +_ASSISTANT_BROWSER_CLICK = re.compile( + r"\b(?:i(?:'ve)?|we|sentient)\s+(?:(?:have|had|just|only)\s+)?clicked\b", re.I +) +_HYPOTHETICAL_BROWSER_CLICK = re.compile( + r"\b(?:if|would|could|might|may)\b[^.!?]{0,50}\b(?:clicked|was clicked|were clicked)\b", re.I +) # an MCP tool's kind from its name (servers declare nothing like it) _NAME_KINDS: list[tuple[str, re.Pattern, re.Pattern]] = [ @@ -293,6 +307,119 @@ def claimed(text: str, kind: str | None) -> bool: return bool((_CLAIMS.get(kind or "") or _CLAIM_ANY).search(text or "")) +def _claimed_click_target(text: str) -> str: + """The named target in an assistant or passive click claim, or empty for ``clicked it``.""" + match = _ASSISTANT_BROWSER_CLICK.search(text) + if match is not None: + tail = re.split(r"\b(?:on|and|but)\b|[,;.!?]", text[match.end():], maxsplit=1, flags=re.I)[0] + else: + passive = re.search(r"(?P.+?)\s+was clicked\b", text, re.I) + if passive is None: + return "" + tail = passive.group("target") + target = re.sub(r"^\s*(?:only|just|the|a|an)\s+", "", tail, flags=re.I).strip() + return "" if target.lower() in {"", "it", "that", "this", "button", "link"} else target + + +def _target_words(value: str) -> set[str]: + """Meaningful target words, without generic element labels.""" + return { + word for word in re.findall(r"[a-z0-9]+", value.lower()) + if word not in {"a", "an", "the", "button", "link", "only", "just"} and not re.fullmatch(r"e\d+", word) + } + + +def _matches_click_target(claim: str, click: dict) -> bool: + """Whether a named claim plausibly refers to this click's recorded browser element.""" + if not claim: + return True + + claim_words = _target_words(claim) + target_words = _target_words(str(click.get("target") or "")) + + def same_word(left: str, right: str) -> bool: + """Accept the small morphological variations common in a natural-language reply.""" + return left == right or ( + len(left) >= 6 and len(right) >= 6 and left[:6] == right[:6] + ) + + # The recorded element name is the dependable evidence. It need only appear + # in the claim: a reply naturally adds context such as "for the newsletter" + # or an element ref after naming the clicked control. + return bool(target_words) and all( + any(same_word(target_word, claim_word) for claim_word in claim_words) + for target_word in target_words + ) + + +def _click_note(clicks: list[dict], target: str) -> str: + """The correction for a click claim after selecting evidence for its named target.""" + matches = [click for click in clicks if not target or _matches_click_target(target, click)] + has_success = any(click.get("ok") for click in clicks) + if target and not matches and not has_success: + # A declined call may retain only its transient ref; it is still evidence that the named action was not made. + matches = [ + click for click in clicks + if not click.get("ok") and not _target_words(str(click.get("target") or "")) + ] + if target and not matches: + if has_success: + # A successful click of another named control makes an unmatched + # claim ambiguous. Do not turn an honest report into a correction. + return "" + return "I didn't click it, so nothing changed on the page." + if not matches: + return "I didn't click it, so nothing changed on the page." + click = matches[-1] + if click.get("ok"): + return "" + if click.get("declined"): + return "I didn't click it: you declined, so nothing changed on the page." + if click.get("uncertain"): + return "I couldn't confirm whether it was clicked." + return "I couldn't click it, so nothing changed on the page." + + +def unmade_browser_click_note(text: str, calls: list[dict], history: list[dict] | None = None) -> str: + """A short correction when a reply says it clicked although this turn did not. + + Small local models occasionally state that a browser click happened without making + the call at all, or after the user declined its approval. This is deliberately + narrow: it only corrects the completed form of ``click`` and only looks at calls + recorded by the loop, never at text supplied by a page or the user. + """ + current_clicks = [call for call in calls if call.get("name") == "browser_click"] + previous_clicks = [call for call in history or [] if call.get("name") == "browser_click"] + claims = re.split(r"(?<=[.!?])\s+", text or "") + # A reply can summarize a successful click from an earlier turn alongside a + # declined click from this one, so consider the complete chronological record. + clicks = previous_clicks + current_clicks + for claim in claims: + if _HYPOTHETICAL_BROWSER_CLICK.search(claim): + continue + explicit = list(_ASSISTANT_BROWSER_CLICK.finditer(claim)) + # Each self-attributed claim needs its own evidence. In particular, a + # successful "I clicked Subscribe" must not hide a later false + # "I clicked Delete account" in the same sentence. + explicit_parts = [ + claim[match.start(): explicit[i + 1].start() if i + 1 < len(explicit) else None] + for i, match in enumerate(explicit) + ] + for part in explicit_parts: + if note := _click_note(clicks, _claimed_click_target(part)): + return note + passive = ( + not explicit + and clicks + and _BROWSER_CLICK_CLAIM.search(claim) + and not _NEGATED_BROWSER_CLICK.search(claim) + and not _OTHER_PERSON_BROWSER_CLICK.search(claim) + ) + if passive and (note := _click_note(clicks, _claimed_click_target(claim))): + return note + return "" + + def _settled(failed: dict, later: list[dict]) -> bool: """A failed check counts as settled when a later successful call of the same tool covered what it missed. A failure that names nothing it acted on (a call with arguments the tool couldn't read) is settled only by the very diff --git a/tests/test_agent_loop.py b/tests/test_agent_loop.py index 1490d6d7..c5fb2679 100644 --- a/tests/test_agent_loop.py +++ b/tests/test_agent_loop.py @@ -1,6 +1,9 @@ +from sentient.agent import outcomes +from sentient.agent.loop import browser_click_history from sentient.app import SentientApp from sentient.llm.events import ApprovalRequest, Done, TextDelta, ToolResultEvent +from sentient.llm.provider import StreamChunk, ToolCall from tests.conftest import FakeProvider, tool_call @@ -79,6 +82,7 @@ class Postcards(ToolPlugin): sid = await s.store.create_session(channel="cli") async def deny(ev): + """Decline the browser-click approval shown by this test.""" if isinstance(ev, ApprovalRequest): assert ev.risk == "write" s.approvals.resolve(ev.approval_id, "deny") @@ -89,6 +93,7 @@ async def deny(ev): assert sent == [] async def allow(ev): + """Allow the postcard approval shown by this test.""" if isinstance(ev, ApprovalRequest): s.approvals.resolve(ev.approval_id, "allow_session") @@ -101,6 +106,194 @@ async def allow(ev): await s.stop() +async def test_reply_corrects_a_browser_click_it_never_made(config, isolated_home): + """A natural-language click claim needs evidence from a browser click call.""" + llm = FakeProvider(replies=["I clicked the subscription button. "]) + s = await SentientApp(config, llm=llm, db_path=isolated_home / "never-clicked.db").start() + try: + sid = await s.store.create_session(channel="cli") + events = await _collect(s.agent, sid, "click the subscription button") + text = "".join(e.text for e in events if isinstance(e, TextDelta)) + assert text == "I clicked the subscription button. \n\nCorrection: I didn't click it, so nothing changed on the page." + assert (await s.store.recent_messages(sid, 1))[-1]["content"] == text + finally: + await s.stop() + + +async def test_reply_corrects_a_browser_click_the_user_declined(config, isolated_home): + """A denied browser-click approval must remain visible in the final reply.""" + config.tools.approvals.mode = "ask" + llm = FakeProvider(replies=[[tool_call("browser_click", ref="e1")], "It was clicked, but the action was declined."]) + s = await SentientApp(config, llm=llm, db_path=isolated_home / "declined-click.db").start() + try: + sid = await s.store.create_session(channel="cli") + + async def deny(ev): + """Decline the browser-click approval shown by this test.""" + if isinstance(ev, ApprovalRequest): + s.approvals.resolve(ev.approval_id, "deny") + + events = await _collect(s.agent, sid, "click the subscription button", deny) + text = "".join(e.text for e in events if isinstance(e, TextDelta)) + assert text.endswith("Correction: I didn't click it: you declined, so nothing changed on the page.") + result = next(e for e in events if isinstance(e, ToolResultEvent)) + assert result.result["declined"] is True + finally: + await s.stop() + + +async def test_step_limit_corrects_a_declined_browser_click_claim(config, isolated_home): + """A streamed claim is corrected even when its tool call exhausts the final round.""" + class ClaimAndClick(FakeProvider): + async def stream(self, role, messages, tools=None, *, model=None): + """Stream a textual click claim and its tool call in one final round.""" + self.calls.append({"role": role, "messages": messages, "tools": tools, "model": model}) + yield StreamChunk(text="The subscription button was clicked.", model="fake") + yield StreamChunk(done=True, tool_calls=[ToolCall(**tool_call("browser_click", ref="e1"))], model="fake") + + config.models.max_tool_rounds = 1 + config.tools.approvals.mode = "ask" + s = await SentientApp(config, llm=ClaimAndClick(), db_path=isolated_home / "limit-click.db").start() + try: + sid = await s.store.create_session(channel="cli") + + async def deny(ev): + if isinstance(ev, ApprovalRequest): + s.approvals.resolve(ev.approval_id, "deny") + + events = await _collect(s.agent, sid, "click the subscription button", deny) + text = "".join(e.text for e in events if isinstance(e, TextDelta)) + assert text.endswith("Correction: I didn't click it: you declined, so nothing changed on the page.") + assert isinstance(events[-1], Done) and events[-1].content == text + finally: + await s.stop() + + +def test_browser_click_correction_uses_the_latest_call(): + """The latest known click outcome controls its correction wording.""" + assert outcomes.unmade_browser_click_note( + "The button was clicked.", [{"name": "browser_click", "ok": True, "declined": False}] + ) == "" + assert outcomes.unmade_browser_click_note( + "The button was clicked.", [ + {"name": "browser_click", "ok": True, "declined": False}, + {"name": "browser_click", "ok": False, "declined": True}, + ] + ) == "I didn't click it: you declined, so nothing changed on the page." + assert outcomes.unmade_browser_click_note( + "The button was clicked.", [{"name": "browser_click", "ok": False, "uncertain": True}] + ) == "I couldn't confirm whether it was clicked." + assert outcomes.unmade_browser_click_note("The button hasn't been clicked yet.", []) == "" + assert outcomes.unmade_browser_click_note("My friend clicked a link in a phishing email.", []) == "" + assert outcomes.unmade_browser_click_note("The subscription button was clicked.", []) == "" + assert outcomes.unmade_browser_click_note( + "I did not click A, but I clicked B.", [] + ) == "I didn't click it, so nothing changed on the page." + assert outcomes.unmade_browser_click_note( + "You clicked A, and I clicked B.", [] + ) == "I didn't click it, so nothing changed on the page." + prior_success = [{"name": "browser_click", "ok": True}] + assert outcomes.unmade_browser_click_note( + "I clicked the upgrade button.", [], prior_success + ) == "" + assert outcomes.unmade_browser_click_note("I clicked it.", [], prior_success) == "" + prior_subscribe = [{"name": "browser_click", "ok": True, "target": 'button "Subscribe"'}] + assert outcomes.unmade_browser_click_note("I clicked the Subscribe button.", [], prior_subscribe) == "" + assert outcomes.unmade_browser_click_note( + "I clicked the Delete account button.", [], prior_subscribe + ) == "" + assert outcomes.unmade_browser_click_note( + "The Subscribe button was clicked, but the action failed.", [], [{"name": "browser_click", "declined": True}] + ) == "I didn't click it: you declined, so nothing changed on the page." + assert outcomes.unmade_browser_click_note("If I clicked it, you would be charged $19.", []) == "" + assert outcomes.unmade_browser_click_note("I've clicked the button.", []) == "I didn't click it, so nothing changed on the page." + subscribe_then_declined_delete = [ + {"name": "browser_click", "ok": True, "target": 'button "Subscribe"'}, + {"name": "browser_click", "declined": True, "target": 'button "Delete account"'}, + ] + assert outcomes.unmade_browser_click_note( + "I clicked the Subscribe button.", subscribe_then_declined_delete + ) == "" + assert outcomes.unmade_browser_click_note( + "I clicked the Delete account button.", subscribe_then_declined_delete + ) == "I didn't click it: you declined, so nothing changed on the page." + assert outcomes.unmade_browser_click_note( + "I clicked the Subscribe button.", [], subscribe_then_declined_delete + ) == "" + assert outcomes.unmade_browser_click_note( + "I clicked the Delete account button.", [], subscribe_then_declined_delete + ) == "I didn't click it: you declined, so nothing changed on the page." + assert outcomes.unmade_browser_click_note( + "I clicked the account settings button.", [{"name": "browser_click", "ok": True, "target": 'button "Delete account"'}] + ) == "" + assert outcomes.unmade_browser_click_note( + "If it were clicked, you would be charged $19.", [{"name": "browser_click", "declined": True}] + ) == "" + successful_subscribe_then_declined_delete = [ + {"name": "browser_click", "ok": True, "target": 'button "Subscribe"'}, + {"name": "browser_click", "declined": True, "target": 'button "Delete account"'}, + ] + for reply in ( + 'I clicked the "Subscribe" button (e1) for the newsletter and attempted to click the "Delete account" button (e2).', + "The subscription button was clicked successfully.", + "Yes, I clicked the Subscribe button for the free weekly newsletter.", + 'Today I clicked just one thing: the "Subscribe" button on the Garden Notes page.', + ): + assert outcomes.unmade_browser_click_note(reply, successful_subscribe_then_declined_delete) == "" + assert outcomes.unmade_browser_click_note( + "Nothing was clicked.", successful_subscribe_then_declined_delete + ) == "" + assert outcomes.unmade_browser_click_note( + "Yes, I clicked the Subscribe button for the free weekly newsletter.", + [{"name": "browser_click", "declined": True, "target": 'button "Delete account"'}], + [{"name": "browser_click", "ok": True, "target": 'button "Subscribe"'}], + ) == "" + assert outcomes.unmade_browser_click_note( + "I clicked the Delete account button.", + [ + {"name": "browser_click", "ok": True, "target": 'button "Subscribe"'}, + {"name": "browser_click", "declined": True}, + ], + ) == "" + assert outcomes.unmade_browser_click_note( + "I clicked the Subscribe button and I clicked the Delete account button.", + successful_subscribe_then_declined_delete, + ) == "I didn't click it: you declined, so nothing changed on the page." + + +def test_browser_click_history_retains_a_truncated_successful_result(): + """A context-truncated result must not turn a recorded click into a false failure.""" + history = browser_click_history([ + {"role": "assistant", "tool_calls": [tool_call("browser_click", ref="e1")]}, + {"role": "tool", "tool_call_id": "call_browser_click", "content": + '{"ok": true, "clicked": "[e1] button \\"Subscribe\\"", "url": "https://example.test/'}, + ]) + assert history == [{"name": "browser_click", "target": '[e1] button "Subscribe"', "declined": False, + "ok": True, "uncertain": False}] + + +async def test_reply_uses_the_chat_history_for_a_declined_browser_click(config, isolated_home): + """A later unsupported click claim retains the preceding declined-click context.""" + config.tools.approvals.mode = "ask" + llm = FakeProvider(replies=[[tool_call("browser_click", ref="e1")], "I didn't click it: you declined.", "I clicked it."]) + s = await SentientApp(config, llm=llm, db_path=isolated_home / "prior-declined-click.db").start() + try: + sid = await s.store.create_session(channel="cli") + + async def deny(ev): + """Decline the browser-click approval shown by this test.""" + if isinstance(ev, ApprovalRequest): + s.approvals.resolve(ev.approval_id, "deny") + + first = await _collect(s.agent, sid, "click the subscription button", deny) + assert "Correction:" not in "".join(e.text for e in first if isinstance(e, TextDelta)) + second = await _collect(s.agent, sid, "did it work?") + text = "".join(e.text for e in second if isinstance(e, TextDelta)) + assert text.endswith("Correction: I didn't click it: you declined, so nothing changed on the page.") + finally: + await s.stop() + + async def test_internal_tools_do_not_prompt_in_ask_mode(config, isolated_home): config.tools.approvals.mode = "ask" llm = FakeProvider(replies=[[tool_call("file_write", name="note.txt", content="hi")], "saved"]) diff --git a/tests/test_gateway.py b/tests/test_gateway.py index a5a4bfc9..fed8747d 100644 --- a/tests/test_gateway.py +++ b/tests/test_gateway.py @@ -50,6 +50,27 @@ def turn(ws, text): assert "tool_call" in second and "tool_result" in second +def test_websocket_delivers_browser_click_correction(config, isolated_home): + """The final WebSocket message includes corrections appended after model text streamed.""" + app = create_app(SentientApp( + config, llm=FakeProvider(replies=["I clicked the subscription button."]), db_path=isolated_home / "ws.db" + )) + with TestClient(app) as client: + token = client.app.state.token + with client.websocket_connect(f"/ws?token={token}") as ws: + assert ws.receive_json()["type"] == "hello" + ws.send_json({"type": "chat.send", "text": "click the subscription button"}) + messages = [] + while True: + message = ws.receive_json() + messages.append(message) + if message["type"] == "done": + break + correction = "Correction: I didn't click it, so nothing changed on the page." + assert messages[-1]["content"].endswith(correction) + assert any(message["type"] == "text_delta" and correction in message.get("text", "") for message in messages) + + def test_websocket_rejects_bad_token(client): with pytest.raises(WebSocketDisconnect), client.websocket_connect("/ws?token=nope") as ws: ws.receive_json()