Merge pull request 'Fixes from the rest of the live session' (#122) from live-findings into main
Reviewed-on: #122 Reviewed-by: Don Garrett <dgarrett@acm.org>
This commit was merged in pull request #122.
This commit is contained in:
commit
09232dcbdd
9 files changed
+242
-14
No files matched your search
@@ -569,6 +569,9 @@ Work arrives as Gitea issues or conversation, and leaves as reviewed PRs.
|
||||
is. The agent acknowledges it ("queued myproject#14, 1 ahead"). Your issue is the
|
||||
item's record. Only issues filed or assigned by allowlisted operators count (L11).
|
||||
- **Unassigned issues are backlog,** never work, though the agent will discuss them.
|
||||
- **An issue assigned again after its item closed is a new item,** which notes the
|
||||
one before it; an issue whose item is still open, or whose assignment predates
|
||||
the close, stays with that item.
|
||||
- **Open Gitea dependencies make an issue blocked** (L1).
|
||||
- **Work that starts in conversation gets a record** once it is work on a project,
|
||||
not at its first message: the transcript already keeps the conversation. The
|
||||
|
||||
@@ -261,6 +261,31 @@ class GiteaClient:
|
||||
who = (event.get("user") or {}).get("login")
|
||||
return who
|
||||
|
||||
def assigned_at(self, repo: str, index: int, assignee: str) -> str | None:
|
||||
"""When the issue was last assigned to ``assignee`` (ISO 8601), from
|
||||
its timeline, oldest first and paged to the end; None if the
|
||||
timeline doesn't say."""
|
||||
when = None
|
||||
for page in range(1, MAX_PR_PAGES + 1):
|
||||
events = (
|
||||
self._request(
|
||||
"GET",
|
||||
f"/repos/{repo}/issues/{index}/timeline",
|
||||
params={"limit": PR_PAGE_SIZE, "page": page},
|
||||
)
|
||||
or []
|
||||
)
|
||||
for event in events:
|
||||
if (
|
||||
event.get("type") == "assignees"
|
||||
and not event.get("removed_assignee")
|
||||
and (event.get("assignee") or {}).get("login") == assignee
|
||||
):
|
||||
when = event.get("created_at")
|
||||
if len(events) < PR_PAGE_SIZE:
|
||||
break
|
||||
return when
|
||||
|
||||
def repository(self, repo: str) -> dict[str, Any]:
|
||||
"""The repository as this account sees it, permissions included."""
|
||||
return dict(self._request("GET", f"/repos/{repo}"))
|
||||
|
||||
@@ -22,6 +22,7 @@ import time
|
||||
import uuid
|
||||
from collections.abc import Callable, Iterable, Sequence
|
||||
from dataclasses import dataclass, replace
|
||||
from datetime import UTC, datetime
|
||||
|
||||
from nisse_dev.cancel import Cancel
|
||||
from nisse_dev.canonical import (
|
||||
@@ -87,6 +88,19 @@ _NEXT = re.compile(rf"(?:do|work on) {_ISSUE} (?:next|first|now)")
|
||||
_NAMED = re.compile(r"(go|approve|abandon|stop) (?:([a-z0-9][a-z0-9-]*))?#(\d+)")
|
||||
# At top level these need an item; without one they are ambiguous (L5).
|
||||
_NEEDS_ITEM = frozenset({"go", "approve", "abandon"})
|
||||
# Near-misses for abandon: asked about, never taken for it or for a reply.
|
||||
_NEAR_ABANDON = frozenset(
|
||||
{
|
||||
"abort",
|
||||
"abort it",
|
||||
"cancel",
|
||||
"cancel it",
|
||||
"drop it",
|
||||
"quit",
|
||||
"kill it",
|
||||
"scrap it",
|
||||
}
|
||||
)
|
||||
BUG_GRACE_S = 600.0
|
||||
GENERAL_IDLE_S = 86400.0
|
||||
PROGRESS_EVERY_S = 60.0 # progress in prose at a readable rate (L5, #42)
|
||||
@@ -113,6 +127,9 @@ 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.
|
||||
|
||||
Only the operator stops or closes an item (their stop and abandon commands):
|
||||
never say you have stopped or closed anything yourself.
|
||||
|
||||
Before each group of tool calls, say in a sentence what you're doing and why:
|
||||
the operator sees it as your progress.
|
||||
|
||||
@@ -504,7 +521,8 @@ class LoopAgent:
|
||||
self._oplog.emit("loop.intake_unavailable", cause=str(exc))
|
||||
return
|
||||
for issue in issues:
|
||||
if item_for_issue(self._store, issue.number, project.name) is not None:
|
||||
before = item_for_issue(self._store, issue.number, project.name)
|
||||
if before is not None and not self._assigned_again(project, issue, before):
|
||||
continue
|
||||
if _MARKER.search(issue.body or ""):
|
||||
continue # D1: a record of work started in conversation
|
||||
@@ -521,7 +539,7 @@ class LoopAgent:
|
||||
except GiteaError as exc:
|
||||
self._oplog.emit("loop.intake_unavailable", cause=str(exc))
|
||||
continue # taken on the next round
|
||||
self._open(project, issue, tuple(blocked_by))
|
||||
self._open(project, issue, tuple(blocked_by), follows=before)
|
||||
|
||||
# -- PR review and merge (D4, D5) -------------------------------------------
|
||||
|
||||
@@ -689,8 +707,24 @@ class LoopAgent:
|
||||
return False
|
||||
return who in self._gitea_operators
|
||||
|
||||
def _assigned_again(self, project: Project, issue: Issue, before: WorkItem) -> bool:
|
||||
"""D1: an issue is taken once, and again only when it's assigned to
|
||||
the agent anew after its last item closed (abandoned, say, then
|
||||
given back): never on the same assignment twice."""
|
||||
if before.status == "open" or not before.closed_at:
|
||||
return False
|
||||
try:
|
||||
when = self._gitea.assigned_at(project.repo, issue.number, self._account)
|
||||
except GiteaError:
|
||||
return False
|
||||
return bool(when) and _instant(when) > _instant(before.closed_at)
|
||||
|
||||
def _open(
|
||||
self, project: Project, issue: Issue, blocked_by: tuple[str, ...]
|
||||
self,
|
||||
project: Project,
|
||||
issue: Issue,
|
||||
blocked_by: tuple[str, ...],
|
||||
follows: WorkItem | None = None,
|
||||
) -> None:
|
||||
item_id = f"{project.name}-{issue.number}-{uuid.uuid4().hex[:6]}"
|
||||
cause = f"assigned {project.repo}#{issue.number}"
|
||||
@@ -708,6 +742,7 @@ class LoopAgent:
|
||||
issue=issue.number,
|
||||
blocked_by=blocked_by,
|
||||
labels=labels,
|
||||
follows=follows.id if follows else "",
|
||||
)
|
||||
self._threads[(project.name, issue.number)] = root
|
||||
self._store.open_work_item(
|
||||
@@ -729,6 +764,12 @@ class LoopAgent:
|
||||
if project.labels
|
||||
else ""
|
||||
)
|
||||
+ (
|
||||
f"; assigned again after its earlier item {follows.id} closed "
|
||||
f"({follows.outcome})"
|
||||
if follows
|
||||
else ""
|
||||
)
|
||||
)
|
||||
)
|
||||
text = f"{issue.title}\n\n{issue.body}".strip()
|
||||
@@ -830,6 +871,18 @@ class LoopAgent:
|
||||
elif not self._store.active_item():
|
||||
self._transport.post("Nothing is running.", thread=msg.thread)
|
||||
return
|
||||
if command in _NEAR_ABANDON:
|
||||
# L5: a near-miss for a command is asked about, never read as a
|
||||
# reply (the driver can't stop or close anything itself).
|
||||
self._store.transcript(None, "operator").append(
|
||||
UserMessage(text=text, source=f"transport:{msg.sender}")
|
||||
)
|
||||
self._transport.post(
|
||||
f"Did you mean `abandon`? That closes {self._label(item)} and its "
|
||||
"issue; say abandon to confirm. (`stop` ends a run in progress.)",
|
||||
thread=loop_of(item).thread,
|
||||
)
|
||||
return
|
||||
if command in ("status", "details"):
|
||||
# About the item, not a reply to it: nothing changes, a
|
||||
# countdown runs on (L5). Recorded at the agent's level.
|
||||
@@ -1666,7 +1719,9 @@ class LoopAgent:
|
||||
loop = replace(loop, prepared=True)
|
||||
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
|
||||
hold = (
|
||||
"discuss" in loop.labels and not loop.talked
|
||||
) or loop.started # agreed: a restatement is behind it (D1)
|
||||
if loop.question:
|
||||
hold = True # nothing but answering (below)
|
||||
tools = [
|
||||
@@ -2124,8 +2179,19 @@ class LoopAgent:
|
||||
self._gitea.post_comment(project.repo, loop.issue, f"{outcome}: {note}")
|
||||
self._gitea.close_issue(project.repo, loop.issue)
|
||||
except GiteaError as exc:
|
||||
try:
|
||||
blockers = self._gitea.open_dependencies(project.repo, loop.issue)
|
||||
except GiteaError:
|
||||
blockers = []
|
||||
why = (
|
||||
f"Gitea won't close an issue while it depends on open ones "
|
||||
f"({', '.join(blockers)}): close it when they're done, or "
|
||||
"remove the dependency"
|
||||
if blockers
|
||||
else str(exc)
|
||||
)
|
||||
self._transport.post(
|
||||
f"closed here, but could not close #{loop.issue}: {exc}",
|
||||
f"closed here, but could not close #{loop.issue}: {why}",
|
||||
thread=loop.thread,
|
||||
)
|
||||
if outcome == "abandoned":
|
||||
@@ -2133,3 +2199,9 @@ class LoopAgent:
|
||||
|
||||
def _fact(self, item: WorkItem, text: str) -> None:
|
||||
self._store.transcript(item.id, "orchestrator").append(Fact(text=text))
|
||||
|
||||
|
||||
def _instant(stamp: str) -> datetime:
|
||||
"""An ISO 8601 time from Gitea or the state store, comparable either way."""
|
||||
when = datetime.fromisoformat(stamp)
|
||||
return when if when.tzinfo else when.replace(tzinfo=UTC)
|
||||
@@ -108,9 +108,12 @@ def item_for_thread(
|
||||
def item_for_issue(
|
||||
store: StateStore, issue: int, project: str | None = None
|
||||
) -> WorkItem | None:
|
||||
"""Any item, open or closed, recorded by this issue (in ``project``,
|
||||
when given): intake never takes an issue twice (D1)."""
|
||||
for item in store.work_items():
|
||||
if loop_of(item).issue == issue and project in (None, item.scope):
|
||||
return item
|
||||
return None
|
||||
"""The latest item, open or closed, recorded by this issue (in
|
||||
``project``, when given): intake takes an issue once, and again only on a
|
||||
fresh assignment after its item closed (D1)."""
|
||||
found = [
|
||||
item
|
||||
for item in store.work_items()
|
||||
if loop_of(item).issue == issue and project in (None, item.scope)
|
||||
]
|
||||
return found[-1] if found else None
|
||||
@@ -229,3 +229,30 @@ def test_closing_comments_first_then_closes_issue_and_pr() -> None:
|
||||
("POST", "/api/v1/repos/o/r/issues/20/comments", {"body": "abandoned by don"}),
|
||||
("PATCH", "/api/v1/repos/o/r/pulls/20", {"state": "closed"}),
|
||||
]
|
||||
|
||||
|
||||
def test_assigned_at_is_the_last_assignment_across_pages() -> None:
|
||||
def assigned(login: str, at: str, removed: bool = False) -> dict:
|
||||
return {
|
||||
"type": "assignees",
|
||||
"assignee": {"login": login},
|
||||
"removed_assignee": removed,
|
||||
"created_at": at,
|
||||
}
|
||||
|
||||
filler = {"type": "comment", "created_at": "t0"}
|
||||
pages = {
|
||||
1: [assigned("dev-agent", "t1")] + [filler] * (PR_PAGE_SIZE - 1),
|
||||
2: [
|
||||
assigned("dev-agent", "t2", removed=True),
|
||||
assigned("someone", "t3"),
|
||||
assigned("dev-agent", "t4"),
|
||||
],
|
||||
}
|
||||
|
||||
def handler(request: httpx.Request) -> httpx.Response:
|
||||
assert request.url.path == "/api/v1/repos/o/r/issues/6/timeline"
|
||||
return httpx.Response(200, json=pages[int(request.url.params["page"])])
|
||||
|
||||
assert client(handler).assigned_at("o/r", 6, "dev-agent") == "t4"
|
||||
assert client(handler).assigned_at("o/r", 6, "nobody") is None
|
||||
@@ -49,6 +49,7 @@ class FakeGitea:
|
||||
self.comments: list[tuple[int, str]] = []
|
||||
self.closed: list[int] = []
|
||||
self.deps: dict[int, list[int]] = {}
|
||||
self.reassigned: dict[int, str] = {} # issue -> when assigned again
|
||||
|
||||
def create_issue(self, repo: str, title: str, body: str) -> int:
|
||||
n = max([i.number for i in self.issues] + [0]) + 1
|
||||
@@ -67,6 +68,9 @@ class FakeGitea:
|
||||
def assigned_by(self, repo: str, index: int, assignee: str) -> str:
|
||||
return self.assigner
|
||||
|
||||
def assigned_at(self, repo: str, index: int, assignee: str) -> str | None:
|
||||
return self.reassigned.get(index)
|
||||
|
||||
def post_comment(self, repo: str, index: int, body: str) -> None:
|
||||
self.comments.append((index, body))
|
||||
|
||||
|
||||
@@ -1,14 +1,16 @@
|
||||
"""What the first live session of the new loop found (2026-10-07), each as a
|
||||
test: a request ending on the assistant's side (refused by models without
|
||||
prefill: the driver's prose nudge, a helper's first request), `status` in an
|
||||
item's thread taken as a reply, and no way for the driver to answer a remark.
|
||||
item's thread taken as a reply, no way for the driver to answer a remark;
|
||||
later, near-miss commands, the driver claiming a stop, a reassigned issue
|
||||
ignored, an unexplained failed abandon and restating an agreed item.
|
||||
"""
|
||||
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
|
||||
from model_contract import assert_renderable
|
||||
from test_loop import RESTATED, FakeGitea, Harness, ScriptedModel, call, issue
|
||||
from test_loop import DON, RESTATED, FakeGitea, Harness, ScriptedModel, call, issue
|
||||
from test_loop_help import ROLES, hand_back
|
||||
from test_loop_milestones import RESTATE, Forge, loop_agent, say
|
||||
from test_loop_milestones import item as the_item
|
||||
@@ -152,3 +154,85 @@ def test_a_review_comment_can_be_answered_without_a_fix(
|
||||
assert (loop.wait, loop.pr) == (MERGE, pr)
|
||||
assert forge.posted[-1][0] == pr # answered on the PR, where asked
|
||||
assert "It's a constant on purpose" in forge.posted[-1][1]
|
||||
|
||||
|
||||
# --- the session's later findings ----------------------------------------------
|
||||
|
||||
|
||||
def test_a_near_miss_for_abandon_is_asked_about(tmp_path: Path) -> None:
|
||||
model = ScriptedModel(RESTATED) # called once only: nothing reaches it
|
||||
h = Harness(tmp_path, model, FakeGitea(issue()))
|
||||
h.agent.intake()
|
||||
h.say("abort")
|
||||
assert h.posted()[-1].startswith("Did you mean `abandon`?")
|
||||
assert (h.item().status, loop_of(h.item()).wait) == ("open", GO_AHEAD)
|
||||
h.say("abandon")
|
||||
assert h.item().status == "closed"
|
||||
|
||||
|
||||
def test_the_driver_is_told_it_cant_stop_or_close(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()))
|
||||
h.agent.intake()
|
||||
assert "never say you have stopped or closed anything yourself" in seen[0]
|
||||
|
||||
|
||||
def test_an_issue_assigned_again_after_its_item_closed_is_taken_again(
|
||||
tmp_path: Path,
|
||||
) -> None:
|
||||
gitea = FakeGitea(issue())
|
||||
h = Harness(tmp_path, ScriptedModel(RESTATED, RESTATED), gitea)
|
||||
h.agent.intake()
|
||||
h.say("abandon")
|
||||
[old] = h.store.work_items()
|
||||
gitea.closed.remove(14) # the operator reopened it
|
||||
h.agent.intake()
|
||||
assert len(h.store.work_items()) == 1 # the same assignment: not again
|
||||
gitea.reassigned[14] = "2999-01-01T00:00:00Z" # assigned anew, later
|
||||
h.agent.intake()
|
||||
items = h.store.work_items()
|
||||
assert len(items) == 2
|
||||
new = next(i for i in items if i.id != old.id)
|
||||
assert new.status == "open" and loop_of(new).follows == old.id
|
||||
|
||||
|
||||
def test_abandoning_a_blocked_issue_says_why_it_stays_open(tmp_path: Path) -> None:
|
||||
from nisse_dev.gitea import GiteaError
|
||||
|
||||
class Strict(FakeGitea):
|
||||
def close_issue(self, repo: str, index: int) -> None:
|
||||
if self.open_dependencies(repo, index):
|
||||
raise GiteaError("gitea PATCH returned 500: blocked")
|
||||
super().close_issue(repo, index)
|
||||
|
||||
gitea = Strict(issue(14), issue(15, title="other"))
|
||||
gitea.deps[14] = [15]
|
||||
h = Harness(tmp_path, ScriptedModel(RESTATED), gitea)
|
||||
h.agent._running = True # hold the queue: only the abandon matters
|
||||
h.agent.intake()
|
||||
item = next(i for i in h.store.work_items() if loop_of(i).issue == 14)
|
||||
h.transport.receive(DON, "abandon", thread=loop_of(item).thread)
|
||||
said = " ".join(h.posted())
|
||||
assert "won't close an issue while it depends on open ones" in said
|
||||
assert "don/sandbox#15" in said
|
||||
|
||||
|
||||
def test_an_agreed_item_isnt_offered_restate(world: dict[str, Any]) -> 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)
|
||||
|
||||
model = Spy(RESTATE, [call("ask_human", question="anything else?")])
|
||||
agent, transport = loop_agent(world, model)
|
||||
agent.intake()
|
||||
say(transport, world, "go")
|
||||
assert "restate" in offered[0] and "restate" not in offered[1]
|
||||
@@ -61,6 +61,7 @@ class Forge:
|
||||
self.issues = [Issue(14, "Add count_words", "count words", "don", ())]
|
||||
self.deps: dict[int, list[int]] = {} # issue -> the issues it depends on
|
||||
self.assigned: list[int] = [] # issues the agent assigned to itself
|
||||
self.reassigned: dict[int, str] = {} # issue -> when assigned again
|
||||
|
||||
# intake
|
||||
def assigned_issues(self, repo: str, assignee: str) -> list[Issue]:
|
||||
@@ -84,6 +85,9 @@ class Forge:
|
||||
def assigned_by(self, repo: str, index: int, assignee: str) -> str:
|
||||
return "don"
|
||||
|
||||
def assigned_at(self, repo: str, index: int, assignee: str) -> str | None:
|
||||
return self.reassigned.get(index)
|
||||
|
||||
# PRs
|
||||
def _pr(self, n: int) -> PullRequest:
|
||||
p = self.prs[n]
|
||||
|
||||
@@ -400,7 +400,13 @@ class Walk:
|
||||
where = f"seed {self.seed}, steps {self.steps}"
|
||||
store = self.world["store"]
|
||||
items = self.items()
|
||||
issues = [(i.scope, loop_of(i).issue) for i in items if loop_of(i).issue]
|
||||
# Open items: an issue assigned again after its item closed gets a new
|
||||
# one (D1), which loop.qnt doesn't model.
|
||||
issues = [
|
||||
(i.scope, loop_of(i).issue)
|
||||
for i in items
|
||||
if loop_of(i).issue and i.status == "open"
|
||||
]
|
||||
assert len(issues) == len(set(issues)), f"one_item_per_issue: {where}"
|
||||
for item in items:
|
||||
loop = loop_of(item)
|
||||
|
||||
Reference in new issue
Block a user