docs(from-a-loop): Part 2, a software-factory loop that improves

.changeset/code-review-loop-novel-approval.md

@@ -0,0 +1,5 @@
+---
+"@statelyai/agent": patch
+---
+
+`examples/code-review-loop`: a review that approves a patch but still flags a `novel` issue now goes to a person instead of shipping. `checkingShip` enforces the same rule, so the `canReach` proof covers it. `docs/from-a-loop.md` gains Part 2, which ports the software-factory loop in `examples/code-review-loop/before.ts` to the machine and improves its triage from traces.

examples/code-review-loop/README.md

@@ -13,12 +13,13 @@ generating ─▶ verifying ─────────────────
                                       └── REJECT ─▶ rejected
 ```
 
-- **Environment** (`environment.ts`): the machine above. Shipping is computed, never chosen. The policy can only pick one of the offered RETRY or ESCALATE options. RETRY is refused at the round cap (3), when the reviewer marked an issue `novel`, and when the checks failed outside every visible test. `checkingShip` re-checks every ship; `illegallyShipped` is proven unreachable with `canReach`.
+- **Environment** (`environment.ts`): the machine above. Shipping is computed, never chosen, and an approval that still flags a `novel` issue goes to a person. The policy can only pick one of the offered RETRY or ESCALATE options. RETRY is refused at the round cap (3), when the reviewer marked an issue `novel`, and when the checks failed outside every visible test. `checkingShip` re-checks every ship; `illegallyShipped` is proven unreachable with `canReach`.
 - **Policy** (`policy.ts`, `policies/*.json`): a JSON `AgentWorkflowConfig` run as the decide executor for model `"triage"`. `v0` always retries until the cap. `v1` asks `classifyIssue` whether the review says the task itself is broken and escalates early, and otherwise retries with targeted feedback (the failing tests, or the review).
 - **Traces** (`file-store.ts`): every task run is a thread `<split>/<policyVersion>/<taskId>`, one JSONL file per thread.
 - **Objective** (`bench.ts`), scored from the final state: shipped with hidden tests passing = 1 (a person's approval counts as a ship, so its hidden tests run too), a correctly escalated novel task = 1, an escalated resolvable task = 0.5 (a person's reject or no answer scores as the escalation), shipped with hidden tests failing = 0, minus 0.05 per round.
 - **Tasks** (`tasks.ts`, `fixtures/`): eight tiny modules (`clamp`, `slugify`, `chunk`, `parse-duration`, `round-half`, `truncate`, `capitalize-words`, `merge-config`), each with visible tests (run every round) and hidden tests (run only by the objective). `round-half` and `merge-config` have a spec that contradicts their visible tests, so the right move is to escalate.
 - **Improve** (`improve.ts`): reflect on the train traces, gate the candidate with `verifyAgentConfig` and a probe of the real triage boundary, replay train and held-out, and promote only on a strict objective gain with no violation.
+- **Before** (`before.ts`): the same factory as a hand-rolled `while` loop, the starting point of [Part 2 of Migrating from a hand-rolled loop](../../docs/from-a-loop.md#part-2-a-factory-loop-that-improves).
 
 ## Tests
 

examples/code-review-loop/before.test.ts

@@ -0,0 +1,161 @@
+/**
+ * The hand-rolled loop (before.ts) over plain fakes: no keys, no network.
+ * The last test runs the same outage through the machine.
+ */
+import { describe, expect, test, vi } from "vitest";
+import { createAgentRuntime, runToQuiescence, type AgentTextRequest } from "@statelyai/agent";
+import { runFactory, type FactoryDeps } from "./before.js";
+import { codeReviewLoopMachine, createRunChecks, type Review } from "./environment.js";
+import { createPolicyDecide, loadPolicy, withPolicy } from "./policy.js";
+import { briefOf, getTask } from "./tasks.js";
+import {
+  DRAFT_FLAWS,
+  draft,
+  fakeRunner,
+  mockAuthorAndReviewer,
+  mockReview,
+  reference,
+} from "./test-doubles.js";
+
+const notAsked: FactoryDeps["askHuman"] = async () => {
+  throw new Error("askHuman should not be called");
+};
+
+/** The same scripted author, self-check, reviewer, and checks as the machine's tests, as plain functions. */
+function fakes(overrides: Partial<FactoryDeps> = {}): FactoryDeps {
+  return {
+    writePatch: async ({ task, feedback }) => {
+      const first = draft(task.id) ?? reference(task.id)!;
+      return feedback ? (reference(task.id) ?? first) : first;
+    },
+    selfCheck: async ({ task, patch }) =>
+      patch === reference(task.id)
+        ? { ok: true, concerns: [] }
+        : { ok: false, concerns: ["maybe"] },
+    // The reviewer marks a contradiction between spec and tests `novel`.
+    reviewPatch: async ({ task, patch }) =>
+      patch === reference(task.id)
+        ? { approved: true, issues: [] }
+        : {
+            approved: false,
+            issues: [
+              {
+                severity: /contradict/.test(DRAFT_FLAWS[task.id] ?? "") ? "novel" : "major",
+                note: DRAFT_FLAWS[task.id] ?? "The patch is wrong.",
+              },
+            ],
+          },
+    runChecks: ({ taskId, file, patch }) => fakeRunner({ id: taskId, file }, patch, "visible"),
+    askHuman: notAsked,
+    ...overrides,
+  };
+}
+
+describe("the hand-rolled loop", () => {
+  test("a clean task ships on round 1 without a person", async () => {
+    const result = await runFactory(briefOf(getTask("clamp")), fakes());
+    expect(result).toEqual({
+      outcome: "shipped",
+      rounds: 1,
+      humanApproved: false,
+      patch: reference("clamp"),
+    });
+  });
+
+  test("a failing visible test retries with feedback, then ships on round 2", async () => {
+    const deps = fakes();
+    const writePatch = vi.fn(deps.writePatch);
+    const result = await runFactory(briefOf(getTask("slugify")), { ...deps, writePatch });
+    expect(result).toMatchObject({ outcome: "shipped", rounds: 2 });
+    expect(writePatch.mock.calls[1]![0].feedback).toContain("trims leading and trailing dashes");
+  });
+
+  test("a novel task escalates on round 1, and the person's answer ends the run", async () => {
+    const askHuman = vi.fn<FactoryDeps["askHuman"]>(async () => ({
+      type: "REJECT",
+      reason: "fix the spec first",
+    }));
+    const result = await runFactory(briefOf(getTask("round-half")), fakes({ askHuman }));
+    expect(askHuman).toHaveBeenCalledWith(
+      expect.objectContaining({ round: 1, reason: "the reviewer marked an issue novel" }),
+    );
+    expect(result).toMatchObject({ outcome: "rejected", rounds: 1 });
+  });
+
+  // The loop's weakness: a thrown dependency rejects the whole run. Round 1's
+  // patch, its review, and the feedback live only in local variables, so a
+  // reviewer outage in round 2 loses the task, and no person is asked.
+  test("a reviewer outage rejects the loop's promise and loses the round", async () => {
+    const deps = fakes();
+    let reviews = 0;
+    const reviewPatch: FactoryDeps["reviewPatch"] = async (input) => {
+      if (++reviews === 2) throw new Error("reviewer unavailable");
+      return deps.reviewPatch(input);
+    };
+    const askHuman = vi.fn(notAsked);
+    await expect(
+      runFactory(briefOf(getTask("slugify")), { ...deps, reviewPatch, askHuman }),
+    ).rejects.toThrow("reviewer unavailable");
+    expect(askHuman).not.toHaveBeenCalled();
+  });
+
+  test("the machine routes the same outage to a person, with the patch and the round", async () => {
+    let reviews = 0;
+    const review = mockReview as (request: AgentTextRequest) => Review;
+    const text = mockAuthorAndReviewer({
+      reviewPatch: (request: AgentTextRequest) => {
+        if (++reviews === 2) throw new Error("reviewer unavailable");
+        return review(request);
+      },
+    });
+    const result = await runToQuiescence(
+      createAgentRuntime(codeReviewLoopMachine, {
+        executors: withPolicy(text, createPolicyDecide(loadPolicy("v0"))),
+        actors: { runChecks: createRunChecks(fakeRunner) },
+      }),
+      { input: { taskId: "slugify" } },
+    );
+    expect(result.status).toBe("idle");
+    expect(result.snapshot.value).toBe("humanReview");
+    expect(result.snapshot.context).toMatchObject({ round: 2, patch: reference("slugify") });
+  });
+});
+
+describe("the hand-rolled loop follows the machine's rules", () => {
+  const askedWith: string[] = [];
+  const recordAsk: FactoryDeps["askHuman"] = async ({ reason }) => {
+    askedWith.push(reason);
+    return { type: "REJECT", reason: "no" };
+  };
+
+  test("an approval that flags a novel issue goes to a person, not to ship", async () => {
+    askedWith.length = 0;
+    const result = await runFactory(
+      briefOf(getTask("clamp")),
+      fakes({
+        reviewPatch: async () => ({
+          approved: true,
+          issues: [{ severity: "novel", note: "the spec contradicts the tests" }],
+        }),
+        askHuman: recordAsk,
+      }),
+    );
+    expect(result.outcome).not.toBe("shipped");
+    expect(askedWith).toEqual(["the reviewer marked an issue novel"]);
+  });
+
+  test("a failure outside every visible test is escalated, not retried", async () => {
+    askedWith.length = 0;
+    const writePatch = vi.fn<FactoryDeps["writePatch"]>(async ({ task }) => reference(task.id)!);
+    await runFactory(
+      briefOf(getTask("clamp")),
+      fakes({
+        writePatch,
+        runChecks: async () => ({ passed: false, failingTests: [], output: "timed out" }),
+        askHuman: recordAsk,
+      }),
+    );
+    expect(writePatch).toHaveBeenCalledTimes(1);
+    expect(askedWith).toEqual(["the checks failed outside every visible test"]);
+  });
+});

