Threads, commands and labels on the new loop (M2.3)
CI / checks (push) Skipped
CI / checks (pull_request) Successful in 3m45s
CI / checks (push) Skipped
CI / checks (pull_request) Successful in 3m45s
- Commands that name their item ("go #14", "abandon #14", "stop #14")
apply to it wherever they're said; go, approve and abandon alone at top
level are asked about (L5). stop is never taken as an answer.
- A plain-language reply to a restatement is read by GoAheadReader, one
model call that agrees only on a clear yes; questions never reach it. The
agent says "taking that as a go-ahead on rev-N" first, bound to the
revision on screen at arrival (L3, L5, D1).
- Labels steer (D1): `bug` goes ahead after tuning.loop.bug_grace_s
(default 600 s) unless the operator replies; `discuss` isn't offered
restate until the operator has spoken; `bug`, `discuss` and `design` add
to the driver's instructions. A restatement asks no questions.
Tests: label and command unit tests, the property walk with labelled
issues, plain yeses, named gos and time passing (grace_only_unanswered,
discuss_talks_first), and e2e tests for a bug proceeding after its grace
period and a plain-language go-ahead.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PPQ14L3LikyefCMFBqEbKE
This commit is contained in:
1 parent
a504526da2
commit
4d9c1d7529
13 files changed
+793
-35
No files matched your search
@@ -158,6 +158,7 @@ def test_tuning_is_read() -> None:
|
||||
"expert": {"consult_budget": 5},
|
||||
"delegation": {"max_turns": 20, "max_tokens": 100000, "wall_clock_s": 600},
|
||||
"tools": {"command_timeout_s": 300, "max_output_chars": 2000},
|
||||
"loop": {"bug_grace_s": 120},
|
||||
}
|
||||
)
|
||||
tuning = config.tuning
|
||||
@@ -166,6 +167,7 @@ def test_tuning_is_read() -> None:
|
||||
assert tuning.delegation.max_tokens == 100_000
|
||||
assert tuning.delegation.wall_clock_s == 600.0
|
||||
assert tuning.tools.max_output_chars == 2000
|
||||
assert tuning.loop.bug_grace_s == 120.0
|
||||
|
||||
|
||||
def test_a_partly_given_tuning_section_keeps_the_other_defaults() -> None:
|
||||
|
||||
@@ -72,12 +72,18 @@ def issue(number: int = 14, author: str = "don", **kw: Any) -> Issue:
|
||||
title=kw.get("title", "Add count_words"),
|
||||
body=kw.get("body", "count words in a string"),
|
||||
author=author,
|
||||
labels=(),
|
||||
labels=kw.get("labels", ()),
|
||||
)
|
||||
|
||||
|
||||
class Harness:
|
||||
def __init__(self, tmp_path: Path, model: ScriptedModel, gitea: FakeGitea) -> None:
|
||||
def __init__(
|
||||
self,
|
||||
tmp_path: Path,
|
||||
model: ScriptedModel,
|
||||
gitea: FakeGitea,
|
||||
**agent: Any, # more LoopAgent arguments: reader, clock, ...
|
||||
) -> None:
|
||||
tmp_path.mkdir(parents=True, exist_ok=True)
|
||||
self.store = StateStore(
|
||||
tmp_path / "state.db", oplog=OpLog("nisse-dev", io.StringIO())
|
||||
@@ -100,6 +106,7 @@ class Harness:
|
||||
worker=InlineWorker(),
|
||||
cancel=self.cancel,
|
||||
oplog=OpLog("nisse-dev", io.StringIO()),
|
||||
**agent,
|
||||
)
|
||||
self.transport.on_message(self.agent.on_message)
|
||||
|
||||
@@ -215,7 +222,8 @@ def test_a_go_at_top_level_is_not_a_go_ahead(h: Harness) -> None:
|
||||
h.agent.intake()
|
||||
h.say("go", thread=None)
|
||||
assert not loop_of(h.item()).started
|
||||
assert "assign one and I'll pick it up" in h.posted()[-1]
|
||||
# L5: an unnamed command at top level is ambiguous, so it asks.
|
||||
assert "Which item?" in h.posted()[-1] and "#14" in h.posted()[-1]
|
||||
|
||||
|
||||
# --- waiting points (L2) -------------------------------------------------------------
|
||||
|
||||
@@ -0,0 +1,282 @@
|
||||
"""Threads, commands and labels in the new loop (M2.3): commands that name
|
||||
their item, ambiguous ones asked about (L5); plain-language go-aheads; `bug`
|
||||
grace periods, `discuss` and `design` (D1)."""
|
||||
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
|
||||
import pytest
|
||||
from test_loop import DON, RESTATED, FakeGitea, Harness, ScriptedModel, call, issue
|
||||
from test_loop import h as h # noqa: PLC0414 -- the fixture
|
||||
|
||||
from nisse_dev.canonical import Fact, ToolCall
|
||||
from nisse_dev.llm import LLMError
|
||||
from nisse_dev.loop.items import ANSWER, GO_AHEAD, READY, WAITING, loop_of
|
||||
from nisse_dev.loop.reader import GoAheadReader
|
||||
|
||||
STARTED = [call("ask_human", question="started; anything else?")]
|
||||
|
||||
|
||||
class Clock:
|
||||
def __init__(self) -> None:
|
||||
self.now = 1000.0
|
||||
|
||||
def __call__(self) -> float:
|
||||
return self.now
|
||||
|
||||
|
||||
class Reader:
|
||||
"""A scripted go-ahead reader: agrees with what it's told to."""
|
||||
|
||||
def __init__(self, *yes: str) -> None:
|
||||
self.yes = set(yes)
|
||||
self.asked: list[tuple[str, str]] = []
|
||||
|
||||
def agrees(self, proposal: str, reply: str) -> bool:
|
||||
self.asked.append((proposal, reply))
|
||||
return reply in self.yes
|
||||
|
||||
|
||||
def facts(h: Harness) -> list[str]:
|
||||
return [
|
||||
e.event.text
|
||||
for e in h.store.transcript(h.item().id, "orchestrator").read()
|
||||
if isinstance(e.event, Fact)
|
||||
]
|
||||
|
||||
|
||||
def two(tmp_path: Path, *turns: list[Any]) -> Harness:
|
||||
return Harness(
|
||||
tmp_path,
|
||||
ScriptedModel(*turns),
|
||||
FakeGitea(issue(14), issue(15, title="Add count_lines")),
|
||||
)
|
||||
|
||||
|
||||
# --- commands that name their item, and ambiguous ones (L5) -------------------
|
||||
|
||||
|
||||
def test_a_named_go_at_top_level_applies_to_its_item(tmp_path: Path) -> None:
|
||||
h = two(tmp_path, RESTATED, RESTATED, STARTED)
|
||||
h.agent.intake()
|
||||
h.say("go #15", thread=None)
|
||||
items = {loop_of(i).issue: i for i in h.store.work_items()}
|
||||
assert loop_of(items[15]).started and not loop_of(items[14]).started
|
||||
|
||||
|
||||
def test_a_named_abandon_in_another_thread_still_names_its_item(
|
||||
tmp_path: Path,
|
||||
) -> None:
|
||||
h = two(tmp_path, RESTATED, RESTATED)
|
||||
h.agent.intake()
|
||||
items = {loop_of(i).issue: i for i in h.store.work_items()}
|
||||
h.transport.receive(DON, "Abandon #15.", thread=loop_of(items[14]).thread)
|
||||
items = {loop_of(i).issue: i for i in h.store.work_items()}
|
||||
assert items[15].status == "closed" and items[14].status == "open"
|
||||
|
||||
|
||||
def test_an_unnamed_command_at_top_level_is_asked_about(tmp_path: Path) -> None:
|
||||
h = two(tmp_path, RESTATED, RESTATED)
|
||||
h.agent.intake()
|
||||
h.say("abandon", thread=None)
|
||||
assert all(i.status == "open" for i in h.store.work_items())
|
||||
asked = h.posted()[-1]
|
||||
assert "Which item?" in asked and "#14" in asked and "#15" in asked
|
||||
|
||||
|
||||
def test_naming_an_item_that_isnt_open_says_so(h: Harness) -> None:
|
||||
h.agent.intake()
|
||||
h.say("go #99", thread=None)
|
||||
assert "#99 isn't one of my open work items" in h.posted()[-1]
|
||||
|
||||
|
||||
def test_stop_in_a_waiting_items_thread_is_not_an_answer(h: Harness) -> None:
|
||||
h.agent.intake()
|
||||
h.say("stop")
|
||||
assert (h.item().position, loop_of(h.item()).wait) == (WAITING, GO_AHEAD)
|
||||
assert "nothing to stop" in h.posted()[-1]
|
||||
|
||||
|
||||
# --- plain-language go-aheads (L5, D1) ------------------------------------------
|
||||
|
||||
|
||||
def test_a_plain_yes_is_a_go_ahead_said_so_first(tmp_path: Path) -> None:
|
||||
reader = Reader("Yep, that's it")
|
||||
h = Harness(
|
||||
tmp_path, ScriptedModel(RESTATED, STARTED), FakeGitea(issue()), reader=reader
|
||||
)
|
||||
h.agent.intake()
|
||||
h.say("Yep, that's it")
|
||||
assert loop_of(h.item()).started
|
||||
assert "Taking that as a go-ahead on rev-1" in " ".join(h.posted())
|
||||
assert reader.asked == [
|
||||
("I'll add count_words to text.py, with tests.", "Yep, that's it")
|
||||
]
|
||||
assert any(f.startswith("go-ahead on rev-1 from") for f in facts(h))
|
||||
|
||||
|
||||
def test_anything_else_goes_to_the_driver(tmp_path: Path) -> None:
|
||||
h = Harness(
|
||||
tmp_path,
|
||||
ScriptedModel(RESTATED, [call("restate", text="v2")]),
|
||||
FakeGitea(issue()),
|
||||
reader=Reader(),
|
||||
)
|
||||
h.agent.intake()
|
||||
h.say("Mostly, but call it word_count")
|
||||
loop = loop_of(h.item())
|
||||
assert not loop.started and loop.rev == 2
|
||||
|
||||
|
||||
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")]),
|
||||
FakeGitea(issue()),
|
||||
reader=reader,
|
||||
)
|
||||
h.agent.intake()
|
||||
h.say("sounds good?")
|
||||
assert not loop_of(h.item()).started and reader.asked == []
|
||||
|
||||
|
||||
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),
|
||||
FakeGitea(issue()),
|
||||
reader=reader,
|
||||
)
|
||||
h.agent.intake()
|
||||
h.say("change it a bit") # rev-2 goes up
|
||||
h.agent.handle(_msg("yes", h.thread()), about=1)
|
||||
assert not loop_of(h.item()).started
|
||||
assert [reply for _, reply in reader.asked] == ["change it a bit"]
|
||||
|
||||
|
||||
class _Model:
|
||||
def __init__(self, result: Any) -> None:
|
||||
self.result = result
|
||||
|
||||
def complete(self, events, tools, model, system=None) -> list[Any]:
|
||||
if isinstance(self.result, Exception):
|
||||
raise self.result
|
||||
return self.result
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("result", "agrees"),
|
||||
[
|
||||
([ToolCall(call_id="v", tool="verdict", arguments={"answer": "yes"})], True),
|
||||
(
|
||||
[ToolCall(call_id="v", tool="verdict", arguments={"answer": "unclear"})],
|
||||
False,
|
||||
),
|
||||
([], False),
|
||||
(LLMError("down"), False),
|
||||
],
|
||||
)
|
||||
def test_the_reader_agrees_only_on_a_clear_yes(result: Any, agrees: bool) -> None:
|
||||
reader = GoAheadReader(_Model(result), "m") # type: ignore[arg-type]
|
||||
assert reader.agrees("add count_words", "ok") is agrees
|
||||
|
||||
|
||||
# --- labels (D1) ----------------------------------------------------------------
|
||||
|
||||
|
||||
def bug(tmp_path: Path, *turns: list[Any], **kw: Any) -> tuple[Harness, Clock]:
|
||||
clock = Clock()
|
||||
h = Harness(
|
||||
tmp_path,
|
||||
ScriptedModel(*turns),
|
||||
FakeGitea(issue(labels=("bug",))),
|
||||
clock=clock,
|
||||
bug_grace_s=600,
|
||||
**kw,
|
||||
)
|
||||
return h, clock
|
||||
|
||||
|
||||
def test_a_bug_goes_ahead_after_the_grace_period(tmp_path: Path) -> None:
|
||||
h, clock = bug(tmp_path, RESTATED, STARTED)
|
||||
h.agent.intake()
|
||||
assert "go ahead in 10 min unless you reply" in h.posted()[-1]
|
||||
assert "going ahead in 10 min" in h.agent.status()
|
||||
clock.now += 599
|
||||
h.agent.intake()
|
||||
assert not loop_of(h.item()).started
|
||||
clock.now += 1
|
||||
h.agent.intake()
|
||||
assert loop_of(h.item()).started
|
||||
assert "go-ahead on rev-1 from the bug grace period" in facts(h)
|
||||
|
||||
|
||||
def test_a_reply_stops_the_bugs_countdown(tmp_path: Path) -> None:
|
||||
h, clock = bug(tmp_path, RESTATED, [call("ask_human", question="which file?")])
|
||||
h.agent.intake()
|
||||
h.say("hmm, what about unicode")
|
||||
clock.now += 3600
|
||||
h.agent.intake()
|
||||
loop = loop_of(h.item())
|
||||
assert not loop.started and loop.grace_until == 0 and loop.wait == ANSWER
|
||||
|
||||
|
||||
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.agent.intake()
|
||||
clock.now += 300
|
||||
h.say("use the other file")
|
||||
assert loop_of(h.item()).grace_until == clock.now + 600
|
||||
clock.now += 600
|
||||
h.agent.intake()
|
||||
assert "go-ahead on rev-2 from the bug grace period" in facts(h)
|
||||
|
||||
|
||||
def test_the_bug_label_reaches_the_driver(tmp_path: Path) -> None:
|
||||
seen: list[str] = []
|
||||
|
||||
class Spy(ScriptedModel):
|
||||
def complete(self, events, tools, model, system=None): # type: ignore[no-untyped-def]
|
||||
seen.append(system or "")
|
||||
return super().complete(events, tools, model, system)
|
||||
|
||||
h = Harness(tmp_path, Spy(RESTATED), FakeGitea(issue(labels=("bug", "ui"))))
|
||||
h.agent.intake()
|
||||
assert "never fix a bug you couldn't see" in seen[0]
|
||||
assert "labelled discuss" not in seen[0]
|
||||
|
||||
|
||||
def test_discuss_talks_before_it_restates(tmp_path: Path) -> None:
|
||||
offered: list[set[str]] = []
|
||||
|
||||
class Spy(ScriptedModel):
|
||||
def complete(self, events, tools, model, system=None): # type: ignore[no-untyped-def]
|
||||
offered.append({t["function"]["name"] for t in tools})
|
||||
return super().complete(events, tools, model, system)
|
||||
|
||||
h = Harness(
|
||||
tmp_path,
|
||||
Spy([call("ask_human", question="what's it for?")], RESTATED),
|
||||
FakeGitea(issue(labels=("discuss",))),
|
||||
)
|
||||
h.agent.intake()
|
||||
assert "restate" not in offered[0]
|
||||
assert loop_of(h.item()).wait == ANSWER
|
||||
h.say("for the stats page")
|
||||
assert "restate" in offered[1]
|
||||
assert loop_of(h.item()).wait == GO_AHEAD
|
||||
|
||||
|
||||
def test_an_unlabelled_issue_waits_for_go_with_no_countdown(h: Harness) -> None:
|
||||
h.agent.intake()
|
||||
loop = loop_of(h.item())
|
||||
assert loop.wait == GO_AHEAD and loop.grace_until == 0
|
||||
assert h.item().position != READY
|
||||
|
||||
|
||||
def _msg(text: str, thread: str) -> Any:
|
||||
from nisse_dev.transport import InboundMessage
|
||||
|
||||
return InboundMessage(DON, text, thread=thread)
|
||||
@@ -19,7 +19,7 @@ from test_loop_milestones import DON, Forge, call
|
||||
from test_loop_milestones import world as world # noqa: PLC0414 -- the fixture
|
||||
|
||||
from nisse_dev.cancel import Cancel
|
||||
from nisse_dev.canonical import Fact, ToolCall
|
||||
from nisse_dev.canonical import Fact, PlanSubmitted, ToolCall, UserMessage
|
||||
from nisse_dev.gitea import Comment, Issue
|
||||
from nisse_dev.loop.agent import LoopAgent
|
||||
from nisse_dev.loop.driver import Driver
|
||||
@@ -60,6 +60,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 "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}")]
|
||||
if "wait_for_review" in offered and self.rng.random() < 0.2:
|
||||
@@ -89,8 +91,14 @@ class Walk:
|
||||
self.steps: list[str] = []
|
||||
self.shown_at_go: list[str] = [] # item:revision on screen at each go
|
||||
self.next_issue = 15
|
||||
self.now = 1000.0
|
||||
self.agent, self.transport = self._agent()
|
||||
|
||||
def reads_yes(self, proposal: str, reply: str) -> bool:
|
||||
"""The scripted go-ahead reader: a plain yes usually reads as one,
|
||||
and nothing else ever does."""
|
||||
return reply == "yep, that's right" and self.rng.random() < 0.7
|
||||
|
||||
def _agent(self) -> tuple[LoopAgent, InMemoryTransport]:
|
||||
transport = InMemoryTransport()
|
||||
cancel = Cancel()
|
||||
@@ -110,10 +118,16 @@ class Walk:
|
||||
cancel=cancel,
|
||||
oplog=OpLog("nisse-dev", _Null()),
|
||||
milestones=w["milestones"],
|
||||
reader=self, # type: ignore[arg-type]
|
||||
bug_grace_s=600,
|
||||
clock=lambda: self.now,
|
||||
)
|
||||
transport.on_message(agent.on_message)
|
||||
return agent, transport
|
||||
|
||||
def agrees(self, proposal: str, reply: str) -> bool:
|
||||
return self.reads_yes(proposal, reply)
|
||||
|
||||
def items(self) -> list[Any]:
|
||||
return self.world["store"].work_items()
|
||||
|
||||
@@ -127,7 +141,7 @@ class Walk:
|
||||
loop = loop_of(item) if item else None
|
||||
if item and loop:
|
||||
weights.update({"go": 3, "answer": 2, "abandon": 1, "go-stale": 1})
|
||||
weights.update({"first": 1})
|
||||
weights.update({"first": 1, "plain-yes": 2, "named-go": 1, "wait": 2})
|
||||
if loop.pr:
|
||||
weights.update(
|
||||
{"comment": 4, "merge": 2, "close-pr": 1, "comment-merge": 2}
|
||||
@@ -139,11 +153,33 @@ class Walk:
|
||||
elif action == "issue":
|
||||
# A new issue, sometimes depending on one that is still open.
|
||||
n, self.next_issue = self.next_issue, self.next_issue + 1
|
||||
forge.issues.append(Issue(n, f"task {n}", f"do task {n}", "don", ()))
|
||||
labels = self.rng.choice([(), (), ("bug",), ("discuss",), ("design",)])
|
||||
forge.issues.append(Issue(n, f"task {n}", f"do task {n}", "don", labels))
|
||||
live = [i.number for i in forge.assigned_issues("", "") if i.number != n]
|
||||
if live and self.rng.random() < 0.5:
|
||||
forge.deps[n] = [self.rng.choice(live)]
|
||||
self.agent.intake()
|
||||
elif action == "plain-yes" and loop:
|
||||
# Read as a go-ahead or not; either way bound to what's up now.
|
||||
if item.position == WAITING and loop.wait == GO_AHEAD:
|
||||
self.shown_at_go.append(f"{item.id}:{loop.rev}")
|
||||
self.transport.receive(DON, "yep, that's right", thread=loop.thread)
|
||||
elif action == "named-go" and loop:
|
||||
if item.position == WAITING and loop.wait == GO_AHEAD:
|
||||
self.shown_at_go.append(f"{item.id}:{loop.rev}")
|
||||
self.transport.receive(DON, f"go #{loop.issue}")
|
||||
elif action == "wait":
|
||||
# Time passes: bug countdowns may run out.
|
||||
self.now += self.rng.choice([60, 700])
|
||||
for other in open_items:
|
||||
o = loop_of(other)
|
||||
if (
|
||||
other.position == WAITING
|
||||
and o.wait == GO_AHEAD
|
||||
and 0 < o.grace_until <= self.now
|
||||
):
|
||||
self.shown_at_go.append(f"{other.id}:{o.rev}")
|
||||
self.agent.intake()
|
||||
elif action == "first" and loop:
|
||||
self.agent.handle(InboundMessage(DON, f"do #{loop.issue} next"), None)
|
||||
elif action == "crash":
|
||||
@@ -213,7 +249,7 @@ class Walk:
|
||||
and f.event.text.startswith("go-ahead on rev-")
|
||||
]
|
||||
assert all(f"{item.id}:{rev}" in self.shown_at_go for rev in gos), (
|
||||
f"go_binds: {gos}: {where}"
|
||||
f"go_binds: {item.id} {gos} not in {self.shown_at_go}: {where}"
|
||||
)
|
||||
# Blocked comes from external truth (L1): an item whose issue
|
||||
# had an open dependency from the start never ran before it closed.
|
||||
@@ -238,6 +274,36 @@ class Walk:
|
||||
assert not [
|
||||
i for i in store.pending_intents() if i.work_item == item.id
|
||||
], f"closed_items_leave_nothing (intent): {where}"
|
||||
for item in items:
|
||||
# D1: a bug's grace period runs out only on an unanswered
|
||||
# restatement: any reply since it went up stops the countdown.
|
||||
replied = False
|
||||
for e in store.transcript(item.id, "orchestrator").read():
|
||||
text = getattr(e.event, "text", "")
|
||||
if isinstance(e.event, Fact) and text.startswith("restatement rev-"):
|
||||
replied = False
|
||||
elif isinstance(e.event, UserMessage) and e.actor == "operator":
|
||||
replied = replied or e.event.source.startswith("transport:")
|
||||
elif text.endswith("from the bug grace period"):
|
||||
assert not replied, (
|
||||
f"grace_only_unanswered: #{loop_of(item).issue}: {where}"
|
||||
)
|
||||
for item in items:
|
||||
# D1: a `discuss` item talks before it restates.
|
||||
if "discuss" not in loop_of(item).labels:
|
||||
continue
|
||||
events = store.transcript(item.id, "driver").read()
|
||||
plans = [e.id for e in events if isinstance(e.event, PlanSubmitted)]
|
||||
spoke = [
|
||||
e.id
|
||||
for e in events
|
||||
if e.actor == "operator"
|
||||
and isinstance(e.event, UserMessage)
|
||||
and e.event.source.startswith("transport:")
|
||||
]
|
||||
assert not plans or (spoke and spoke[0] < plans[0]), (
|
||||
f"discuss_talks_first: #{loop_of(item).issue}: {where}"
|
||||
)
|
||||
assert not [i for i in items if i.position == RUNNING], (
|
||||
f"one_active (nothing left running): {where}"
|
||||
)
|
||||
|
||||
Reference in new issue
Block a user