Skip to content

Commit aad6b4b

Browse files
fix(agent): the second look's reason for a task's code step matches the task (#517)
* fix(agent): the second look's reason for a task's code step matches the task In a task run the second look's model check read the latest 1500 characters of the executor's system prompt as the user's words. Those are Sentient's own instructions ("If a tool fails, analyze the error..."), so a held code step gave reasons that had nothing to do with the task. The executor now hands the check the task's request in plain words (its objective, the user's own words, the approved plan and checklist answers); the deterministic check and the hold itself are unchanged. Closes #455 * fix(agent): keep a routine's confirmed answers in the second look's task request Confirmed checklist answers now come before the plan, and plan steps are added only while they fit, so a long plan never cuts them off.
1 parent 6716b21 commit aad6b4b

5 files changed

Lines changed: 180 additions & 6 deletions

File tree

‎changes/455.fixed.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
**When a task's step is held for a second look, the reason now matches what you asked the task to do.** The check
2+
used to read the task's built-in instructions as your words, so it could say things like "the user asked to analyze
3+
an error". It now compares the step with the task you set up and the plan you approved. Steps are held exactly as
4+
before.

‎sentient/agent/second_look.py‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,10 @@
1717
(``Tool.destinations_fn``); one that a Reply-To header redirects to another address is flagged too.
1818
2. ``model_check`` (only for effective risk ``send`` or ``exec``, and only when the call would otherwise run without
1919
asking or the run read outside content): one short, tool-free prompt on the ``fast`` role that sees only the
20-
user's request, the rules and the action's arguments, never the outside content, and answers whether the action
21-
matches what was asked. No usable answer (an error, a refusal, a timeout, unparseable JSON) counts as a mismatch:
22-
it fails closed. ``tools.approvals.second_look_model`` turns this part off; the deterministic part always runs.
20+
user's request (in a task run, the task's brief in plain words, ``TASK_REQUEST_KEY``), the rules and the action's
21+
arguments, never the outside content, and answers whether the action matches what was asked. No usable answer
22+
(an error, a refusal, a timeout, unparseable JSON) counts as a mismatch: it fails closed.
23+
``tools.approvals.second_look_model`` turns this part off; the deterministic part always runs.
2324
2425
A warning only ever adds a question, never removes one (AGENTS.md: a model may raise a risk, never lower it). The
2526
agent loop raises the call to at least ``send``, asks whatever the approvals mode, Allow rules, "Allow for this
@@ -74,6 +75,10 @@
7475
# what such a run's user asked, in Sentient's words (it holds no outside text): the model check reads it, so a reply
7576
# the email asked for isn't flagged as "not requested"
7677
BRIEF_REQUEST = "Do what the email I forwarded asks."
78+
# ``ToolContext.extra`` key: what the user asked a task to do, in plain words (its objective and approved plan, set by
79+
# the task executor). The model check reads it instead of the executor's system prompt, whose own instructions ("If a
80+
# tool fails, analyze the error") it otherwise took for the user's words (#455). Never used for a brief from an email.
81+
TASK_REQUEST_KEY = "task_request"
7782

7883
_EMAIL = re.compile(r"[A-Za-z0-9._%+\-]+@[A-Za-z0-9\-]+(?:\.[A-Za-z0-9\-]+)*\.[A-Za-z]{2,}")
7984
_PHONE = re.compile(r"^\+?[\d\s().\-]{7,24}$")
@@ -754,6 +759,16 @@ def evidence(self) -> Evidence:
754759
)
755760
return self._evidence
756761