examples/code-review-loop/before.ts

@@ -0,0 +1,89 @@
+/**
+ * BEFORE: the code-review factory as a hand-rolled loop, the starting point
+ * of Part 2 in docs/from-a-loop.md. environment.ts is the same factory as a
+ * machine.
+ *
+ * Every dependency takes the input and returns the output of the matching
+ * machine request or actor (`writePatch`, `selfCheckPatch`, `reviewPatch`,
+ * `runChecks`), so each line maps to one machine construct. `askHuman`
+ * answers with the machine's APPROVE or REJECT event. before.test.ts runs it
+ * with fakes.
+ */
+import type { CheckResult } from "./checks.js";
+import type { Review, SelfCheck } from "./environment.js";
+import type { TaskBrief } from "./tasks.js";
+
+type PatchInput = { task: TaskBrief; patch: string };
+
+export type FactoryDeps = {
+  writePatch: (input: {
+    task: TaskBrief;
+    round: number;
+    previous: string;
+    feedback: string;
+  }) => Promise<string>;
+  selfCheck: (input: PatchInput) => Promise<SelfCheck>;
+  reviewPatch: (input: PatchInput) => Promise<Review>;
+  runChecks: (input: { taskId: string; file: string; patch: string }) => Promise<CheckResult>;
+  askHuman: (input: {
+    task: TaskBrief;
+    patch: string;
+    round: number;
+    reason: string;
+  }) => Promise<{ type: "APPROVE" } | { type: "REJECT"; reason: string }>;
+};
+
+export type FactoryResult = {
+  outcome: "shipped" | "rejected";
+  rounds: number;
+  humanApproved: boolean;
+  patch: string;
+};
+
+export async function runFactory(task: TaskBrief, deps: FactoryDeps): Promise<FactoryResult> {
+  let patch = task.starter;
+  let feedback = "";
+  let round = 0;
+
+  const escalate = async (reason: string): Promise<FactoryResult> => {
+    const answer = await deps.askHuman({ task, patch, round, reason }); // blocks the process
+    return answer.type === "APPROVE"
+      ? { outcome: "shipped", rounds: round, humanApproved: true, patch }
+      : { outcome: "rejected", rounds: round, humanApproved: false, patch };
+  };
+
+  while (round < 3) {
+    patch = await deps.writePatch({ task, round, previous: patch, feedback });
+    round++;
+    const [self, review] = await Promise.all([
+      deps.selfCheck({ task, patch }),
+      deps.reviewPatch({ task, patch }),
+    ]);
+    const checks = await deps.runChecks({ taskId: task.id, file: task.file, patch });
+
+    // A novel issue goes to a person even when the reviewer approved.
+    if (review.issues.some((issue) => issue.severity === "novel")) {
+      return escalate("the reviewer marked an issue novel");
+    }
+    if (checks.passed && review.approved && self.ok) {
+      return { outcome: "shipped", rounds: round, humanApproved: false, patch };
+    }
+    // A failure outside every visible test (a crash, a timeout) is not retried.
+    if (
+      !checks.passed &&
+      !checks.failingTests.some((name) => task.visibleTestNames.includes(name))
+    ) {
+      return escalate("the checks failed outside every visible test");
+    }
+    feedback = [
+      checks.passed
+        ? ""
+        : `Failing visible tests: ${checks.failingTests.join(", ")}\n${checks.output}`,
+      ...review.issues.map((issue) => `Reviewer [${issue.severity}]: ${issue.note}`),
+      ...self.concerns.map((concern) => `Self-check: ${concern}`),
+    ]
+      .filter(Boolean)
+      .join("\n");
+  }
+  return escalate("round cap");
+}

