Match approvals and commands regardless of case and trailing punctuation #72
No files matched your search
@@ -23,7 +23,14 @@ from typing import Any, Protocol
|
|||||||
|
|
||||||
from nisse_dev.cancel import Cancel
|
from nisse_dev.cancel import Cancel
|
||||||
from nisse_dev.canonical import GateOpened, PlanSubmitted, UserMessage
|
from nisse_dev.canonical import GateOpened, PlanSubmitted, UserMessage
|
||||||
from nisse_dev.gates import Approved, Gate, Ignored, PlanRevision, Rejected
|
from nisse_dev.gates import (
|
||||||
|
Approved,
|
||||||
|
Gate,
|
||||||
|
Ignored,
|
||||||
|
PlanRevision,
|
||||||
|
Rejected,
|
||||||
|
command_form,
|
||||||
|
)
|
||||||
from nisse_dev.git import GitError
|
from nisse_dev.git import GitError
|
||||||
from nisse_dev.gitea import GiteaError
|
from nisse_dev.gitea import GiteaError
|
||||||
from nisse_dev.llm import RoleMap
|
from nisse_dev.llm import RoleMap
|
||||||
@@ -165,6 +172,12 @@ class StubAgent:
|
|||||||
return SubmitPlan(f"[stub] {gate} plan for {project}{notes}")
|
return SubmitPlan(f"[stub] {gate} plan for {project}{notes}")
|
||||||
|
|
||||||
|
|
||||||
|
def _project_name(text: str) -> str:
|
||||||
|
"""The name in 'use <name>', as written: only the command word is
|
||||||
|
case-insensitive."""
|
||||||
|
return text.strip().rstrip(".!")[len("use") :].strip()
|
||||||
|
|
||||||
|
|
||||||
def _fresh_data() -> dict[str, Any]:
|
def _fresh_data() -> dict[str, Any]:
|
||||||
return {
|
return {
|
||||||
"revs": {},
|
"revs": {},
|
||||||
@@ -331,7 +344,7 @@ class Orchestrator:
|
|||||||
if msg.sender not in self._operators:
|
if msg.sender not in self._operators:
|
||||||
return
|
return
|
||||||
text = msg.text.strip()
|
text = msg.text.strip()
|
||||||
if text == "stop":
|
if command_form(text) == "stop":
|
||||||
# Straight to the flag, not behind the work in the queue: the
|
# Straight to the flag, not behind the work in the queue: the
|
||||||
# work is what it has to reach (A4).
|
# work is what it has to reach (A4).
|
||||||
if self._worker.busy:
|
if self._worker.busy:
|
||||||
@@ -434,9 +447,10 @@ class Orchestrator:
|
|||||||
state = self._store.get_state()
|
state = self._store.get_state()
|
||||||
current = state.state if state else IDLE
|
current = state.state if state else IDLE
|
||||||
text = msg.text.strip()
|
text = msg.text.strip()
|
||||||
if text == "status":
|
command = command_form(text)
|
||||||
|
if command == "status":
|
||||||
self._send_status(current)
|
self._send_status(current)
|
||||||
elif text == "abandon":
|
elif command == "abandon":
|
||||||
# Like status, a command in every waiting state: it takes
|
# Like status, a command in every waiting state: it takes
|
||||||
# precedence over reading the word as an answer or feedback.
|
# precedence over reading the word as an answer or feedback.
|
||||||
if current == IDLE:
|
if current == IDLE:
|
||||||
@@ -444,8 +458,8 @@ class Orchestrator:
|
|||||||
else:
|
else:
|
||||||
self._abandon(msg.sender)
|
self._abandon(msg.sender)
|
||||||
elif current == IDLE:
|
elif current == IDLE:
|
||||||
if text.startswith("use "):
|
if command.startswith("use "):
|
||||||
self._begin_project(text[4:].strip())
|
self._begin_project(_project_name(text))
|
||||||
else:
|
else:
|
||||||
self._transport.send_message("no active project; say 'use <project>'")
|
self._transport.send_message("no active project; say 'use <project>'")
|
||||||
elif current in (AWAIT_ARCH_SIGNOFF, AWAIT_MILESTONE_SIGNOFF):
|
elif current in (AWAIT_ARCH_SIGNOFF, AWAIT_MILESTONE_SIGNOFF):
|
||||||
@@ -592,7 +606,7 @@ class Orchestrator:
|
|||||||
name, rev = gate_data["name"], gate_data["rev"]
|
name, rev = gate_data["name"], gate_data["rev"]
|
||||||
milestone = self._data["milestone"]
|
milestone = self._data["milestone"]
|
||||||
if gate_data["approved"]:
|
if gate_data["approved"]:
|
||||||
if text == f"start milestone {milestone}":
|
if command_form(text) == f"start milestone {milestone}":
|
||||||
self._transition(
|
self._transition(
|
||||||
EXECUTING, cause=f"operator command: start milestone {milestone}"
|
EXECUTING, cause=f"operator command: start milestone {milestone}"
|
||||||
)
|
)
|
||||||
@@ -941,14 +955,15 @@ class Orchestrator:
|
|||||||
)
|
)
|
||||||
|
|
||||||
def _handle_pr_open(self, text: str) -> None:
|
def _handle_pr_open(self, text: str) -> None:
|
||||||
if text == "next milestone":
|
command = command_form(text)
|
||||||
|
if command == "next milestone":
|
||||||
self._unwatch("review")
|
self._unwatch("review")
|
||||||
self._data["milestone"] = int(self._data.get("milestone", 1)) + 1
|
self._data["milestone"] = int(self._data.get("milestone", 1)) + 1
|
||||||
self._data["gate"] = None
|
self._data["gate"] = None
|
||||||
self._data["pr"] = None
|
self._data["pr"] = None
|
||||||
self._transition(MILESTONE_PLANNING, cause="operator: next milestone")
|
self._transition(MILESTONE_PLANNING, cause="operator: next milestone")
|
||||||
self._run_planning_turn(f"milestone-{self._data['milestone']}")
|
self._run_planning_turn(f"milestone-{self._data['milestone']}")
|
||||||
elif text == "project complete":
|
elif command == "project complete":
|
||||||
self.stop_watching()
|
self.stop_watching()
|
||||||
self._data = {**_fresh_data()}
|
self._data = {**_fresh_data()}
|
||||||
self._project = None
|
self._project = None
|
||||||
@@ -1022,9 +1037,9 @@ class Orchestrator:
|
|||||||
if pending is None:
|
if pending is None:
|
||||||
# Reached only if state was forced externally or a pending item
|
# Reached only if state was forced externally or a pending item
|
||||||
# was lost; starting over must still work from here.
|
# was lost; starting over must still work from here.
|
||||||
if text.startswith("use "):
|
if command_form(text).startswith("use "):
|
||||||
self.stop_watching()
|
self.stop_watching()
|
||||||
self._begin_project(text[4:].strip())
|
self._begin_project(_project_name(text))
|
||||||
return
|
return
|
||||||
self._transport.send_message(
|
self._transport.send_message(
|
||||||
"waiting for direction, but I have no pending item to resume; "
|
"waiting for direction, but I have no pending item to resume; "
|
||||||
@@ -1061,7 +1076,7 @@ class Orchestrator:
|
|||||||
"""Answer to the question a restart asks (A3 step 7). Never a blind
|
"""Answer to the question a restart asks (A3 step 7). Never a blind
|
||||||
resume: 'continue' re-runs the interrupted planning turn, or returns
|
resume: 'continue' re-runs the interrupted planning turn, or returns
|
||||||
an interrupted milestone to its approved plan to be started again."""
|
an interrupted milestone to its approved plan to be started again."""
|
||||||
choice = text.strip().rstrip(".!").lower()
|
choice = command_form(text)
|
||||||
if choice not in ("continue", "re-plan", "replan"):
|
if choice not in ("continue", "re-plan", "replan"):
|
||||||
self._data["pending"] = pending
|
self._data["pending"] = pending
|
||||||
self._transport.ask_question(_BOOT_CHOICES)
|
self._transport.ask_question(_BOOT_CHOICES)
|
||||||
|
|||||||
@@ -50,6 +50,14 @@ class Ignored:
|
|||||||
_APPROVE_RE = re.compile(r"^approve(?:\s+(rev-\d+))?$")
|
_APPROVE_RE = re.compile(r"^approve(?:\s+(rev-\d+))?$")
|
||||||
|
|
||||||
|
|
||||||
|
def command_form(text: str) -> str:
|
||||||
|
"""How a message reads as a command: case, surrounding whitespace, runs
|
||||||
|
of inner whitespace and trailing '.' or '!' do not matter, so a phone's
|
||||||
|
"Approve." is still an approval. Only for matching; what the operator
|
||||||
|
wrote is what gets recorded and passed on."""
|
||||||
|
return " ".join(text.strip().rstrip(".!").split()).lower()
|
||||||
|
|
||||||
|
|
||||||
class Gate:
|
class Gate:
|
||||||
def __init__(self, revision: PlanRevision, operators: AbstractSet[str]) -> None:
|
def __init__(self, revision: PlanRevision, operators: AbstractSet[str]) -> None:
|
||||||
self._revision = revision
|
self._revision = revision
|
||||||
@@ -61,7 +69,7 @@ class Gate:
|
|||||||
if sender not in self._operators:
|
if sender not in self._operators:
|
||||||
return Ignored(notice=None)
|
return Ignored(notice=None)
|
||||||
text = text.strip()
|
text = text.strip()
|
||||||
match = _APPROVE_RE.match(text)
|
match = _APPROVE_RE.match(command_form(text))
|
||||||
if match is None:
|
if match is None:
|
||||||
return Rejected(feedback=text, approver=sender, channel=channel)
|
return Rejected(feedback=text, approver=sender, channel=channel)
|
||||||
named = match.group(1)
|
named = match.group(1)
|
||||||
|
|||||||
@@ -269,3 +269,17 @@ def test_use_starts_over_when_nothing_is_pending(h: Harness) -> None:
|
|||||||
h.store.transition(AWAIT_INPUT, cause="forced", project="demo", data={})
|
h.store.transition(AWAIT_INPUT, cause="forced", project="demo", data={})
|
||||||
h.transport.receive("don", "use demo")
|
h.transport.receive("don", "use demo")
|
||||||
assert h.state == AWAIT_ARCH_SIGNOFF
|
assert h.state == AWAIT_ARCH_SIGNOFF
|
||||||
|
|
||||||
|
|
||||||
|
def test_commands_ignore_case_but_the_project_name_keeps_it(h: Harness) -> None:
|
||||||
|
h.transport.receive("don", "Use Demo.")
|
||||||
|
assert h.state == AWAIT_ARCH_SIGNOFF
|
||||||
|
assert h.store.get_state().project == "Demo"
|
||||||
|
h.transport.receive("don", "Status")
|
||||||
|
assert "AWAIT_ARCH_SIGNOFF" in h.outbound_texts()[-1]
|
||||||
|
h.transport.receive("don", "Approve")
|
||||||
|
h.transport.receive("don", "Approve.")
|
||||||
|
h.transport.receive("don", "Start Milestone 1")
|
||||||
|
assert h.state == PR_OPEN
|
||||||
|
h.transport.receive("don", "Project Complete!")
|
||||||
|
assert h.state == IDLE
|
||||||
@@ -1,3 +1,5 @@
|
|||||||
|
import pytest
|
||||||
|
|
||||||
from nisse_dev.gates import Approved, Gate, Ignored, PlanRevision, Rejected
|
from nisse_dev.gates import Approved, Gate, Ignored, PlanRevision, Rejected
|
||||||
|
|
||||||
REVISION = PlanRevision(gate="arch", rev=2, content="the plan")
|
REVISION = PlanRevision(gate="arch", rev=2, content="the plan")
|
||||||
@@ -46,3 +48,22 @@ def test_approve_with_trailing_words_is_rejection() -> None:
|
|||||||
# typed word approve, nothing else).
|
# typed word approve, nothing else).
|
||||||
result = gate().evaluate("don", "approve, but rename the module", "transport")
|
result = gate().evaluate("don", "approve, but rename the module", "transport")
|
||||||
assert isinstance(result, Rejected)
|
assert isinstance(result, Rejected)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
"text", ["Approve", "APPROVE", "Approve.", "approve!", " Approve rev-2 "]
|
||||||
|
)
|
||||||
|
def test_approval_ignores_case_and_trailing_punctuation(text: str) -> None:
|
||||||
|
# A phone keyboard's "Approve" used to count as rejection feedback (#64).
|
||||||
|
assert isinstance(gate().evaluate("don", text, channel="transport"), Approved)
|
||||||
|
|
||||||
|
|
||||||
|
def test_a_capitalised_stale_approval_is_still_refused() -> None:
|
||||||
|
result = gate().evaluate("don", "Approve REV-1", channel="transport")
|
||||||
|
assert isinstance(result, Ignored)
|
||||||
|
|
||||||
|
|
||||||
|
def test_a_sentence_containing_approve_is_still_feedback() -> None:
|
||||||
|
result = gate().evaluate("don", "I approve of most of it", channel="transport")
|
||||||
|
assert isinstance(result, Rejected)
|
||||||
|
assert result.feedback == "I approve of most of it"
|
||||||
@@ -5,6 +5,8 @@ import threading
|
|||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from typing import Any
|
from typing import Any
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
from nisse_dev.cancel import STOPPED_RESULT, Cancel
|
from nisse_dev.cancel import STOPPED_RESULT, Cancel
|
||||||
from nisse_dev.canonical import AssistantText, ToolCall, ToolResult, UserMessage
|
from nisse_dev.canonical import AssistantText, ToolCall, ToolResult, UserMessage
|
||||||
from nisse_dev.control import (
|
from nisse_dev.control import (
|
||||||
@@ -207,7 +209,8 @@ class BlocksUntilStopped:
|
|||||||
return ExecutorOutcome("completed", "never stopped")
|
return ExecutorOutcome("completed", "never stopped")
|
||||||
|
|
||||||
|
|
||||||
def test_stop_reaches_work_running_on_the_worker(tmp_path: Path) -> None:
|
@pytest.mark.parametrize("word", ["stop", "Stop!", " STOP "])
|
||||||
|
def test_stop_reaches_work_running_on_the_worker(tmp_path: Path, word: str) -> None:
|
||||||
# The bug this design fixes: the work used to run on the transport's own
|
# The bug this design fixes: the work used to run on the transport's own
|
||||||
# thread, so no message, stop included, was read until it finished.
|
# thread, so no message, stop included, was read until it finished.
|
||||||
worker = SerialWorker()
|
worker = SerialWorker()
|
||||||
@@ -231,7 +234,7 @@ def test_stop_reaches_work_running_on_the_worker(tmp_path: Path) -> None:
|
|||||||
assert executor.running.wait(5)
|
assert executor.running.wait(5)
|
||||||
|
|
||||||
transport.receive("don", "status") # delivered while the milestone runs
|
transport.receive("don", "status") # delivered while the milestone runs
|
||||||
transport.receive("don", "stop")
|
transport.receive("don", word) # a phone may capitalise it (#64)
|
||||||
worker.call(lambda: None) # wait for the stop to be recorded
|
worker.call(lambda: None) # wait for the stop to be recorded
|
||||||
|
|
||||||
texts = [m.text for m in transport.outbound]
|
texts = [m.text for m in transport.outbound]
|
||||||
|
|||||||
Reference in new issue
Block a user