762+
def request(self) -> str:
763+
"""What the model check reads as the user's words: a task run's brief in plain words when the run has one
764+
(``TASK_REQUEST_KEY``, #455), else the request ``gather`` found. The deterministic check keeps reading all of
765+
the run's own words."""
766+
extra = getattr(self.ctx, "extra", None) or {}
767+
brief = str(extra.get(TASK_REQUEST_KEY) or "").strip()
768+
if brief and extra.get("task_id") and not extra.get(BRIEF_FROM_KEY):
769+
return brief
770+
return self.evidence().request
771+
757772
async def destinations(self) -> list[dict]:
758773
"""Who the call goes to when its arguments don't say (a reply: the thread's headers, #144), also kept on the
759774
plan so the approval card and the undo window show it. [] when the tool can't tell; the call itself then
@@ -798,8 +813,7 @@ async def model(self) -> str:
798813
if wording.get("target"):
799814
action += f" ({wording['target']})"
800815
warning = await model_check(
801-
self.agent.llm, self.evidence().request, action, self.arguments, attended=self.attended,
802-
model=self.model_name,
816+
self.agent.llm, self.request(), action, self.arguments, attended=self.attended, model=self.model_name,
803817
)
804818
if warning:
805819
log.info("second look: %s flagged (model): %s", self.tool.name, warning)

‎sentient/tasks/executor.py‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929

3030
from sentient.agent import dry_run, outcomes, preview, rule_levels
3131
from sentient.agent.loop import ASK_FIRST_KEY, CAN_PAUSE_KEY, Budget, LoopResult, history_to_openai
32-
from sentient.agent.second_look import BRIEF_FROM_KEY
32+
from sentient.agent.second_look import BRIEF_FROM_KEY, TASK_REQUEST_KEY
3333
from sentient.agent.sources import AnswerSources
3434
from sentient.llm.events import TextDelta, ThinkingDelta, ToolCallEvent, ToolResultEvent, Usage
3535
from sentient.llm.provider import ToolCall
@@ -44,6 +44,7 @@
4444
RESULT_GENERATOR_SYSTEM_PROMPT,
4545
RESUME_NOTE,
4646
build_executor_prompt,
47+
build_second_look_request,
4748
)
4849
from sentient.tasks.schedule import get_tz
4950
from sentient.tools.base import Risk, effective_risk
@@ -425,6 +426,11 @@ async def alive(event: Any) -> None:
425426
ctx.extra[ASK_FIRST_KEY] = True
426427
if email_inbox.is_email_task(task): # its brief was written from an email, not by the user (#159)
427428
ctx.extra[BRIEF_FROM_KEY] = email_inbox.app_name(task)
429+
else: # what the second look's model check compares a send or a code run with (#455)
430+
ctx.extra[TASK_REQUEST_KEY] = build_second_look_request(
431+
name=task.get("name") or "", description=task.get("description") or "",
432+
original_prompt=task.get("original_prompt") or "", plan=plan, confirmed=checklist.prompt_lines(task),
433+
)
428434
checkpoint = run.get("messages") if resume else None
429435
if isinstance(checkpoint, list) and checkpoint:
430436
if svc.holding(run_id): # paused before an approved call ran: it runs once the task is resumed

‎sentient/tasks/prompts.py‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -495,6 +495,38 @@ def build_executor_prompt(
495495
)
496496

497497

498+
def _one_line(text: Any, limit: int) -> str:
499+
text = " ".join(str(text or "").split())
500+
return text if len(text) <= limit else text[: limit - 3].rstrip() + "..."
501+
502+
503+
def build_second_look_request(
504+
*, name: str, description: str, original_prompt: str = "", plan: list[dict], confirmed: str = "",
505+
limit: int = 1500,
506+
) -> str:
507+
"""What the user asked a task to do, in their voice and plain words, for the second look's model check (#455).
508+
509+
The rest of the executor's system prompt is Sentient's own instructions ("If a tool fails, analyze the error"),
510+
so read as the user's words it gave reasons like "the user asked to analyze an error". The objective comes
511+
first, then what the user confirmed, then as many whole plan steps as fit under ``limit``."""
512+
objective = name if not description or description == name else f"{name} - {description}"
513+
lines = [f"My task: {_one_line(objective, 500)}"]
514+
said = _one_line(original_prompt, 500)
515+
if said and said.lower() not in objective.lower(): # the words the user typed, when the task was reworded
516+
lines.append(f"In my words: {said}")
517+
# what the user confirmed in a routine's checklist, without the executor prompt's heading
518+
lines += [_one_line(line, 200) for line in confirmed.splitlines() if line.strip().startswith("- ")]
519+
steps = [s for s in plan or [] if isinstance(s, dict) and str(s.get("description") or "").strip()]
520+
if steps:
521+
lines.append("The plan I approved:")
522+
for i, step in enumerate(steps[:8], 1):
523+
line = f"{i}. {_one_line(step['description'], 200)}" + (f" (with {step['tool']})" if step.get("tool") else "")
524+
if len("\n".join([*lines, line])) > limit:
525+
break
526+
lines.append(line)
527+
return "\n".join(lines)[:limit]
528+
529+
498530
async def build_tool_catalog(app: SentientApp) -> dict[str, str]:
499531
"""``{plugin_id: "Display name: when to use. Tools: a, b"}`` for usable plugins.
500532
Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
"""What the second look's model check reads in a task run (#455): the task's own request, in plain words.
2+
3+
It used to read the latest 1500 characters of the executor's system prompt as "the user said", which are Sentient's
4+
instructions ("If a tool fails, analyze the error..."), so a flagged code step gave reasons like "the user asked to
5+
analyze an error". A held call still asks exactly as before.
6+
"""
7+
8+
from __future__ import annotations
9+
10+
from types import SimpleNamespace
11+
12+
from sentient.agent import second_look as sl
13+
from sentient.tasks import email_inbox
14+
from sentient.tasks.prompts import build_second_look_request
15+
from sentient.tools.base import Risk, ToolContext, ToolPlugin, tool
16+
from tests.conftest import FakeProvider, tool_call
17+
from tests.tasks.conftest import json_calls
18+
19+
NAME = "Prime squares"
20+
DESCRIPTION = "Add up the squares of the first 500 prime numbers and tell me the total."
21+
PROMPT = "work out the sum of the squares of the first 500 primes for me"
22+
STEP = "Run a short Python script that adds up the squares of the first 500 primes and prints the total"
23+
RUN = [tool_call("runner_run", code="print(sum(p * p for p in primes(500)))")]
24+
25+
26+
def _runner(log: list[str]) -> ToolPlugin:
27+
@tool("runner_run", risk=Risk.exec)
28+
async def runner_run(ctx: ToolContext, code: str) -> dict:
29+
"""Run a Python script."""
30+
log.append(code)
31+
return {"output": "1"}
32+
33+
class Runner(ToolPlugin):
34+
id = "runner"
35+
display_name = "Runner"
36+
tools = [runner_run]
37+
38+
return Runner()
39+
40+
41+
async def _held_run(make_app, config, **fields) -> tuple[FakeProvider, dict, list[str]]:
42+
config.tools.approvals.second_look_model = True
43+
log: list[str] = []
44+
reason = "The user asked for the sum of squares of primes, but this runs code"
45+
llm = FakeProvider(replies=[RUN, "Waiting for the OK."], json_replies=[{"match": False, "reason": reason}])
46+
app = await make_app(llm)
47+
app.registry.register(_runner(log))
48+
now = app.tasks.now_iso()
49+
task_id = await app.tasks.repo.insert_task({
50+
"name": NAME, "description": DESCRIPTION, "original_prompt": PROMPT, "status": "approval_pending",
51+
"schedule": {"type": "once", "run_at": None}, "plan": [{"tool": "runner", "description": STEP}],
52+
"created_at": now, "updated_at": now, **fields,
53+
})
54+
await app.tasks.approve(task_id)
55+
await app.tasks.drain()
56+
return llm, (await app.tasks.get(task_id))["runs"][-1], log
57+
58+
59+
async def test_a_flagged_code_step_is_checked_against_the_tasks_request(make_app, config):
60+
llm, run, log = await _held_run(make_app, config)
61+
checks = json_calls(llm, "fast")
62+
assert len(checks) == 1
63+
said = checks[0]["messages"][1]["content"].split("The assistant now wants", 1)[0]
64+
assert f"My task: {NAME} - {DESCRIPTION}" in said
65+
assert f"In my words: {PROMPT}" in said
66+
assert f"1. {STEP} (with runner)" in said
67+
# none of the executor's own instructions read as the user's words
68+
for engine_words in ("analyze the error", "EXECUTION STRATEGY", "Begin executing the plan", "Step 1"):
69+
assert engine_words not in said
70+
# the hold itself is unchanged: nothing ran, the run asks, with the reason first
71+
assert log == [] and run["status"] == "waiting_for_user"
72+
question = run["pending_question"]["question"]
73+
assert question.startswith("A second look found this may not match what you asked: The user asked for the sum")
74+
assert 'This task wants to use "Run" in Runner' in question
75+
76+
77+
async def test_a_task_by_email_still_never_reads_its_brief_as_the_users_words(make_app, config, monkeypatch):
78+
monkeypatch.setattr(email_inbox, "is_email_task", lambda task: True)
79+
monkeypatch.setattr(email_inbox, "app_name", lambda task: "Email (IMAP)")
80+
llm, run, log = await _held_run(make_app, config)
81+
said = json_calls(llm, "fast")[0]["messages"][1]["content"].split("The assistant now wants", 1)[0]
82+
assert sl.BRIEF_REQUEST in said and DESCRIPTION not in said and "My task:" not in said
83+
assert log == [] and run["status"] == "waiting_for_user"
84+
85+
86+
def test_the_request_puts_the_objective_first_and_stays_short():
87+
steps = [{"tool": "web", "description": "Read the page " + "x" * 400}] * 12
88+
confirmed = "**What the user confirmed for this task:**\n- The user expects: a short list\n\n"
89+
text = build_second_look_request(
90+
name="Studio events", description="Email me new studio events", original_prompt="Email me new studio events",
91+
plan=steps, confirmed=confirmed,
92+
)
93+
assert text.startswith("My task: Studio events - Email me new studio events\n- The user expects: a short list\n"
94+
"The plan I approved:\n1. Read the page")
95+
assert "In my words" not in text # the same words as the description are not repeated
96+
assert len(text) <= sl.MAX_REQUEST and "9. " not in text # whole steps only, as many as fit, each cut short
97+
assert text.splitlines()[-1].endswith("... (with web)")
98+
# a long objective, long words and a long plan still keep what the user confirmed
99+
long = build_second_look_request(name="N" * 600, description="", original_prompt="P" * 600, plan=steps,
100+
confirmed=confirmed)
101+
assert "\n- The user expects: a short list\n" in long and len(long) <= sl.MAX_REQUEST
102+
short = build_second_look_request(name="Digest", description="", plan=[], confirmed=confirmed)
103+
assert short == "My task: Digest\n- The user expects: a short list"
104+
105+
106+
def test_the_brief_is_used_only_in_a_task_run_and_never_for_a_brief_from_email():
107+
agent = SimpleNamespace(registry=None, config=SimpleNamespace(assistant=SimpleNamespace(user_name="Maya")))
108+
messages = [{"role": "system", "content": "**Primary Objective:** 'Email Arjun the notes'"},
109+
{"role": "user", "content": "Begin."}]
110+
111+
def request(extra: dict) -> str:
112+
return sl.Look(agent, None, None, SimpleNamespace(extra=extra), None, messages).request()
113+
114+
assert request({"task_id": "t1", sl.TASK_REQUEST_KEY: "My task: Email Arjun"}) == "My task: Email Arjun"
115+
assert "Email Arjun the notes" in request({"task_id": "t1"}) # no brief: what gather found, as before
116+
assert request({sl.TASK_REQUEST_KEY: "My task: x"}) != "My task: x" # not a task run
117+
assert request({"task_id": "t1", sl.TASK_REQUEST_KEY: "My task: x", sl.BRIEF_FROM_KEY: "Gmail"}).startswith(
118+
sl.BRIEF_REQUEST)

0 commit comments

Comments
 (0)