nisse/nisse-dev/tests/test_review_loop.py

"""Reacting to PR review comments while in PR_OPEN (architecture D5)."""

import io
from pathlib import Path
from typing import Any

from nisse_dev.canonical import CanonicalEvent, StoredEvent, ToolCall
from nisse_dev.control import AWAIT_INPUT, PR_OPEN, Orchestrator
from nisse_dev.executor import ExecutorOutcome
from nisse_dev.gitea import Comment, GiteaError, PullRequest
from nisse_dev.llm import RoleMap
from nisse_dev.oplog import OpLog
from nisse_dev.planner import LLMPlanningAgent
from nisse_dev.publish import PublishResult
from nisse_dev.state import StateStore
from nisse_dev.transport import InMemoryTransport

ROLES = RoleMap(executor="x", planner="y", delegate="y", consultant="y")


class ScriptedClient:
    def __init__(self) -> None:
        self.calls = 0

    def complete(self, events, tools, model, system=None) -> list[CanonicalEvent]:
        self.calls += 1
        return [
            ToolCall(
                call_id=f"c{self.calls}",
                tool="submit_plan",
                arguments={"type": "x", "content": f"plan {self.calls}"},
            )
        ]


class RecordingExecutor:
    def __init__(self, outcomes: list[str] | None = None) -> None:
        self.runs: list[int] = []
        self.seen_events: list[list[StoredEvent]] = []
        # One entry per run, so a test can let the milestone succeed and
        # only the review response fail.
        self._outcomes = outcomes or []
        self.store: StateStore | None = None

    def run(self, milestone: int) -> ExecutorOutcome:
        self.runs.append(milestone)
        if self.store is not None:
            self.seen_events.append(self.store.read_events())
        status = self._outcomes.pop(0) if self._outcomes else "completed"
        return ExecutorOutcome(status, f"executor said {status}")


class RecordingPublisher:
    def __init__(self, *, sha: str | None = "abc12345", error: Exception | None = None):
        self._sha = sha
        self._error = error
        self.followups: list[dict[str, Any]] = []
        self.comments: list[tuple[int, str]] = []

    def reconcilers(self) -> dict[str, Any]:
        return {}

    def publish(self, **kwargs: Any) -> PublishResult:
        return PublishResult(
            PullRequest(number=42, state="open", head="h", url="u"), "opened PR #42: u"
        )

    def push_followup(self, **kwargs: Any) -> str | None:
        self.followups.append(kwargs)
        if self._error is not None:
            raise self._error
        return self._sha

    def comment(self, index: int, body: str) -> None:
        self.comments.append((index, body))


def build(
    tmp_path: Path,
    executor: Any,
    publisher: Any,
    gitea_operators: set[str] | None = None,
) -> tuple[Orchestrator, StateStore, InMemoryTransport]:
    store = StateStore(tmp_path / "state.db", oplog=OpLog("nisse-dev", io.StringIO()))
    if hasattr(executor, "store"):
        executor.store = store
    transport = InMemoryTransport()
    orch = Orchestrator(
        store,
        transport,
        LLMPlanningAgent(ScriptedClient(), store, ROLES),
        operators={"don"},
        roles=ROLES,
        executor=executor,
        publisher=publisher,
        gitea_operators=gitea_operators if gitea_operators is not None else {"don"},
    )
    orch.start()
    transport.receive("don", "use demo")
    transport.receive("don", "approve")
    transport.receive("don", "approve")
    transport.receive("don", "start milestone 1")
    return orch, store, transport


def review(
    cid: int = 5, author: str = "don", body: str = "rename the module"
) -> Comment:
    return Comment(id=cid, author=author, body=body, created_at="t")


def test_review_comment_triggers_a_fix_and_a_push(tmp_path: Path) -> None:
    executor, publisher = RecordingExecutor(), RecordingPublisher()
    orch, store, transport = build(tmp_path, executor, publisher)
    assert store.get_state().state == PR_OPEN

    orch.handle_review_comment(review())

    assert executor.runs == [1, 1]  # milestone run, then the review response
    assert publisher.followups[0]["milestone"] == 1
    assert "abc12345" in transport.outbound[-1].text
    assert publisher.comments[0][0] == 42  # replied on the PR


def test_comment_enters_the_transcript_before_the_executor_runs(
    tmp_path: Path,
) -> None:
    executor, publisher = RecordingExecutor(), RecordingPublisher()
    orch, _, _ = build(tmp_path, executor, publisher)
    orch.handle_review_comment(review(body="please add a test"))

    # The executor must see the feedback as conversation, not as an
    # out-of-band instruction it was never shown.
    texts = [getattr(e.event, "text", "") for e in executor.seen_events[-1]]
    assert any("please add a test" in t for t in texts)


def test_non_operator_comments_are_ignored(tmp_path: Path) -> None:
    executor, publisher = RecordingExecutor(), RecordingPublisher()
    orch, store, _ = build(tmp_path, executor, publisher)
    orch.handle_review_comment(review(author="drive-by"))

    assert executor.runs == [1]  # only the original milestone run
    assert publisher.followups == []
    assert store.get_state().state == PR_OPEN


def test_comments_outside_pr_open_are_ignored(tmp_path: Path) -> None:
    executor, publisher = RecordingExecutor(), RecordingPublisher()
    orch, _store, transport = build(tmp_path, executor, publisher)
    transport.receive("don", "project complete")  # back to IDLE

    orch.handle_review_comment(review())
    assert executor.runs == [1]


def test_failed_review_response_gates_to_the_operator(tmp_path: Path) -> None:
    # Milestone succeeds; the response to the review comment does not.
    executor = RecordingExecutor(outcomes=["completed", "escalate"])
    publisher = RecordingPublisher()
    orch, store, transport = build(tmp_path, executor, publisher)
    orch.handle_review_comment(review())

    assert store.get_state().state == AWAIT_INPUT
    assert "could not address the review comment" in transport.outbound[-1].text
    assert publisher.followups == []


def test_no_code_change_needed_is_reported(tmp_path: Path) -> None:
    executor, publisher = RecordingExecutor(), RecordingPublisher(sha=None)
    orch, _, transport = build(tmp_path, executor, publisher)
    orch.handle_review_comment(review(body="just curious, why sqlite?"))

    assert "no code change was needed" in transport.outbound[-1].text


def test_push_failure_is_reported_without_losing_pr_open(tmp_path: Path) -> None:
    executor = RecordingExecutor()
    publisher = RecordingPublisher(error=GiteaError("push rejected"))
    orch, store, transport = build(tmp_path, executor, publisher)
    orch.handle_review_comment(review())

    assert "could not push the fix" in transport.outbound[-1].text
    assert store.get_state().state == PR_OPEN  # still reviewable, can retry