orchestrator/agent-team/agent_team/nodes/review_loop.py

"""GPT-4.1 review-loop node (design §3.3, §7.1 Phase P2).

The review loop is the adversarial plan-review stage of the Plane-2 pipeline::

    INTAKE -> CLARIFIER -> PLANNER -> REVIEW LOOP -> BUILDERS -> ...

After the planner produces a phased plan, this node runs an **adversarial
cross-family review** of that plan — the ``sh-plan-review`` / ``cross_reviewer``
discipline (GPT-4.1, a different model family than the Claude planner, so it
catches different blind spots). The reviewer returns a verdict:

* ``APPROVE`` — the plan clears the bar; the task advances to the builders.
* ``REQUEST_CHANGES`` — the plan has gaps; the task **loops back to the
  planner** carrying the reviewer's findings, and the planner revises.

The loop is bounded. After ``max_rounds`` of ``REQUEST_CHANGES`` without
convergence the node **escalates to Adam** (parks the task with an ALARM)
rather than spinning — design §3.3 stability bound (6): "a task that stalls ...
parks and ALARMs rather than spinning", and §7.1 P2 "including loop-back and
the escalate-to-Adam path".

Design notes honoured here:

* The reviewer is **GPT-4.1 via the orchestrator's** ``cross_reviewer`` agent,
  reached through the local ``run.py`` (design §3.2: the R720 coordinator calls
  the local ``~/orchestrator/run.py`` for non-Claude single-shot sub-tasks, so
  they keep API billing + LangSmith tracing). It is therefore **not** routed
  through :mod:`agent_team.billing` (which is the *Claude* seam). The actual
  call is delegated to an injectable :data:`ReviewInvoker` so this node stays
  dependency-free and unit-testable, mirroring the ``billing.set_invoker``
  pattern in the foundation.
* The node is a pure function over
  :class:`~agent_team.task_model.PipelineState`: it returns only the partial
  state keys it changes (``review_verdicts``, ``current_phase``, ``status``),
  which the LangGraph reducer merges. It never writes to repos and performs no
  durable I/O of its own — the SQLite checkpointer persists the merged state.
* :func:`route_after_review` is the LangGraph conditional-edge function that
  reads the verdict this node recorded and returns the next node name
  (``"build"`` / ``"plan"`` / ``"parked"``).
"""

from __future__ import annotations

import json
import os
import subprocess
from dataclasses import dataclass
from datetime import datetime, timezone
from enum import Enum
from typing import Any, Callable, Mapping

from agent_team.task_model import Phase, PipelineState, TaskStatus

__all__ = [
    "DEFAULT_MAX_REVIEW_ROUNDS",
    "ReviewInvoker",
    "ReviewOutcome",
    "ReviewResult",
    "ReviewVerdict",
    "review_node",
    "route_after_review",
    "set_review_invoker",
]

# Node names this stage routes to (LangGraph graph node ids). Kept as module
# constants so the conditional-edge mapping and the node body cannot drift.
BUILD_NODE = "build"
PLAN_NODE = "plan"
PARKED_NODE = "parked"

# Default cap on adversarial review rounds before the loop escalates to Adam
# (design §3.3 stability bound 6; §7.1 P2 escalate-to-Adam path). Overridable
# per task via config["max_review_rounds"].
DEFAULT_MAX_REVIEW_ROUNDS = 3

# Config / env key naming the per-task review-round cap.
_MAX_ROUNDS_CONFIG_KEY = "max_review_rounds"
_MAX_ROUNDS_ENV = "AGENT_TEAM_MAX_REVIEW_ROUNDS"

# Config key naming the orchestrator entry point (the local run.py). Defaults
# to the rsync'd path on the R720; overridable for tests / non-default installs.
_RUN_PY_CONFIG_KEY = "orchestrator_run_py"
_DEFAULT_RUN_PY = os.path.expanduser("~/Documents/repositories/orchestrator/run.py")

