Plan review

1"""Plan review — one gstack review over a plan, as a step the agent runs.23Replicates the reviews in gstack's plan flow, using the passes each gstack4review actually runs (`plan-*-review/sections/review-sections.md`, office-hours'5`design-and-handoff.md`):67    office-hours spec review ─> CEO ─> Design ─> DevEx ─> Eng89## Where each part of gstack lives in a workspace1011A workspace is a chat AND a resource graph, driven by INSTRUCTIONS. gstack's12review is split across all three, and this node is only the middle one:1314  - the INSTRUCTIONS are the program: which reviews run for this plan (Design15    only with UI scope, DevEx only with DX scope), the spec review's five-run16    cap, the order, the gates;17  - the CHAT is where a human decides: the agent passes this node's `decisions`18    straight to `requestUserDecision`. At the end of the decision step it appends19    the answers to the canvas's "Decisions" node, which feeds this node's20    `decisions` input by edge, and reruns it, which applies them;21  - this NODE is the computation: one lens over one plan, persisted on the22    graph so it re-runs when the plan changes, runs on whatever model the node23    picks, and keeps five reviews of a long document out of the agent's 25k24    loaded-section budget.2526So it always completes. An earlier version stopped with `outcome: waiting` and27asked through a lane's question card; that duplicated `requestUserDecision`28(worse — no option descriptions, no multi-select), gave the user two places to29answer one question, and contradicted the approved design, which already said30"Review in M1 is a requestUserDecision chat gate (chat-turn tool, not a node)".3132## The decisions output3334Every entry is a ready-made `requestUserDecision` question — `{key, question,35header, context, recommendation, options: [{id, label, description,36recommended}]}` — ordered by what blocks what. The option the review37recommends is flagged and its reason is the `recommendation` line: the card38marks it, as gstack marks one option "(recommended)", and pre-selects nothing.39The agent asks them (≤4 per call, the tool's recommended limit), appends the40answers to "Decisions" — wired into this node's `decisions` — and reruns it:41answered questions are applied and never proposed again — neither verbatim nor reworded42or narrowed (`refines`), which becomes a minor finding for the implementer. A43rerun — this review's own questions answered, told apart by each row's `lane` —44asks only while something still blocks; with nothing blocking, what it would45ask goes to the implementer too (route_decisions).4647`decisions` comes by edge from the one "Decisions" list every review and Lane48reads (2026-09-28). It used to be each review's own copy, written by the agent;49rewriting those copies lost answers. A shared list marks finished reviews out50of date on every answer — but once the agent revises the design doc after each51answer, they are out of date anyway. Each row's `lane` names the review that52asked, which is how a run tells its own answered questions from earlier53reviews' (asked_here).54"""5556import asyncio57import json58import re59import sys60import traceback61from typing import Any6263from gais import Gais6465LENSES = {66    # office-hours, "Spec Review Loop": the adversarial pass on the DOCUMENT,67    # run before any of the autoplan phases.68    "spec": {69        "who": "an adversarial spec reviewer with no stake in this plan",70        "source": "gstack /office-hours spec review",71        "passes": [72            "Completeness — what a reader needs to build this that the document does not say",73            "Consistency — where two sections of the document contradict each other",74            "Clarity — which sentences two engineers would implement differently",75            "Scope — work that is in the document but not needed for its stated goal",76            "Feasibility — what the document assumes is possible that may not be",77        ],78    },79    "ceo": {80        "who": "a founder-CEO reviewing whether this is the right scope, built right",81        "source": "gstack /plan-ceo-review, 11 sections",82        "passes": [83            "Architecture",84            "Error & rescue map — every failure, what the user sees, how they recover",85            "Security & threat model",86            "Data flow & interaction edge cases",87            "Code quality",88            "Tests",89            "Performance",90            "Observability & debuggability",91            "Deployment & rollout",92            "Long-term trajectory — where this is in a year",93            "Design & UX — ONLY if the plan has a user interface",94        ],95    },96    "design": {97        "who": "a product designer who will be blamed for what the user sees",98        "source": "gstack /plan-design-review, 7 passes",99        "passes": [100            "Information architecture",101            "Interaction state coverage — loading, empty, error, success, partial, for every surface",102            "User journey & emotional arc",103            "AI slop risk — generic, template-looking output",104            "Design system alignment — reuse existing patterns, do not invent a second way",105            "Responsive & accessibility",106            "Unresolved design decisions",107        ],108    },109    "eng": {110        "who": "a staff engineer who will maintain this after the author leaves",111        "source": "gstack /plan-eng-review — scope challenge, then 4 sections",112        "passes": [113            "Scope challenge — what existing code already solves each sub-problem",114            "Scope challenge — the minimum change that achieves the goal; what can be deferred",115            "Complexity gate — 8+ files or 2+ new classes/services MUST become a decision",116            "Completeness — tests, edge cases and error paths are cheap; do not ship shortcuts",117            "Architecture review",118            "Code quality review",119            "Test review — every changed behaviour needs a regression test",120            "Performance review",121        ],122    },123    "devex": {124        "who": "the next developer to touch this, on their first day",125        "source": "gstack /plan-devex-review, 8 passes",126        "passes": [127            "Getting started — zero friction to first success",128            "API / CLI / SDK design — usable and useful",129            "Error messages & debugging — fight uncertainty",130            "Documentation & learning — findable, learn by doing",131            "Upgrade & migration path — credible",132            "Developer environment & tooling",133            "Community & ecosystem",134            "DX measurement & feedback loops",135        ],136    },137}138139SCOPE_POSTURE = {140    "expansion": "SCOPE EXPANSION: offer additions one at a time, each as its own decision. Nothing is added without approval.",141    "selective": "SELECTIVE EXPANSION: offer cherry-picks one at a time, each as its own decision.",142    "hold": "HOLD SCOPE: review the plan as written. Propose no additions.",143    "reduction": "SCOPE REDUCTION: offer cuts one at a time, each as its own decision. Aim for the minimum valuable scope.",144}145146LANGUAGE_NAMES = {147    "it": "Italian", "en": "English", "zh": "Chinese", "de": "German", "es": "Spanish",148    "fr": "French", "ja": "Japanese", "ko": "Korean", "nl": "Dutch", "pt": "Portuguese",149    "ru": "Russian",150}151152SYSTEM = """You are {who}. You are running {source}.153154{posture}155156{language}157158Review the WHOLE plan through EACH of these passes, in order — every pass,159none skipped:160{passes}161162Return ONLY a JSON object — no prose around it, no code fence:163164{{165  "summary": "<one paragraph: your verdict>",166  "decisions": [167    {{"question": "<one choice the human must make, stated so it can be answered>",168      "header": "<a label of at most 12 characters>",169      "options": [{{"label": "<the choice, a few words>", "description": "<its consequence>"}}],170      "recommended": "<the label of the option you recommend, copied exactly>",171      "recommendation": "<which option and why, in one sentence>",172      "pass": "<which pass raised it>",173      "refines": "<the <already-decided> question this rewords or narrows, verbatim — empty when its topic is new>"}}174  ],175  "findings": [176    {{"severity": "blocking"|"significant"|"minor",177      "title": "<short, specific claim>",178      "detail": "<why it is wrong and what it costs>",179      "component": "<area>", "files": ["<path>"], "pass": "<which pass>"}}180  ],181  "tasks": [182    {{"priority": "P1"|"P2"|"P3", "effort_human": "<~2d>", "effort_cc": "<~2h>",183      "component": "<area>", "title": "<the change to make in the repository>",184      "source_finding": "<finding title>", "files": ["<path>"]}}185  ],186  "not_in_scope": ["<work considered and excluded — and why>"],187  "already_exists": ["<existing code the plan should reuse instead of rebuilding>"]188}}189190Rules:191- A DECISION is a real fork with a consequence either way — something reasonable192  people would choose differently. Not "should we handle errors" (yes).193- List EVERY decision this plan needs from the human, in this one pass, ordered194  by what blocks what — not a few now and the rest next run. The human answers195  them together; a decision you hold back costs a whole extra review round.196- A topic in <already-decided> is SETTLED, however you would reword or narrow197  it. Detail its answer leaves open — exact copy, an encoding, a size, a name —198  is the implementer's call, not the human's. If you still want to flag such a199  detail, return it as a decision with `refines` set to the settled question:200  it reaches the implementer as a note and is never asked.201- The plan decides WHAT to build and WHY, at the level a coding agent's brief202  needs. HOW to build it — exact copy, which image files or URLs, encodings,203  animation technique, exact sizes — is the implementer's call: never a204  decision and never a blocking finding. At most a minor finding.205- A FINDING is an objection. If the plan is sound in a pass, say nothing for it;206  an invented objection costs more than a missing one.207- List EVERY finding, in this one pass — every gap, contradiction, factual208  error and risk you can find, not the most important few. The plan is revised209  and reviewed again: a finding held back surfaces on the next run and costs a210  whole review round.211- `blocking` = must not reach implementation as written. Never use it for taste,212  or for detail the implementer decides.213- Every task traces to a finding by title. A task with no finding behind it is214  scope the reviewer introduced.215- A TASK is work the implementer does in the repository, written as that work:216  "Add a <noscript> note to index.html", not "Specify a no-JavaScript fallback".217  Never a task on the plan itself — "Specify…", "Decide…", "Document in the218  plan…", "Acknowledge…": what the plan is missing is a finding, and a choice219  only the human can make is a decision.220- FILES AND "ALREADY EXISTS" COME FROM THE CODEBASE MAP. When a <codebase> map221  is given, every path in `files` and every entry in `already_exists` must222  appear in it — the map was read from the repository; you were not. When no223  map is given, leave every `files` empty and put in `already_exists` only224  what the plan itself says exists. An invented path is worse than none.225- Use the map's Conventions: a plan that breaks one is a finding."""226227228def normalize_rows(raw: Any) -> list:229    """A list of dicts, whatever arity the board delivered.230231    One wired edge gives a bare list; several give a list of lists. A board that232    works with one reviewer must not break the moment a second is wired in.233    """234    if not isinstance(raw, list):235        return []236    out = []237    for item in raw:238        if isinstance(item, list):239            out.extend(x for x in item if isinstance(x, dict))240        elif isinstance(item, dict):241            out.append(item)242    return out243244245# Kept for the name the first version exported; tests and callers may use it.246normalize_prior = normalize_rows247248249def question_key(text: Any) -> str:250    return " ".join(str(text or "").lower().split())251252253def answered_rows(decisions: list) -> list:254    """Decisions rows that actually carry an answer."""255    return [d for d in decisions if str(d.get("answer") or "").strip()]256257258HEADER_MAX = 12259OPTION_IDS = "abcdefgh"260261262def _options(raw: Any) -> list:263    """Options as requestUserDecision wants them: `[{id, label, description}]`.264265    A model sometimes returns bare strings instead of objects. Both shapes are266    accepted — rejecting one would drop a real decision over formatting.267    """268    if not isinstance(raw, list):269        return []270    out = []271    for item in raw:272        if isinstance(item, dict):273            label = str(item.get("label") or "").strip()274            description = str(item.get("description") or "").strip()275        else:276            label, description = str(item or "").strip(), ""277        if not label:278            continue279        option = {"id": OPTION_IDS[len(out)], "label": label}280        if description:281            option["description"] = description282        out.append(option)283        if len(out) == len(OPTION_IDS):284            break285    return out286287288def refines(proposed: dict) -> str:289    """The settled question a proposed decision rewords or narrows, or ""."""290    return str(proposed.get("refines") or "").strip()291292293def implementer_notes(proposed: list, decisions: list | None = None, every: bool = False) -> list:294    """Proposed decisions that go to the implementer as minor findings, not questions.295296    By default, only refinements of a settled question — the ones the model297    labels through `refines`, since the exact-text filter in open_decisions()298    cannot see a question that comes back reworded or one level narrower. With299    `every`, all of them (route_decisions: a rerun with nothing blocking). A300    question already answered verbatim is never a note: it is settled.301    """302    done = {question_key(d.get("question")) for d in answered_rows(decisions or [])}303    out = []304    for d in proposed:305        if not isinstance(d, dict) or not (every or refines(d)):306            continue307        q = str(d.get("question") or "").strip()308        if not q or question_key(q) in done:309            continue310        recommendation = str(d.get("recommendation") or "").strip()311        origin = (f"Follows from the settled answer to \"{refines(d)}\"." if refines(d)312                  else "Raised on a rerun with nothing blocking.")313        out.append({314            "severity": "minor",315            "title": f"Left to the implementer: {q}",316            "detail": " ".join(part for part in (317                origin,318                f"Suggested: {recommendation}" if recommendation else "",319            ) if part),320            "component": "plan",321            "files": [],322            "pass": str(d.get("pass") or "").strip(),323        })324    return out325326327def asked_here(row: dict, lens: str) -> bool:328    """True when this review asked the row's question.329330    Rows carry `lane`: the label of the review that asked — "Review · Spec", or331    "Spec" in older runs. Answers from EARLIER reviews also arrive here: every332    review reads the same "Decisions" list.333    """334    lane = str(row.get("lane") or "").strip().lower()335    return re.sub(r"^review\s*[·:\-]?\s*", "", lane).strip() == lens336337338def route_decisions(proposed: list, decisions: list, blocking: int, lens: str = "eng") -> tuple[list, list]:339    """(questions for the human, notes for the implementer).340341    A first run asks every real fork. A RERUN — one whose OWN questions have been342    answered (asked_here) — asks only while something still blocks. Answers343    handed down from earlier reviews do not make a run a rerun: on 2026-09-28344    that mistake silenced the Design review's first run, whose only real345    question (the timeline on mobile) became a note. The 2026-09-28 spec review ran nine346    times with every answer reaching it: each answer was written into the design347    doc as new, more specific text, and the next run questioned that text ("the348    'simple geometric' SVG" right after the user chose "Simple geometric"). With349    nothing blocking, those follow-ups are detail the implementer settles — and350    a review that asks nothing is how the agent's loop ends.351    """352    own = [r for r in answered_rows(decisions) if asked_here(r, lens)]353    if own and blocking == 0:354        return [], implementer_notes(proposed, decisions, every=True)355    return open_decisions(proposed, decisions, lens), implementer_notes(proposed, decisions)356357358def recommended_index(options: list, proposed: dict) -> int | None:359    """Which option the review recommends: the one its `recommended` names,360    else the one its `recommendation` sentence opens with.361362    Models name the pick loosely — shortened, extended, differently cased —363    so a label that starts the other counts; the longest wins.364    """365    labels = [question_key(o.get("label")) for o in options]366    named = question_key(proposed.get("recommended"))367    if named:368        if named in labels:369            return labels.index(named)370        hits = [i for i, lab in enumerate(labels) if lab and (named.startswith(lab) or lab.startswith(named))]371        if hits:372            return max(hits, key=lambda i: len(labels[i]))373    text = question_key(proposed.get("recommendation"))374    hits = [i for i, lab in enumerate(labels) if lab and text.startswith(lab)]375    return max(hits, key=lambda i: len(labels[i])) if hits else None376377378def open_decisions(proposed: list, decisions: list, lens: str = "eng") -> list:379    """The unanswered decisions, as ready-made `requestUserDecision` questions.380381    The prompt already tells the model what was decided, so it should not382    re-propose those. This is the second layer: an answered question that comes383    back anyway must not be asked twice — the only thing worse than an384    unanswered decision is a user answering the same one on every rerun.385    """386    done = {question_key(d.get("question")) for d in answered_rows(decisions)}387    source = LENSES[lens]["source"] if lens in LENSES else "gstack review"388    out = []389    for d in proposed:390        if not isinstance(d, dict):391            continue392        q = str(d.get("question") or "").strip()393        if not q or question_key(q) in done or refines(d):394            continue395        header = str(d.get("header") or d.get("pass") or lens).strip()396        recommendation = str(d.get("recommendation") or "").strip()397        pass_name = str(d.get("pass") or "").strip()398        options = _options(d.get("options"))399        pick = recommended_index(options, d)400        if pick is not None:401            options[pick]["recommended"] = True402        question = {403            # Unique within one call, which is all the tool needs. The agent404            # maps an answer back through `question`, not `key`.405            "key": f"{lens}-{len(out) + 1}",406            "question": q,407            "header": header[:HEADER_MAX],408            "context": f"Raised by {source}" + (f", {pass_name}" if pass_name else ""),409            "options": options,410        }411        if recommendation:412            question["recommendation"] = recommendation413        out.append(question)414    return out415416417def build_user_message(plan: str, lens: str, prior: list, decisions: list | None = None,418                       codebase: str = "") -> str:419    parts = []420421    if codebase and codebase.strip():422        # First, so every later block — decisions, prior findings, the plan —423        # is read against what the repository actually contains.424        parts.append(f"<codebase>\n{codebase.strip()}\n</codebase>")425426    answered = answered_rows(decisions or [])427    if answered:428        lines = [f"- Q: {d.get('question', '')}\n  A: {d.get('answer', '')}" for d in answered]429        parts.append(430            "<already-decided>\n" + "\n".join(lines) + "\n</already-decided>\n"431            "These are SETTLED. Do not ask them again. Apply every answer to your "432            "review — findings and tasks must reflect what was chosen."433        )434435    if prior:436        lines = [437            f"- [{f.get('severity', '?')}] {f.get('title', '')} ({f.get('lens', 'earlier review')})"438            for f in prior if isinstance(f, dict)439        ]440        if lines:441            parts.append(442                "<already-raised>\n" + "\n".join(lines) + "\n</already-raised>\n"443                "Do not re-raise these. Disagree with one explicitly if you think it is wrong."444            )445446    parts.append(f"<plan>\n{plan}\n</plan>")447    return "\n\n".join(parts)448449450def coerce(raw: str) -> dict:451    """The model's JSON, or a failure marked `unreadable` rather than a crash.452453    review() asks again on `unreadable` and fails the run if the second reply454    is unreadable too — a parse failure must never read as a clean pass.455    """456    text = raw.strip()457    if text.startswith("```"):458        text = text.split("\n", 1)[-1]459        if text.rstrip().endswith("```"):460            text = text.rstrip()[:-3]461    try:462        parsed = json.loads(text)463        if isinstance(parsed, dict):464            return parsed465    except json.JSONDecodeError:466        pass467    return {468        "unreadable": True,469        "summary": "The reviewer did not return readable JSON.",470        "decisions": [],471        "findings": [{472            "severity": "minor",473            "title": "Review output could not be parsed",474            "detail": (text[:800] + "…") if len(text) > 800 else text,475            "component": "plan-review", "files": [], "pass": "",476        }],477        "tasks": [],478    }479480481def language_line(language: str) -> str:482    """The output-language directive. `auto` defers to the plan.483484    Decisions are shown to the user VERBATIM through requestUserDecision, so a485    wrong language here is not cosmetic — it is a question the user may not be486    able to answer. Unknown codes fall back to auto rather than being pasted487    into the prompt: the value can arrive from a wired upstream output.488    """489    name = LANGUAGE_NAMES.get((language or "").strip().lower())490    if not name:491        return "Write every question, option, finding, task and summary in the language the plan is written in."492    return (f"OUTPUT LANGUAGE: {name}. Write every question, option label, option description, finding, "493            f"task and summary in {name}, whatever language the plan is in. Keep file paths, code "494            f"identifiers and proper nouns as they are.")495496497def system_prompt(lens: str, scope: str, language: str = "auto") -> str:498    spec = LENSES[lens]499    return SYSTEM.format(500        who=spec["who"],501        source=spec["source"],502        posture=SCOPE_POSTURE[scope],503        language=language_line(language),504        passes="\n".join(f"{i}. {p}" for i, p in enumerate(spec["passes"], start=1)),505    )506507508async def review(plan, lens, scope, language, prior, decisions, model_id, codebase="") -> dict:509    messages = [510        {"role": "system", "content": system_prompt(lens, scope, language)},511        {"role": "user", "content": build_user_message(plan, lens, prior, decisions, codebase)},512    ]513    # An unreadable reply is NOT a review: coerced, it has no decisions and514    # nothing blocking, which the agent cannot tell from a clean pass. On515    # 2026-09-28 a spec review failed to parse and the agent moved on as if the516    # design doc had passed. Ask once more; then fail the run so it is rerun,517    # naming finish_reason and length so the cause is visible next time.518    for attempt in (1, 2):519        response = await Gais.llm.chat_async(messages, model_id=model_id, temperature=0, thinking=False)520        text = response.text or ""521        result = coerce(text)522        if not result.get("unreadable"):523            return result524        finish = (getattr(response, "data", None) or {}).get("finish_reason")525        print(f"[plan-review] unreadable reply, attempt {attempt}: finish_reason={finish} "526              f"chars={len(text)} tail={text[-300:]!r}", file=sys.stderr)527    raise ValueError(528        f"The {lens} review returned no readable JSON twice (last finish_reason={finish}, "529        f"{len(text)} chars). Run this review again."530    )531532533def text_list(raw: Any) -> list:534    return [str(x).strip() for x in raw if str(x).strip()] if isinstance(raw, list) else []535536537def main() -> None:538    try:539        envelope = json.loads(sys.stdin.read() or "{}")540        inputs: dict[str, Any] = envelope.get("inputs", {}) if isinstance(envelope, dict) else {}541542        plan = inputs.get("plan")543        lens = (inputs.get("lens") or "eng").strip().lower()544        scope = (inputs.get("scope") or "hold").strip().lower()545        language = (inputs.get("language") or "auto").strip().lower()546        codebase = inputs.get("codebase") if isinstance(inputs.get("codebase"), str) else ""547        model_id = inputs.get("llmModelId")548        prior = normalize_rows(inputs.get("priorFindings"))549        decisions = normalize_rows(inputs.get("decisions"))550551        if not plan:552            raise ValueError("Required input 'plan' not provided")553        if lens not in LENSES:554            raise ValueError(f"Unknown lens {lens!r}. Expected one of: {', '.join(sorted(LENSES))}")555        if scope not in SCOPE_POSTURE:556            raise ValueError(f"Unknown scope {scope!r}. Expected one of: {', '.join(sorted(SCOPE_POSTURE))}")557        if not model_id:558            raise ValueError("Required input 'llmModelId' not provided")559560        print(f"[plan-review] lens={lens} scope={scope} model={model_id} plan_chars={len(plan)} "561              f"prior={len(prior)} answered={len(answered_rows(decisions))}", file=sys.stderr)562563        result = asyncio.run(review(plan, lens, scope, language, prior, decisions, model_id, codebase))564565        findings = [f for f in result.get("findings") or [] if isinstance(f, dict)]566        tasks = [t for t in result.get("tasks") or [] if isinstance(t, dict)]567        blocking = sum(1 for f in findings if f.get("severity") == "blocking")568        pending, notes = route_decisions(result.get("decisions") or [], decisions, blocking, lens)569        findings += notes570571        # Stamp the lens: the next review's already-raised block and the572        # aggregate's ordering both need to know who said what.573        for f in findings:574            f["lens"] = lens575        for i, t in enumerate(tasks, start=1):576            t["lens"] = lens577            t.setdefault("id", f"{lens.upper()}-{i}")578579        print(f"[plan-review] done open_decisions={len(pending)} "580              f"findings={len(findings)} blocking={blocking} tasks={len(tasks)}", file=sys.stderr)581582        json.dump({583            "decisions": pending,584            "findings": findings,585            "tasks": tasks,586            "notInScope": text_list(result.get("not_in_scope")),587            "alreadyExists": text_list(result.get("already_exists")),588            "summary": result.get("summary") or "",589            "blocking": blocking,590        }, sys.stdout)591    except Exception as e:592        print(json.dumps({"error": str(e), "errorType": type(e).__name__,593                          "traceback": traceback.format_exc()}), file=sys.stderr)594        sys.exit(1)595596597if __name__ == "__main__":598    main()599