From ae51b795cc55082b33248631ba490ec09512b733 Mon Sep 17 00:00:00 2001
From: homen <hshum2018@gmail.com>
Date: Thu, 15 Oct 2026 09:34:58 -0700
Subject: [PATCH] fix: reject invalid iteration budgets before judging

---
 .tours/02-agent-execution.tour |   6 +-
 docs/START_HERE.md             |  20 +++--
 iterate.mjs                    |   7 +-
 package.json                   |   3 +-
 test/iteration-budget.test.mjs | 139 +++++++++++++++++++++++++++++++++
 5 files changed, 163 insertions(+), 12 deletions(-)
 create mode 100644 test/iteration-budget.test.mjs

diff --git a/.tours/02-agent-execution.tour b/.tours/02-agent-execution.tour
index 2c0f105..0ec6d84 100644
--- a/.tours/02-agent-execution.tour
+++ b/.tours/02-agent-execution.tour
@@ -61,14 +61,14 @@
     },
     {
       "file": "iterate.mjs",
-      "line": 62,
+      "line": 67,
       "pattern": "for \\(let r = 1; r <= rounds",
       "title": "9 — The loop: render, judge, revise",
-      "description": "One round per iteration against a pass mark, default 28/40. Read the header comment above: a gate that runs once, after the work, is a grade. A gate that runs every round, before the work is called done, is a process. One thing the step order here hides: this is not the judge you just read. `iterate.mjs:72` (`judge-rubric.mjs`) spawns the other, gate-aware judge, and nothing in this repository spawns `iterate.mjs` at all — `npm run iterate` is its only caller."
+      "description": "One round per iteration against a pass mark, default 28/40. Read the header comment above: a gate that runs once, after the work, is a grade. A gate that runs every round, before the work is called done, is a process. One thing the step order here hides: this is not the judge you just read. `iterate.mjs:77` (`judge-rubric.mjs`) spawns the other, gate-aware judge. `npm run iterate` is the application entry; offline iteration-budget tests also spawn it against a synthetic child judge."
     },
     {
       "file": "iterate.mjs",
-      "line": 99,
+      "line": 104,
       "pattern": "writeFileSync\\(`\\$\\{base\\}\\.next-cut\\.md`",
       "title": "10 — It writes a brief and refuses to apply it",
       "description": "Below the gate, the critic's revision brief is written to `<video>.next-cut.md` and the process exits 1. It deliberately does not edit your storyboard: a loop that applies its own critic's notes converges on what the critic likes. The human is the steering mechanism, on purpose."
diff --git a/docs/START_HERE.md b/docs/START_HERE.md
index 031eef0..5248a8d 100644
--- a/docs/START_HERE.md
+++ b/docs/START_HERE.md
@@ -203,10 +203,10 @@ emits something that passes for evidence.** The same reasoning added the
 ## Step 4 — Agent orchestration: not on this path, and here is where it is
 
 **File:** `iterate.mjs`
-**Symbol:** the top-level round loop — `iterate.mjs:62` (`for (let r = 1; r <= rounds`)
-**Called by:** `npm run iterate` — `package.json:45` (`"iterate": "node iterate.mjs"`) — and
-nothing else. No script and no other file in this repository spawns it.
-**Calls next:** `iterate.mjs:72` (`judge-rubric.mjs`) → Google Gemini
+**Symbol:** the top-level round loop — `iterate.mjs:67` (`for (let r = 1; r <= rounds`)
+**Called by:** `npm run iterate` — `package.json:45` (`"iterate": "node iterate.mjs"`).
+The offline iteration-budget tests also spawn it against a synthetic child judge.
+**Calls next:** `iterate.mjs:77` (`judge-rubric.mjs`) → Google Gemini
 
 **Why this exists — and why the capture/render path has no agent at all**
 The stage the HUMAN-READY template calls "agent orchestration" **does not exist in
@@ -224,7 +224,7 @@ below it, so the loop can read pass/fail from the exit code. `npm run clip` does
 near-copies that write the same `.judge.json` / `.judge.md` pair; if you are reading
 a scorecard and cannot tell which produced it, that is why.
 
-**Core code** — the round's verdict, `iterate.mjs:92` (`if (passed) {`) onward:
+**Core code** — the round's verdict, `iterate.mjs:97` (`if (passed) {`) onward:
 ```js
   if (passed) {
     console.log(`\n[iterate] round ${r}: ${total}/40 >= ${gate} — shippable. History in ${LOG}`);
@@ -242,11 +242,17 @@ and, on the last round, the brief it refuses to apply:
 
 **Input** — a rendered MP4, an audience description (`--for`), a pass mark
 (`--gate`, default 28/40), and `GEMINI_API_KEY` in the environment.
+`--rounds` must be a positive safe integer; omitting it defaults to one round.
+Invalid, empty or missing supplied values now exit 1 before rendering or judging.
+Previously, zero/negative/nonnumeric budgets could report success without a review,
+and a fractional budget could report success after a failed review. The no-key
+`npm run test:iteration-budget` checks admission, outcomes and retained history
+with a synthetic judge; it does not measure video quality or provider behavior.
 **Output** — `<video>.judge.json`, `<video>.judge.md`, a per-round history appended to
 `<video>.rounds.md`, and on the final failing round `<video>.next-cut.md` —
-`iterate.mjs:99` (`next-cut.md`).
+`iterate.mjs:104` (`next-cut.md`).
 **Failure behavior** — no key: `judge-rubric.mjs:46` (`set GEMINI_API_KEY or GOOGLE_GENERATIVE_AI_API_KEY`)
-throws. Below the gate: writes the brief and exits 1 — `iterate.mjs:111` (`revision brief written to`).
+throws. Below the gate: writes the brief and exits 1 — `iterate.mjs:116` (`revision brief written to`).
 **It deliberately does not edit your storyboard** — a loop that applies its own
 critic's notes converges on what the critic likes. The human is the steering
 mechanism, on purpose.
diff --git a/iterate.mjs b/iterate.mjs
index 63813de..327f03f 100644
--- a/iterate.mjs
+++ b/iterate.mjs
@@ -37,7 +37,7 @@ const comp = flag("comp");
 const out = flag("out", "out/clip.mp4");
 const audience = flag("for", "a smart newcomer who has never seen this product and does not know the domain jargon");
 const gate = flag("gate", "28");           // 28/40 — see README for where this number came from
-const rounds = Number(flag("rounds", "1"));
+const rounds = Number(flag("rounds", has("rounds") ? NaN : "1"));
 const entry = flag("entry", "src/index.js");
 
 if (!comp) {
@@ -45,6 +45,11 @@ if (!comp) {
   process.exit(1);
 }
 
+if (!Number.isSafeInteger(rounds) || rounds < 1) {
+  console.error("[iterate] --rounds must be a positive safe integer.");
+  process.exit(1);
+}
+
 // Why there is no `shell` here, and why the render below is not spawned in this
 // file: `npx` is a .cmd shim Node cannot exec without a shell, and a shell
 // re-splits every argument on whitespace -- which once truncated the audience
diff --git a/package.json b/package.json
index 91b233c..f8db7ff 100644
--- a/package.json
+++ b/package.json
@@ -21,7 +21,7 @@
     "streamlit"
   ],
   "scripts": {
-    "check": "node check.mjs && npm run test:evidence && npm run test:judge-model && npm run proof",
+    "check": "node check.mjs && npm run test:evidence && npm run test:judge-model && npm run test:iteration-budget && npm run proof",
     "probe:opening": "node probe-opening-frame.mjs",
     "probe:maxpath": "node probe-max-path.mjs",
     "audit:ui": "node audit-ui.mjs",
@@ -50,6 +50,7 @@
     "captions": "node apply-captions.mjs",
     "test:evidence": "node --test test/featureclip-evidence.test.mjs",
     "test:judge-model": "node --test test/judge-model-request.test.mjs",
+    "test:iteration-budget": "node --test test/iteration-budget.test.mjs",
     "evidence:check": "node scripts/featureclip-evidence.mjs check",
     "evidence:project": "node scripts/featureclip-evidence.mjs project --write",
     "proof": "node scripts/featureclip-evidence.mjs proof"
diff --git a/test/iteration-budget.test.mjs b/test/iteration-budget.test.mjs
new file mode 100644
index 0000000..b5fab3c
--- /dev/null
+++ b/test/iteration-budget.test.mjs
@@ -0,0 +1,139 @@
+import test from "node:test";
+import assert from "node:assert/strict";
+import { execFile } from "node:child_process";
+import { promisify } from "node:util";
+import { mkdtempSync, readFileSync, writeFileSync, existsSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { fileURLToPath } from "node:url";
+
+const exec = promisify(execFile);
+const iterator = fileURLToPath(new URL("../iterate.mjs", import.meta.url));
+const audience = "a newcomer reviewing the release candidate";
+// Fixed child outcomes isolate the real iterator's budget, history and exit contract.
+const judge = `
+import { appendFileSync, readFileSync, writeFileSync } from "node:fs";
+const { statuses, missingVerdict } = JSON.parse(readFileSync("scenario.json", "utf8"));
+appendFileSync("calls.jsonl", JSON.stringify(process.argv.slice(2)) + "\\n");
+const count = readFileSync("calls.jsonl", "utf8").trim().split("\\n").length;
+const status = statuses[Math.min(count - 1, statuses.length - 1)];
+const base = process.argv[2].replace(/\\.(mp4|webm|mov)$/i, "");
+if (!missingVerdict) writeFileSync(base + ".judge.json", JSON.stringify({
+  scores: { craft: { score: status === 0 ? 32 : 12 } },
+  verdict: status === 0 ? "pass" : "revise",
+  weakest: "premise", weakest_comprehension: "context",
+  next_cut: [{ dimension: "context", where: "opening", change: "Show the user goal" }]
+}));
+process.exit(status);
+`;
+
+function fixture(statuses = [0], missingVerdict = false) {
+  const cwd = mkdtempSync(join(tmpdir(), "fcs-iteration-budget-"));
+  writeFileSync(join(cwd, "judge-rubric.mjs"), judge);
+  writeFileSync(join(cwd, "scenario.json"), JSON.stringify({ statuses, missingVerdict }));
+  console.log(`[iteration fixture] ${cwd}`);
+  return cwd;
+}
+
+async function run(cwd, budget = []) {
+  const args = [iterator, "--comp", "OfflineRelease", "--no-render",
+    "--out", "release clip.mp4", "--for", audience, "--gate", "28", ...budget];
+  let result;
+  try {
+    result = { status: 0, ...await exec(process.execPath, args, {
+      cwd, timeout: 10_000, maxBuffer: 256 * 1024,
+    }) };
+  } catch (error) {
+    assert.equal(error.killed, false, "iterator must finish within the fixture deadline");
+    assert.equal(typeof error.code, "number", "iterator must exit, not fail to launch");
+    result = { status: error.code, stdout: error.stdout, stderr: error.stderr };
+  }
+  const read = (name) => existsSync(join(cwd, name)) ? readFileSync(join(cwd, name), "utf8") : "";
+  result.calls = read("calls.jsonl").trim().split("\n").filter(Boolean).map(JSON.parse);
+  result.history = read("release clip.rounds.md");
+  result.brief = read("release clip.next-cut.md");
+  // Retain observations with the fixture, including failing/original-source runs.
+  writeFileSync(join(cwd, `observation-${Date.now()}.json`), JSON.stringify(result, null, 2));
+  return result;
+}
+
+function assertCalls(result, count) {
+  assert.equal(result.calls.length, count);
+  assert.equal((result.history.match(/^## Round /gm) ?? []).length, count);
+  for (const args of result.calls)
+    assert.deepEqual(args, ["release clip.mp4", "--for", audience, "--gate", "28"]);
+}
+
+for (const value of ["0", "-1", "not-a-number", "1.5", "Infinity", "-Infinity", "9007199254740992", ""]) {
+  test(`release agent refuses invalid round budget ${JSON.stringify(value)} before judging`, async () => {
+    const result = await run(fixture(), ["--rounds", value]);
+    assert.equal(result.status, 1);
+    assert.match(result.stderr, /--rounds must be a positive safe integer/);
+    assertCalls(result, 0);
+    assert.equal(result.brief, "");
+  });
+}
+
+test("release agent refuses an explicitly missing budget", async () => {
+  const result = await run(fixture(), ["--rounds"]);
+  assert.equal(result.status, 1);
+  assert.match(result.stderr, /--rounds must be a positive safe integer/);
+  assertCalls(result, 0);
+  assert.equal(result.brief, "");
+});
+
+test("a valid review passes once for default, explicit and large safe budgets", async () => {
+  for (const budget of [[], ["--rounds", "1"], ["--rounds", "2"], ["--rounds", "3"], ["--rounds", "9007199254740991"]]) {
+    const result = await run(fixture(), budget);
+    assert.equal(result.status, 0);
+    assertCalls(result, 1);
+    assert.match(result.history, /32\/40 — pass/);
+    assert.equal(result.brief, "");
+  }
+});
+
+test("failed reviews exhaust exactly three rounds and preserve actionable feedback", async () => {
+  const result = await run(fixture([1]), ["--rounds", "3"]);
+  assert.equal(result.status, 1);
+  assertCalls(result, 3);
+  assert.match(result.brief, /round 3/);
+  assert.match(result.brief, /Show the user goal/);
+});
+
+test("the second review can pass inside a two-round budget", async () => {
+  const result = await run(fixture([1, 0]), ["--rounds", "2"]);
+  assert.equal(result.status, 0);
+  assertCalls(result, 2);
+  assert.match(result.history, /12\/40 — revise[\s\S]*32\/40 — pass/);
+  assert.equal(result.brief, "");
+});
+
+test("a judge failure without a scorecard remains a failed default round", async () => {
+  const result = await run(fixture([1], true));
+  assert.equal(result.status, 1);
+  assertCalls(result, 1);
+  assert.match(result.history, /no verdict/);
+  assert.match(result.brief, /round 1/);
+});
+
+test("four simultaneous release jobs keep their histories and outcomes separate", async () => {
+  const outcomes = await Promise.all([0, 1, 0, 1].map(async (status) => {
+    const result = await run(fixture([status]), ["--rounds", "2"]);
+    assert.equal(result.status, status);
+    assertCalls(result, status === 0 ? 1 : 2);
+    return result.status;
+  }));
+  assert.deepEqual(outcomes, [0, 1, 0, 1]);
+});
+
+test("twelve later attempts preserve earlier review history on the same output", async () => {
+  const cwd = fixture();
+  let previous = "";
+  for (let attempt = 1; attempt <= 12; attempt++) {
+    const result = await run(cwd);
+    assert.equal(result.status, 0);
+    assertCalls(result, attempt);
+    assert.ok(result.history.startsWith(previous));
+    previous = result.history;
+  }
+});