# Verdict tokens the reviewer output is scanned for. REQUEST_CHANGES wins on a
# tie so an ambiguous review fails closed (loops back / escalates) rather than
# advancing a plan the reviewer flagged.
_APPROVE_TOKENS = ("APPROVE", "APPROVED", "LGTM", "NO BLOCKERS", "NO BLOCKING")
_CHANGES_TOKENS = (
    "REQUEST CHANGES",
    "REQUEST_CHANGES",
    "REQUESTCHANGES",
    "BLOCK",
    "BLOCKING",
    "NEEDS CHANGES",
    "NEEDS WORK",
)


class ReviewVerdict(Enum):
    """The adversarial reviewer's verdict on a plan (design §3.3)."""

    APPROVE = "approve"
    REQUEST_CHANGES = "request_changes"


class ReviewOutcome(Enum):
    """What the loop decided to do after recording a verdict.

    ``APPROVED`` — advance to the builders. ``LOOP_BACK`` — return to the
    planner with the findings. ``ESCALATE`` — the round cap was hit without
    convergence; park the task and ALARM Adam (design §3.3 bound 6, §7.1 P2).
    """

    APPROVED = "approved"
    LOOP_BACK = "loop_back"
    ESCALATE = "escalate"


@dataclass
class ReviewResult:
    """One adversarial review pass, appended to ``review_verdicts`` (design §3.3).

    ``verdict`` is the parsed :class:`ReviewVerdict`; ``round_index`` is the
    1-based review round; ``outcome`` is the loop decision this pass produced;
    ``findings`` is the reviewer's full text (the planner consumes it on a
    loop-back); ``reviewer`` records the agent/model for the audit trail;
    ``raw`` is the untouched invoker payload; ``created_at`` is an ISO-8601 UTC
    timestamp.
    """

    verdict: ReviewVerdict
    round_index: int
    outcome: ReviewOutcome
    findings: str
    reviewer: str = "cross_reviewer"
    raw: Any = None
    created_at: str | None = None

    def to_dict(self) -> dict[str, Any]:
        """Serialize to a JSON-safe dict for the ``review_verdicts`` ledger."""
        return {
            "verdict": self.verdict.value,
            "round_index": self.round_index,
            "outcome": self.outcome.value,
            "findings": self.findings,
            "reviewer": self.reviewer,
            "created_at": self.created_at,
        }


# Pluggable reviewer call: signature (prompt, **kw) -> str (the reviewer's
# text). The default shells out to the orchestrator's cross_reviewer via the
# local run.py. Leaves/tests rebind it with set_review_invoker().
ReviewInvoker = Callable[..., str]


def _orchestrator_invoker(prompt: str, *, run_py: str, **_kw: Any) -> str:
    """Default reviewer: call the orchestrator's ``cross_reviewer`` (GPT-4.1).

    Invokes the local ``run.py`` with the review prompt. The orchestrator's
    router sends adversarial-review tasks to ``cross_reviewer`` (GPT-4.1); this
    keeps the review cross-family (a different model than the Claude planner)
    and API-billed + LangSmith-traced per design §3.2. Returns the orchestrator's
    stdout (the reviewer's verdict + findings).
    """
    if not os.path.exists(run_py):
        raise FileNotFoundError(
            f"orchestrator entry point not found: {run_py}; set "
            f"config[{_RUN_PY_CONFIG_KEY!r}] or rebind via set_review_invoker()."
        )
    completed = subprocess.run(  # noqa: S603 - args are not shell-interpolated
        ["python3", run_py, prompt],
        capture_output=True,
        text=True,
        check=False,
    )
    if completed.returncode != 0:
        raise RuntimeError(
            "orchestrator review call failed "
            f"(exit {completed.returncode}): {completed.stderr.strip()}"
        )
    return completed.stdout


_review_invoker: ReviewInvoker = _orchestrator_invoker


def set_review_invoker(invoker: ReviewInvoker) -> None:
    """Bind the function that performs the adversarial review call.

    Leaves call this once at startup with an implementation that returns the
    reviewer's text for a prompt. Keeping the call injectable keeps this node
    dependency-free and unit-testable (mirrors ``billing.set_invoker``).
    """
    global _review_invoker
    _review_invoker = invoker


