From 8994b3fb4b1784412c9a3647f53a8c62467c7de3 Mon Sep 17 00:00:00 2001 From: Don Garrett Date: Wed, 7 Oct 2026 18:01:51 +0000 Subject: [PATCH] Conversation on the new loop (M2.7) - Questions never change state (L5, #77): a message ending in "?" to a waiting project item starts a question run (reply and look-only tools, an instruction to change nothing), and the item goes back to the very wait it was in: a restatement up stays up and a go binds to it; the agent's own question stays open. - Explanations: a restatement gives its reasons and the main alternative; a revision says what changed (restate(changed), sent back without it); a milestone's summary says what was done, verified and left open. - Progress in prose (#42): the driver's and a helper's own words for a turn are posted, trimmed, at most once a minute per item; `details` shows the last tool calls; a message arriving mid-run is acknowledged at once ("Noted; ..."). - CI: the e2e job's comments and the plan no longer claim a newer push cancels the run it supersedes (Gitea here doesn't); the concurrency block goes. Tests: conversation unit tests (questions in a go-ahead and an answer wait, an unanswerable question, revisions, progress rate and trimming, details, mid-run notes), the helper's progress, the walk asking questions in every state with a questions_change_nothing property (routing questions the old way fails 32 of 48 seeds), and an e2e question while a restatement is up. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01PPQ14L3LikyefCMFBqEbKE --- .gitea/workflows/ci.yml | 13 +- docs/Development-Plan.md | 20 ++- nisse-dev/src/nisse_dev/delegation.py | 8 ++ nisse-dev/src/nisse_dev/loop/agent.py | 157 ++++++++++++++++++++- nisse-dev/src/nisse_dev/loop/driver.py | 9 +- nisse-dev/src/nisse_dev/loop/items.py | 5 + nisse-dev/src/nisse_dev/loop/tools.py | 7 +- nisse-dev/tests/test_loop.py | 4 +- nisse-dev/tests/test_loop_help.py | 23 +++ nisse-dev/tests/test_loop_labels.py | 15 +- nisse-dev/tests/test_loop_multi.py | 2 +- nisse-dev/tests/test_loop_properties.py | 28 +++- nisse-dev/tests/test_loop_talk.py | 163 ++++++++++++++++++++++ tests/integration/test_end_to_end_loop.py | 32 +++++ 14 files changed, 456 insertions(+), 30 deletions(-) create mode 100644 nisse-dev/tests/test_loop_talk.py diff --git a/.gitea/workflows/ci.yml b/.gitea/workflows/ci.yml index 6129e3a..9d2fd61 100644 --- a/.gitea/workflows/ci.yml +++ b/.gitea/workflows/ci.yml @@ -58,18 +58,15 @@ jobs: e2e: # The end-to-end suite (scripts/e2e.sh): the whole agent against real # services, every role on a local model, so it spends nothing but keeps - # the RTX 4090 busy for ~25 minutes. On PRs only, as its own job so the - # checks above report first; a newer push to the same PR cancels the - # run it supersedes. Locally, run only what you're writing (-k) or - # diagnosing (--lf). Only after the checks pass: a PR that's red already - # doesn't spend the GPU. + # the RTX 4090 busy for ~25 minutes. On PRs only, and only after the + # checks pass: a PR that's red already doesn't spend the GPU. There is + # one runner and Gitea doesn't honour cancel-in-progress, so a run a + # newer push supersedes holds the runner until it's cancelled by hand. + # Locally, run only what you're writing (-k) or diagnosing (--lf). if: github.event_name != 'push' needs: checks runs-on: ubuntu-latest timeout-minutes: 90 - concurrency: - group: e2e-${{ github.head_ref || github.ref }} - cancel-in-progress: true steps: - uses: actions/checkout@v4 diff --git a/docs/Development-Plan.md b/docs/Development-Plan.md index 17ea9e4..6efe6c8 100644 --- a/docs/Development-Plan.md +++ b/docs/Development-Plan.md @@ -76,8 +76,9 @@ Only `nisse-dev` has real code; `nisse-core` and `nisse-ops` are placeholders. - **The end-to-end test runs on every PR** (M1.6): the real agent, every role on a local model (so it costs nothing per call), a repository of the run's own and a real build host, driven by a scripted operator. CI runs it as its own job on pull - requests, cancelling a run a newer push supersedes. Locally only the tests being - written (`-k`) or diagnosed (`--lf`); `scripts/e2e.sh` reports what a run spent. Manual + requests, after the checks pass. There is one runner, and a run a newer push + supersedes has to be cancelled by hand. Locally only the tests being written + (`-k`) or diagnosed (`--lf`); `scripts/e2e.sh` reports what a run spent. Manual live runs are for the new loop's first runs and phase ends. Findings are filed as issues and either fixed before moving on or deliberately deferred. - **The Quint models move with the loop:** `orchestrator.qnt` while `control.py` @@ -240,8 +241,19 @@ deletes afterwards. - Evaluation capture: delegations, `ask_expert` and the reader's verdicts. - Deferred: the context-won't-fit trigger (L6); the action classifier (M3.2). -**M2.7 Conversation.** *Pre-approved.* Explanations, progress in prose, asking -mid-work (L5, #42, #77). +**M2.7 Conversation.** *Pre-approved.* **Done** (this PR). Explanations, progress +in prose, asking mid-work (L5, #42, #77). +- Questions never change state: a message ending in `?` to a waiting project item + is answered in a question run (`reply` and look-only tools), and the item goes + back to the very wait it was in, so a restatement up stays up and a go binds to + it. +- A restatement gives its reasons and the main alternative; a revision says what + changed (`restate(changed)`, sent back without it). +- Progress: the driver's and a helper's own words for a turn are posted, trimmed, + at most once a minute per item; `details` shows the last tool calls. A message + arriving mid-run is acknowledged at once. +- Deferred: streaming (#42); the question property in `specs/loop.qnt` (the walk + checks it against the code). **M1.4 Projects** (D8). *Pre-approved.* **Done** (this PR). - The project entry in configuration: repository, default branch, build VM, label diff --git a/nisse-dev/src/nisse_dev/delegation.py b/nisse-dev/src/nisse_dev/delegation.py index 59e83f6..4c908e1 100644 --- a/nisse-dev/src/nisse_dev/delegation.py +++ b/nisse-dev/src/nisse_dev/delegation.py @@ -173,6 +173,7 @@ class Delegation: now: Callable[[], float] = time.monotonic, cancel: Cancel | None = None, reader: str = "", + on_turn: Callable[[str], None] | None = None, ) -> None: self._client = client self._store = store @@ -180,6 +181,8 @@ class Delegation: # Who acts on the handback, from its capability profile (L9): never # a model's name. self._reader = reader or "a materially weaker model" + # The delegate's own text for each turn, for progress (#42). + self._on_turn = on_turn self._git = git_repo self._model = model self._backing_model = backing_model @@ -400,6 +403,11 @@ class Delegation: transcript.append(event) if isinstance(event, ToolCall): calls.append(event) + said = " ".join( + e.text.strip() for e in produced if isinstance(e, AssistantText) + ).strip() + if said and self._on_turn is not None: + self._on_turn(said) if not calls: # A delegate that stops without handing back has not reported, diff --git a/nisse-dev/src/nisse_dev/loop/agent.py b/nisse-dev/src/nisse_dev/loop/agent.py index eedb357..18ad3aa 100644 --- a/nisse-dev/src/nisse_dev/loop/agent.py +++ b/nisse-dev/src/nisse_dev/loop/agent.py @@ -29,6 +29,8 @@ from nisse_dev.canonical import ( DelegationStarted, Fact, PlanSubmitted, + ToolCall, + ToolResult, UserMessage, ) from nisse_dev.config import GENERAL @@ -87,6 +89,9 @@ _NAMED = re.compile(r"(go|approve|abandon|stop) (?:([a-z0-9][a-z0-9-]*))?#(\d+)" _NEEDS_ITEM = frozenset({"go", "approve", "abandon"}) BUG_GRACE_S = 600.0 GENERAL_IDLE_S = 86400.0 +PROGRESS_EVERY_S = 60.0 # progress in prose at a readable rate (L5, #42) +PROGRESS_CHARS = 400 +DETAILS = 10 # tool calls shown by `details` FILE_ISSUE = "file_issue" # the intent of opening a record issue (L8) # Carried by a record issue the agent opens for work started in conversation: # intake never takes it for new work, even before its item exists (D1). @@ -102,9 +107,14 @@ The issue and the conversation about it are below. Before any work, find out what you need, then use restate to put your understanding to the operator: what you take the request to mean, how it fits -the project, and your first step. Wait for their go-ahead. If the request is -unclear, ask_human instead of guessing: a restatement asks no questions, so -the operator can agree to it as it stands. +the project, your first step, and why that way rather than the main +alternative. A later restatement says what changed. Wait for their go-ahead. +If the request is unclear, ask_human instead of guessing: a restatement asks +no questions, so the operator can agree to it as it stands. At any point, +asking is better than guessing. + +Before each group of tool calls, say in a sentence what you're doing and why: +the operator sees it as your progress. The project's documents are in its repository: read ARCHITECTURE.md and any build plans under docs/plans/ before you restate. @@ -119,7 +129,8 @@ the same milestone (don't start another), run the tests, and call finish_milestone again to push the fix. When a review needs no change from you, or you are only waiting for the operator to merge (you can't merge it yourself), call wait_for_review. The next milestone starts only after the -previous PR has merged. When the last PR has merged, call close_item with +previous PR has merged. A milestone's summary says what was done, what was +verified, and what is left open. When the last PR has merged, call close_item with what was done, what was verified, and what is left open. If a PR is closed without merging, ask the operator what next. @@ -133,6 +144,12 @@ Keep ARCHITECTURE.md current: a design change updates it in the same PR. {workspace}""" +QUESTION = """ + +The operator has asked a question (the last message). Answer it with reply, +from what you know and what you can look at; don't change anything. The item +then goes back to waiting exactly as it was, so a restatement up stays up.""" + CONVERSATION = """You are nisse-dev, a software agent, in a conversation with the operator in a Matrix thread. It isn't about a project yet: it may be a question, thoughts on feasibility or hardware, or the start of some work. @@ -257,6 +274,9 @@ class LoopAgent: # The project whose workspace was last worked on: switching to # another parks it (L1). None after a boot: unknown, so all are. self._last_project: str | None = None + # Per item: when progress was last posted (L5: a readable rate). + self._progressed: dict[str, float] = {} + self._active_name = "" # the running item, for "noted" on arrival self._running = False self._stop = threading.Event() self._intake_thread: threading.Thread | None = None @@ -743,6 +763,16 @@ class LoopAgent: # Straight to the flag: the run is what it has to reach (L2). self._cancel.set() about = self._shown.get(thread) if thread else None + if self._running and command not in ("stop", "status"): + # L5: say what happens to a message that can't be read yet. + here = thread and thread == self._active_thread + self._transport.post( + "Noted; I'll read it when this turn ends." + if here + else f"Noted; I'm in the middle of {self._active_name}, and I'll " + "read it when that turn ends.", + thread=thread or msg.event_id or None, + ) self._worker.submit( self._settled("your message", lambda: self.handle(msg, about)) ) @@ -814,6 +844,30 @@ class LoopAgent: self._close(item, loop, "abandoned", f"abandoned by {msg.sender}") elif command == "done" and loop.new_project: self._provisioned(item, loop) + elif command == "details": + self._transport.post(self._details(item), thread=loop.thread) + elif ( + item.position == WAITING + and item.scope != GENERAL + and is_question(text) + and loop.wait in (GO_AHEAD, ANSWER, MERGE) + ): + # L5: questions never change state. Answered, then back to this + # very wait: a restatement up stays up, a go still binds to it. + save( + self._store, + item.id, + READY, + replace( + loop, + question=True, + back_to=loop.wait, + back_note=loop.note, + wait="", + note="", + ), + cause="the operator asked a question", + ) elif command == "stop": # Stopping a run took effect on arrival; a stop is never an answer. if not loop.note.startswith("stopped ("): @@ -1332,6 +1386,9 @@ class LoopAgent: caps=self._helpers.caps, cancel=self._cancel, reader=self._helpers.reader(roles), + on_turn=lambda said: self._progress( + f"{item.id}:helper", loop.thread, said, prefix="helper: " + ), ) git = milestones.git before = git.head_sha() # where the milestone is, uncommitted work aside @@ -1432,6 +1489,54 @@ class LoopAgent: save(self._store, item.id, READY, loop_of(item), cause="a helper finished") return True + def _changed_problem(self, item_id: str, args: dict[str, object]) -> str | None: + """L5: a revision says what changed from the one before.""" + item = self._store.work_item(item_id) + if item is None or loop_of(item).rev == 0: + return None + if str(args.get("changed", "")).strip(): + return None + return ( + f"not posted: this is a revision of rev-{loop_of(item).rev}, so say " + "in changed what's different and why" + ) + + def _progress(self, item_id: str, thread: str, said: str, prefix: str = "") -> None: + """A turn's own words, posted at a readable rate (L5, #42): at most + one a minute per item, the latest when the minute is up.""" + now = self._clock() + if now - self._progressed.get(item_id, float("-inf")) < PROGRESS_EVERY_S: + return + self._progressed[item_id] = now + text = said if len(said) <= PROGRESS_CHARS else said[:PROGRESS_CHARS] + " …" + self._transport.post(f"{prefix}{text}", thread=thread) + + def _details(self, item: WorkItem) -> str: + """L5: raw tool activity, on request: the item's last tool calls.""" + events = self._store.transcript(item.id, "driver").read() + results = { + e.event.call_id: e.event for e in events if isinstance(e.event, ToolResult) + } + lines = [] + for e in events: + if not isinstance(e.event, ToolCall): + continue + call, result = e.event, results.get(e.event.call_id) + target = call.arguments.get("path") or call.arguments.get("command") or "" + outcome = ( + "no result yet" + if result is None + else ("ok" if result.success else "failed") + + (f": {result.result.strip()[:120]}" if result.result.strip() else "") + ) + who = "" if e.actor == "driver" else f"{e.actor}: " + lines.append(f"- {who}{call.tool}({str(target)[:80]}) → {outcome}") + if not lines: + return f"No tool calls yet on {self._label(item)}." + return f"The last tool calls on {self._label(item)}:\n" + "\n".join( + lines[-DETAILS:] + ) + def _design_problem(self, item_id: str) -> str | None: """D1, D2: a `design` item agrees its design before any code. Until a PR of it has merged, a milestone holds documents only.""" @@ -1544,6 +1649,8 @@ class LoopAgent: save(self._store, item.id, RUNNING, loop, cause="driver run") # D1: a `discuss` item talks before it restates. hold = "discuss" in loop.labels and not loop.talked and not loop.started + if loop.question: + hold = True # nothing but answering (below) tools = [ *([] if hold else [RESTATE]), ASK_HUMAN, @@ -1560,6 +1667,8 @@ class LoopAgent: ] ), ] + if loop.question: + tools = [REPLY, *_LOOK] roles = self._roles_of(item) workspace = project.workspace system = SYSTEM.format( @@ -1573,8 +1682,9 @@ class LoopAgent: else "" ), workspace=workspace.describe() if workspace else "", - ) + ) + (QUESTION if loop.question else "") self._active_thread = loop.thread + self._active_name = self._label(item) self._running = True try: outcome = self._driver.run( @@ -1592,10 +1702,14 @@ class LoopAgent: ), }, checks={ - "restate": _restatement_problem, + "restate": lambda args: ( + _restatement_problem(args) + or self._changed_problem(item.id, args) + ), "finish_milestone": lambda args: self._design_problem(item.id), }, model=roles.resolve("driver"), + on_turn=lambda said: self._progress(item.id, loop.thread, said), ) finally: self._running = False @@ -1607,6 +1721,31 @@ class LoopAgent: assert item is not None loop = loop_of(item) args = outcome.arguments or {} + if loop.question: + # Back to the wait the question interrupted, as it was (L5). + if outcome.kind == "wait" and outcome.tool == "reply": + self._transport.post( + str(args.get("text", "")).strip(), thread=loop.thread + ) + else: + self._transport.post( + f"I couldn't answer that ({outcome.note or outcome.kind}).", + thread=loop.thread, + ) + self._wait( + item, + replace( + loop, + wait=loop.back_to, + note=loop.back_note, + question=False, + back_to="", + back_note="", + ), + None, + ) + self._store.set_active(None, cause="question answered") + return if outcome.kind == "wait" and outcome.tool == "restate": text = str(args.get("text", "")).strip() rev = loop.rev + 1 @@ -1622,7 +1761,11 @@ class LoopAgent: f"It's a bug, so I'll go ahead in {round(self._grace / 60)} min " "unless you reply; say go to start now, or reply with changes." ) - self._transport.post(f"rev-{rev}: {text}\n\n{after}", thread=loop.thread) + changed = str(args.get("changed", "")).strip() + since = f"\n\nChanged from rev-{rev - 1}: {changed}" if rev > 1 else "" + self._transport.post( + f"rev-{rev}: {text}{since}\n\n{after}", thread=loop.thread + ) self._wait( item, replace(loop, rev=rev, wait=GO_AHEAD, note="", grace_until=grace), diff --git a/nisse-dev/src/nisse_dev/loop/driver.py b/nisse-dev/src/nisse_dev/loop/driver.py index 0735acc..6d3d3f5 100644 --- a/nisse-dev/src/nisse_dev/loop/driver.py +++ b/nisse-dev/src/nisse_dev/loop/driver.py @@ -12,7 +12,7 @@ from dataclasses import dataclass from typing import Any, Literal from nisse_dev.cancel import Cancel, skip_calls -from nisse_dev.canonical import ToolCall, ToolResult +from nisse_dev.canonical import AssistantText, ToolCall, ToolResult from nisse_dev.llm import LLMClient, LLMError from nisse_dev.loop.tools import WAITING, names from nisse_dev.state import Transcript @@ -63,6 +63,7 @@ class Driver: | None = None, checks: Mapping[str, Callable[[dict[str, Any]], str | None]] | None = None, model: str | None = None, + on_turn: Callable[[str], None] | None = None, ) -> RunOutcome: """``handlers`` answer control tools that don't end the run (e.g. start_milestone): each returns a result and whether it succeeded. @@ -89,6 +90,12 @@ class Driver: transcript.append(event) if isinstance(event, ToolCall): calls.append(event) + # The model's own words for the turn: its progress, in prose (#42). + said = " ".join( + e.text.strip() for e in produced if isinstance(e, AssistantText) + ).strip() + if said and calls and on_turn is not None: + on_turn(said) if not calls: if nudged: return RunOutcome("failed", note="kept replying in prose") diff --git a/nisse-dev/src/nisse_dev/loop/items.py b/nisse-dev/src/nisse_dev/loop/items.py index 00bb07d..bbbb3fa 100644 --- a/nisse-dev/src/nisse_dev/loop/items.py +++ b/nisse-dev/src/nisse_dev/loop/items.py @@ -58,6 +58,11 @@ class Loop: design_merged: bool = False # a `design` item's design PR has merged (D1, D2) helps: int = 0 # delegations so far, for their branch names (L9) helped_at: int = 0 # the milestone a helper was last called in for, unasked + # A question from the operator is answered without changing the item + # (L5): the wait it interrupted, to go back to. + question: bool = False + back_to: str = "" + back_note: str = "" # Where the operator last spoke (D5: answer where asked): "" for the # Matrix thread, or the PR it came from, with the inline location and a # little of what they said, for quoting. diff --git a/nisse-dev/src/nisse_dev/loop/tools.py b/nisse-dev/src/nisse_dev/loop/tools.py index 44a0136..7021972 100644 --- a/nisse-dev/src/nisse_dev/loop/tools.py +++ b/nisse-dev/src/nisse_dev/loop/tools.py @@ -25,9 +25,12 @@ def _tool(tool: str, description: str, /, **params: str) -> dict[str, Any]: RESTATE = _tool( "restate", "Put your understanding of the request to the operator, in their item's " - "thread: what you take it to mean, how it fits the project, and your " - "first step. Ends your turn; work starts when they agree.", + "thread: what you take it to mean, how it fits the project, your first " + "step, and why that way (and the main alternative you considered). Ends " + "your turn; work starts when they agree.", text="Your restatement, in plain language.", + changed="What changed from your previous restatement, and why; leave it " + "empty for the first.", ) ASK_HUMAN = _tool( "ask_human", diff --git a/nisse-dev/tests/test_loop.py b/nisse-dev/tests/test_loop.py index 05d3a8e..ae490c6 100644 --- a/nisse-dev/tests/test_loop.py +++ b/nisse-dev/tests/test_loop.py @@ -214,7 +214,9 @@ def test_go_starts_the_work_and_close_item_closes_the_issue(h: Harness) -> None: def test_a_go_for_an_earlier_revision_is_not_applied(h: Harness) -> None: - h.model.turns.append([call("restate", text="revised: in a new module")]) + h.model.turns.append( + [call("restate", text="revised: in a new module", changed="a new module")] + ) h.agent.intake() h.say("put it in its own module") # changes, so the driver restates rev-2 assert loop_of(h.item()).rev == 2 diff --git a/nisse-dev/tests/test_loop_help.py b/nisse-dev/tests/test_loop_help.py index 0966193..ba8346d 100644 --- a/nisse-dev/tests/test_loop_help.py +++ b/nisse-dev/tests/test_loop_help.py @@ -301,3 +301,26 @@ def test_a_delegation_cut_off_by_a_crash_is_discarded_at_boot( ] assert any("cut off by a restart and discarded" in f for f in facts) assert git + + +def test_a_helpers_progress_is_posted(world: dict[str, Any]) -> None: + helper = Spy( + [ + AssistantText(text="Writing helped.py with the constant.", model="hlp"), + call("write_file", path="helped.py", content="H = 1\n"), + ], + [hand_back()], + ) + model = ScriptedModel( + RESTATE, + [ + call("start_milestone", name="words"), + call("delegate", task="write helped.py"), + call("ask_human", question="done?"), + ], + ) + agent, transport = loop_agent(world, model, helpers=helpers(helper)) + agent.intake() + say(transport, world, "go") + said = [m.text for m in transport.outbound] + assert "helper: Writing helped.py with the constant." in said diff --git a/nisse-dev/tests/test_loop_labels.py b/nisse-dev/tests/test_loop_labels.py index 6f319c6..9176a7c 100644 --- a/nisse-dev/tests/test_loop_labels.py +++ b/nisse-dev/tests/test_loop_labels.py @@ -118,7 +118,7 @@ def test_a_plain_yes_is_a_go_ahead_said_so_first(tmp_path: Path) -> None: def test_anything_else_goes_to_the_driver(tmp_path: Path) -> None: h = Harness( tmp_path, - ScriptedModel(RESTATED, [call("restate", text="v2")]), + ScriptedModel(RESTATED, [call("restate", text="v2", changed="as you asked")]), FakeGitea(issue()), reader=Reader(), ) @@ -132,7 +132,7 @@ def test_a_question_never_reaches_the_reader(tmp_path: Path) -> None: reader = Reader("sounds good?") h = Harness( tmp_path, - ScriptedModel(RESTATED, [call("restate", text="v2")]), + ScriptedModel(RESTATED, [call("restate", text="v2", changed="as you asked")]), FakeGitea(issue()), reader=reader, ) @@ -145,7 +145,9 @@ def test_a_plain_yes_to_an_older_revision_is_not_a_go(tmp_path: Path) -> None: reader = Reader("yes") h = Harness( tmp_path, - ScriptedModel(RESTATED, [call("restate", text="v2")], STARTED), + ScriptedModel( + RESTATED, [call("restate", text="v2", changed="as you asked")], STARTED + ), FakeGitea(issue()), reader=reader, ) @@ -224,7 +226,12 @@ def test_a_reply_stops_the_bugs_countdown(tmp_path: Path) -> None: def test_a_new_revision_of_a_bug_gets_its_own_countdown(tmp_path: Path) -> None: - h, clock = bug(tmp_path, RESTATED, [call("restate", text="v2")], STARTED) + h, clock = bug( + tmp_path, + RESTATED, + [call("restate", text="v2", changed="as you asked")], + STARTED, + ) h.agent.intake() clock.now += 300 h.say("use the other file") diff --git a/nisse-dev/tests/test_loop_multi.py b/nisse-dev/tests/test_loop_multi.py index 69cf7f8..98e7a0c 100644 --- a/nisse-dev/tests/test_loop_multi.py +++ b/nisse-dev/tests/test_loop_multi.py @@ -95,7 +95,7 @@ def test_the_next_milestone_waits_for_the_merge(world: dict[str, Any]) -> None: agent, transport = loop_agent(world, model) agent.intake() say(transport, world, "go") - say(transport, world, "how's it going?") + say(transport, world, "carry on") assert "hasn't merged yet" in result_of(world, "start_milestone") assert (item(world).position, loop_of(item(world)).wait) == (WAITING, MERGE) assert len(forge.prs) == 1 diff --git a/nisse-dev/tests/test_loop_properties.py b/nisse-dev/tests/test_loop_properties.py index 5d3d21d..2413276 100644 --- a/nisse-dev/tests/test_loop_properties.py +++ b/nisse-dev/tests/test_loop_properties.py @@ -76,6 +76,8 @@ class PolicyModel: return [call("ask_human", question=f"question {self.n}?")] if roll < 0.12: return [ToolCall(call_id=f"p{self.n}", tool="nonsense", arguments={})] + if "reply" in offered and "propose_work" not in offered: + return [call("reply", text=f"because {self.n}")] # a question run if "reply" in offered: # a conversation: talk, or propose work if self.rng.random() < 0.1: return [ @@ -99,7 +101,7 @@ class PolicyModel: if "restate" not in offered and "finish_milestone" not in offered: return [call("ask_human", question=f"talk first {self.n}?")] # discuss if "finish_milestone" not in offered: - return [call("restate", text=f"restatement {self.n}")] + return [call("restate", text=f"restatement {self.n}", changed=f"c{self.n}")] if "delegate" in offered and self.rng.random() < 0.1: return [call("delegate", task=f"help {self.n}")] if "wait_for_review" in offered and self.rng.random() < 0.2: @@ -248,7 +250,7 @@ class Walk: forge: Forge = self.worlds[project]["forge"] if item and loop: weights.update({"go": 3, "answer": 2, "abandon": 1, "go-stale": 1}) - weights.update({"plain-yes": 2, "wait": 2}) + weights.update({"plain-yes": 2, "wait": 2, "question": 2}) if loop.new_project: weights.update({"done": 2}) if item.scope != "general": # conversations aren't named by number @@ -264,6 +266,28 @@ class Walk: elif action == "done" and loop: # The operator says a new project is ready; no entry was added. self.transport.receive(DON, "done", thread=loop.thread) + elif action == "question" and loop: + # L5: a question never changes the item it's asked about. + before = (item.position, loop.wait, loop.rev, loop.pr, loop.branch) + self.transport.receive(DON, f"why {len(self.steps)}?", thread=loop.thread) + after_item = self.world["store"].work_item(item.id) + after = loop_of(after_item) + if ( + item.scope != "general" + and item.position == WAITING + and loop.wait in (GO_AHEAD, ANSWER, MERGE) + ): + now = ( + after_item.position, + after.wait, + after.rev, + after.pr, + after.branch, + ) + assert now == before, ( + f"questions_change_nothing: {before} -> {now}: " + f"seed {self.seed}, steps {self.steps}" + ) elif action == "chat": # A top-level message: a conversation of its own (L1, L5). self.transport.receive(DON, f"thinking aloud {len(self.steps)}") diff --git a/nisse-dev/tests/test_loop_talk.py b/nisse-dev/tests/test_loop_talk.py new file mode 100644 index 0000000..f8b2252 --- /dev/null +++ b/nisse-dev/tests/test_loop_talk.py @@ -0,0 +1,163 @@ +"""Conversation on the new loop (M2.7, L5, #42, #77): questions answered +without changing the item, revisions that say what changed, progress in +prose at a readable rate, raw tool activity on request, and a note for a +message that can't be read yet.""" + +from pathlib import Path +from typing import Any + +from test_loop import DON, RESTATED, FakeGitea, Harness, ScriptedModel, call, issue + +from nisse_dev.canonical import AssistantText +from nisse_dev.loop.items import ANSWER, GO_AHEAD, WAITING, loop_of + + +class Clock: + def __init__(self) -> None: + self.now = 1000.0 + + def __call__(self) -> float: + return self.now + + +def harness(tmp_path: Path, *turns: list[Any], **kw: Any) -> Harness: + return Harness(tmp_path, ScriptedModel(*turns), FakeGitea(issue()), **kw) + + +# --- questions never change state (#77) ------------------------------------- + + +def test_a_question_about_a_restatement_leaves_it_up(tmp_path: Path) -> None: + h = harness( + tmp_path, + RESTATED, + [call("reply", text="Because text.py already holds the string helpers.")], + [call("ask_human", question="started; anything else?")], + ) + h.agent.intake() + h.say("Why text.py?") + loop = loop_of(h.item()) + assert (h.item().position, loop.wait, loop.rev) == (WAITING, GO_AHEAD, 1) + assert not loop.question and not loop.started + assert h.posted()[-1] == "Because text.py already holds the string helpers." + h.say("go") # still bound to rev-1 + assert loop_of(h.item()).started + + +def test_a_question_back_keeps_the_agents_own_question_open(tmp_path: Path) -> None: + h = harness( + tmp_path, + [call("ask_human", question="One function or two?")], + [call("reply", text="One handles words, two would split lines too.")], + ) + h.agent.intake() + h.say("What would the second one do?") + loop = loop_of(h.item()) + assert (loop.wait, loop.note) == (ANSWER, "One function or two?") + + +def test_a_question_that_isnt_answered_still_goes_back(tmp_path: Path) -> None: + h = harness( + tmp_path, + RESTATED, + [call("restate", text="sneaky v2", changed="x")], # not offered + [AssistantText(text="hmm", model="m")], + [AssistantText(text="hmm", model="m")], # prose twice: the run fails + ) + h.agent.intake() + h.say("why?") + loop = loop_of(h.item()) + # restate isn't offered in a question run: no new revision. + assert (loop.wait, loop.rev) == (GO_AHEAD, 1) + assert h.posted()[-1].startswith("I couldn't answer that") + + +# --- explanations ------------------------------------------------------------ + + +def test_a_revision_says_what_changed(tmp_path: Path) -> None: + h = harness( + tmp_path, + RESTATED, + [call("restate", text="count_words in words.py", changed="")], + [ + call( + "restate", + text="count_words in words.py", + changed="a module of its own, as you asked", + ) + ], + ) + h.agent.intake() + h.say("put it in its own module") + loop = loop_of(h.item()) + assert loop.rev == 2 + assert "Changed from rev-1: a module of its own, as you asked" in h.posted()[-1] + + +# --- progress and details (#42) ---------------------------------------------- + + +def test_progress_is_posted_at_a_readable_rate(tmp_path: Path) -> None: + clock = Clock() + + class Ticking(ScriptedModel): + def complete(self, events, tools, model, system=None): # type: ignore[no-untyped-def] + clock.now += 30 # each turn takes a while + return super().complete(events, tools, model, system) + + h = Harness( + tmp_path, + Ticking( + [ + AssistantText(text="Looking at text.py first.", model="m"), + call("list_dir", path="."), + ], + [ + AssistantText(text="Reading the tests too.", model="m"), + call("list_dir", path="tests"), + ], + [ + AssistantText(text="x" * 1000, model="m"), + call("list_dir", path="."), + ], + [call("restate", text="I'll add count_words.", changed="")], + ), + FakeGitea(issue()), + clock=clock, + ) + h.agent.intake() + said = h.posted() + assert "Looking at text.py first." in said + assert "Reading the tests too." not in said # 30 s later: folded away + long = [p for p in said if p.startswith("xxx")] + assert long and len(long[0]) < 420 and long[0].endswith(" …") # 60 s on + + +def test_details_shows_the_last_tool_calls(tmp_path: Path) -> None: + h = harness( + tmp_path, + [ + call("list_dir", path="src"), + call("read_file", path="text.py"), + call("restate", text="I'll add count_words.", changed=""), + ], + ) + h.agent.intake() + h.say("details") + shown = h.posted()[-1] + assert shown.startswith("The last tool calls on #14:") + assert "list_dir(src)" in shown and "read_file(text.py)" in shown + assert loop_of(h.item()).wait == GO_AHEAD # nothing changed + + +def test_a_message_mid_run_is_acknowledged(tmp_path: Path) -> None: + h = harness(tmp_path, RESTATED) + h.agent.intake() + h.agent._running = True # as if a driver run were under way + h.agent._active_thread = "elsewhere" + h.agent._active_name = "#15" + h.transport.receive(DON, "one more thing", thread=h.thread()) + assert "Noted; I'm in the middle of #15" in h.posted()[-1] + h.transport.receive(DON, "and another", thread="elsewhere") + assert h.posted()[-1] == "Noted; I'll read it when this turn ends." diff --git a/tests/integration/test_end_to_end_loop.py b/tests/integration/test_end_to_end_loop.py index 492d3b0..a36174a 100644 --- a/tests/integration/test_end_to_end_loop.py +++ b/tests/integration/test_end_to_end_loop.py @@ -634,3 +634,35 @@ def test_a_conversation_becomes_a_new_project( ] assert "ARCHITECTURE.md" in files, files assert any(f.startswith("docs/plans/") for f in files), files + + +def test_a_question_leaves_the_restatement_up( + running: tuple[RunningLoop, InMemoryTransport, str], +) -> None: + """M2.7: a question while a restatement is up is answered, and changes + nothing: the same revision stays up, and a go binds to it.""" + agent, transport, run = running + operator = Operator(agent, transport) + repo = agent.config.gitea_repo + as_operator = operator_gitea(agent.config.gitea_url) + issues: list[int] = [] + try: + issues.append(as_operator.create_issue(repo, f"vowels_{run}", task(run))) + as_operator._request( + "PATCH", + f"/repos/{repo}/issues/{issues[0]}", + json={"assignees": [agent.account]}, + ) + operator.tick() + item = operator.drive_until(GO_AHEAD, issues[0]) + rev = loop_of(item).rev + said = len(transport.outbound) + operator.say("Why put it in that module rather than a new one?", issues[0]) + answer = [m.text for m in transport.outbound[said:]] + assert answer and not answer[-1].startswith("rev-"), answer + loop = loop_of(operator.item(issues[0])) + assert (loop.wait, loop.rev) == (GO_AHEAD, rev), loop + operator.say("go", issues[0]) + assert loop_of(operator.item(issues[0])).started + finally: + operator.tidy(issues, [])