examples/code-review-loop/environment.test.ts

@@ -18,6 +18,7 @@ import { canReach, lintAgentMachine, type ExplorePathsOptions } from "@statelyai
 import {
   MAX_ROUNDS,
   MAX_TOTAL_ROUNDS,
+  autoShippable,
   codeReviewLoopMachine,
   createRunChecks,
   retryBlocker,
@@ -370,6 +371,18 @@ describe("the RETRY rules", () => {
     expect(triagePayloads(reviewContext(context)).RETRY).toEqual([]);
   });
 
+  test("an approval that still flags a novel issue does not ship on its own", () => {
+    const passing = { passed: true, failingTests: [], output: "" };
+    const selfCheck = { ok: true, concerns: [] };
+    const clean = { approved: true, issues: [] };
+    const novel = {
+      approved: true,
+      issues: [{ severity: "novel" as const, note: "spec contradicts tests" }],
+    };
+    expect(autoShippable(reviewContext({ checks: passing, selfCheck, review: clean }))).toBe(true);
+    expect(autoShippable(reviewContext({ checks: passing, selfCheck, review: novel }))).toBe(false);
+  });
+
   test("refused when the checks failed outside every visible test (a crash)", () => {
     const crash = { passed: false, failingTests: ["visible-tests.mjs"], output: "SyntaxError" };
     expect(deciding({ checks: crash }).can(retry)).toBe(false);

examples/code-review-loop/environment.ts

@@ -8,7 +8,7 @@
  *   generating ─▶ verifying ─────────────────────▶ checking ─▶ triaging
  *    ▲  (writePatch)  ├ selfCheck (selfCheckPatch)  (runChecks)     │
  *    │                └ review    (reviewPatch)                     │
- *    │                                                              ├─ checks ∧ approved ∧ self-check ok ─▶ checkingShip ─▶ shipped
+ *    │                                                              ├─ checks ∧ approved ∧ self-check ok ∧ no novel issue ─▶ checkingShip ─▶ shipped
  *    │                                                              ├─ round cap ──────────────────────────▶ humanReview
  *    │                                                              └─▶ deciding (agent.decide, model "triage": the policy)
  *    ├──────────── RETRY {feedback, focus} ◀──────────────────────────────┤
@@ -159,16 +159,30 @@ export function retryBlocker(facts: Facts): string | undefined {
   if (facts.round >= facts.roundLimit) {
     return `round cap reached (${facts.round} of ${facts.roundLimit})`;
   }
-  if (facts.review?.issues.some((issue) => issue.severity === "novel")) {
+  if (hasNovelIssue(facts)) {
     return "the reviewer marked an issue novel";
   }
   if (unattributedFailure(facts)) return "the checks failed outside every visible test";
   return undefined;
 }
 
-/** True when the loop may ship on its own: checks passed, reviewer approved, self-check ok. */
+/** The reviewer flagged an issue no retry can fix (a contradictory spec, say). */
+function hasNovelIssue(facts: Pick<Facts, "review">): boolean {
+  return !!facts.review?.issues.some((issue) => issue.severity === "novel");
+}
+
+/**
+ * True when the loop may ship on its own: checks passed, reviewer approved,
+ * self-check ok, and no novel issue. An approval that still flags a novel
+ * issue goes to a person.
+ */
 export function autoShippable(facts: Facts): boolean {
-  return !!facts.checks?.passed && !!facts.review?.approved && !!facts.selfCheck?.ok;
+  return (
+    !!facts.checks?.passed &&
+    !!facts.review?.approved &&
+    !!facts.selfCheck?.ok &&
+    !hasNovelIssue(facts)
+  );
 }
 
 // ─── What the policy sees and may choose ───
@@ -602,7 +616,7 @@ export const codeReviewLoopMachine = reviewLoop.createMachine({
       choice: ({ context }) => {
         const checked = context.checks !== null && context.checks.passed;
         const approved = context.review !== null && context.review.approved;
-        return context.humanApproved || (checked && approved)
+        return context.humanApproved || (checked && approved && !hasNovelIssue(context))
           ? { target: "shipped" }
           : { target: "illegallyShipped" };
       },

llms.txt

@@ -37,7 +37,7 @@
 - [Testing and verification](docs/verify.md): Statically lint, simulate, and explore agent machines without any API keys or model calls, from `@statelyai/agent/testing`. Exploration branches on payloads and invoke outputs, and `canReach` returns a shortest witness or a complete unreachability result.
 - [Evals](docs/evals.md): Score agent runs on output, trajectory, and token budget using the seams runAgent already returns, with plain vitest or a vendor harness like Braintrust.
 - [Coming from LangGraph](docs/langgraph-comparison.md): A term-by-term translation of LangGraph concepts, the same agent built both ways, an honest dimension-by-dimension comparison, and the ported examples.
-- [Migrating from a hand-rolled loop](docs/from-a-loop.md): Convert a realistic while-loop tool-calling agent into an agent machine one step at a time, and see what you get for free.
+- [Migrating from a hand-rolled loop](docs/from-a-loop.md): Convert a while-loop agent into an agent machine one step at a time, then improve a software-factory loop's triage from its traces without loosening its rules.
 - [Agent patterns](docs/patterns.md): Common agent patterns (ReAct, reflection, plan-and-execute, RAG, supervisor, and more) as copy-paste XState machines, one runnable file each.
 - [Statechart policy examples](docs/statechart-policy-examples.md): Three examples that put consequential decisions in visible machine states: consensus review, booking compensation, and deadline escalation.
 - [Validate and repair model output with a loop](docs/validate-and-repair.md): Check model output with your own host code, feed the failure back as a repair prompt, and cap the loop in the machine.