def _utcnow_iso() -> str:
    """Return the current UTC time as an ISO-8601 string."""
    return datetime.now(timezone.utc).isoformat()


def _resolve_max_rounds(config: Mapping[str, Any] | None) -> int:
    """Resolve the review-round cap from config, then env, then the default.

    A non-positive or non-integer value is rejected so a misconfigured cap
    cannot turn the bounded loop into an unbounded one.
    """
    raw: Any = None
    if config is not None:
        raw = config.get(_MAX_ROUNDS_CONFIG_KEY)
    if raw is None:
        raw = os.environ.get(_MAX_ROUNDS_ENV)
    if raw is None:
        return DEFAULT_MAX_REVIEW_ROUNDS
    try:
        value = int(raw)
    except (TypeError, ValueError) as exc:
        raise ValueError(
            f"invalid {_MAX_ROUNDS_CONFIG_KEY!r}={raw!r}; expected a positive int"
        ) from exc
    if value < 1:
        raise ValueError(f"invalid {_MAX_ROUNDS_CONFIG_KEY!r}={value!r}; must be >= 1")
    return value


def _resolve_run_py(config: Mapping[str, Any] | None) -> str:
    """Resolve the orchestrator ``run.py`` path from config, env, or default."""
    if config is not None:
        configured = config.get(_RUN_PY_CONFIG_KEY)
        if configured:
            return os.path.expanduser(str(configured))
    env = os.environ.get("AGENT_TEAM_ORCHESTRATOR_RUN_PY")
    if env:
        return os.path.expanduser(env)
    return _DEFAULT_RUN_PY


def parse_verdict(text: str) -> ReviewVerdict:
    """Parse a :class:`ReviewVerdict` from the reviewer's free text.

    Scans for explicit ``REQUEST CHANGES`` / ``BLOCK`` tokens and ``APPROVE`` /
    ``LGTM`` tokens (case-insensitive). The result **fails closed**: if a
    change-requesting token is present, or if neither token class is present
    (an ambiguous / empty review), the verdict is ``REQUEST_CHANGES`` so an
    unclear review never silently advances a plan to the builders.
    """
    haystack = (text or "").upper()
    has_changes = any(token in haystack for token in _CHANGES_TOKENS)
    has_approve = any(token in haystack for token in _APPROVE_TOKENS)
    if has_changes:
        return ReviewVerdict.REQUEST_CHANGES
    if has_approve:
        return ReviewVerdict.APPROVE
    # Ambiguous / empty review -> fail closed.
    return ReviewVerdict.REQUEST_CHANGES


def build_review_prompt(state: PipelineState) -> str:
    """Compose the adversarial-review prompt sent to the GPT-4.1 reviewer.

    Embeds the planner's phased plan plus any prior-round findings, and asks for
    a structured verdict the node can parse. Kept deterministic so the prompt is
    testable and the verdict tokens line up with :func:`parse_verdict`.
    """
    plan = state.get("plan") or {}
    plan_text = json.dumps(plan, indent=2, sort_keys=True)

    prior = state.get("review_verdicts") or []
    prior_text = ""
    if prior:
        last = prior[-1]
        if isinstance(last, Mapping):
            prior_text = (
                "\n\nThis plan was REVISED after prior review feedback. The "
                "previous round's findings were:\n"
                f"{last.get('findings', '')}\n"
                "Confirm they are resolved and look for anything new."
            )

    return (
        "You are the adversarial plan reviewer (sh-plan-review / cross_reviewer "
        "discipline). Audit the following phased plan for flawed assumptions, "
        "missing phases, ordering hazards, and convention violations.\n\n"
        "Respond with a verdict line that is exactly one of:\n"
        "  VERDICT: APPROVE\n"
        "  VERDICT: REQUEST CHANGES\n"
        "followed by your findings. Default to REQUEST CHANGES if anything is "
        "unclear or risky.\n\n"
        f"PLAN:\n{plan_text}"
        f"{prior_text}"
    )


def _review_round_index(state: PipelineState) -> int:
    """Return the 1-based index of the review round about to run.

    Counts only prior *review* verdict entries already in ``review_verdicts``
    (entries this node appended), so a loop-back/re-entry increments correctly.
    """
    prior = state.get("review_verdicts") or []
    return len(prior) + 1


def review_node(
    state: PipelineState,
    config: Mapping[str, Any] | None = None,
) -> PipelineState:
    """LangGraph node: run one adversarial review round on the current plan.

    Returns a **partial** :class:`PipelineState` the reducer merges:

    * ``review_verdicts`` — the prior verdicts **plus** this round's
      :class:`ReviewResult` (as a dict). (Returned as the full list rather than
      a single item so the node is reducer-agnostic — it works whether or not
      ``review_verdicts`` has an append-reducer configured.)
    * ``current_phase`` / ``status`` — set per the loop decision:
        - ``APPROVE`` -> phase ``BUILD``, status ``ACTIVE`` (advance to builders).
        - ``REQUEST_CHANGES`` under the round cap -> phase ``PLAN``, status
          ``ACTIVE`` (loop back to the planner with the findings).
        - ``REQUEST_CHANGES`` at/over the round cap -> phase ``PARKED``, status
          ``PARKED`` (escalate to Adam / ALARM; design §3.3 bound 6, §7.1 P2).
    * ``updated_at`` — refreshed ISO-8601 UTC timestamp.

    The plan is required; calling this node with no ``plan`` in state is a
    programming error (the planner runs first) and raises ``ValueError``.
    """
    if not state.get("plan"):
        raise ValueError(
            "review_node requires a plan in state; the planner stage must run "
            "before the review loop (design §3.3 INTAKE->...->PLANNER->REVIEW)."
        )

    max_rounds = _resolve_max_rounds(config)
    run_py = _resolve_run_py(config)
    round_index = _review_round_index(state)

    prompt = build_review_prompt(state)
    raw = _review_invoker(prompt, run_py=run_py, config=config)
    text = raw if isinstance(raw, str) else str(raw)
    verdict = parse_verdict(text)

    if verdict is ReviewVerdict.APPROVE:
        outcome = ReviewOutcome.APPROVED
        next_phase = Phase.BUILD
        next_status = TaskStatus.ACTIVE
    elif round_index >= max_rounds:
        # Cap reached without convergence -> escalate to Adam (park + ALARM).
        outcome = ReviewOutcome.ESCALATE
        next_phase = Phase.PARKED
        next_status = TaskStatus.PARKED
    else:
        outcome = ReviewOutcome.LOOP_BACK
        next_phase = Phase.PLAN
        next_status = TaskStatus.ACTIVE

    result = ReviewResult(
        verdict=verdict,
        round_index=round_index,
        outcome=outcome,
        findings=text,
        raw=raw,
        created_at=_utcnow_iso(),
    )

    prior = list(state.get("review_verdicts") or [])
    prior.append(result.to_dict())

    update: PipelineState = {
        "review_verdicts": prior,
        "current_phase": next_phase.value,
        "status": next_status.value,
        "updated_at": _utcnow_iso(),
    }
    return update


def route_after_review(state: PipelineState) -> str:
    """LangGraph conditional-edge: next node after the review loop.

    Reads the outcome of the most recent verdict this node recorded and maps it
    to a node id: ``APPROVED`` -> ``"build"``, ``LOOP_BACK`` -> ``"plan"``,
    ``ESCALATE`` -> ``"parked"``. A state with no recorded verdict is a
    programming error (route is called after :func:`review_node`) and parks
    fail-closed rather than advancing.
    """
    verdicts = state.get("review_verdicts") or []
    if not verdicts:
        return PARKED_NODE
    last = verdicts[-1]
    outcome = last.get("outcome") if isinstance(last, Mapping) else None
    if outcome == ReviewOutcome.APPROVED.value:
        return BUILD_NODE
    if outcome == ReviewOutcome.LOOP_BACK.value:
        return PLAN_NODE
    # ESCALATE or any unexpected value -> park fail-closed.
    return PARKED_NODE