1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312// The review loop's stopping rule, and what it costs to get it wrong.
//
// Reader -> Editor -> Reader is the loop, and the editor call is the most expensive
// thing in a run: the whole body goes in, and what comes back is every block the editor
// changed (#250). Some issues are unresolvable HERE by design and the Reader is told to
// report them anyway — an undecidable pair of same-worded headings (EDITOR_SYSTEM:
// "leave both headings exactly as they are"), a [page not fully transcribed] marker that
// only re-extraction can settle. A document whose remaining issues are those gets the
// same answer from the editor every round, so running to the cap buys a full re-read and
// another whole-body rewrite per round, to deliver the document already in hand.
//
// So the loop stops when a round changes nothing — but only when the editor ANSWERED. A
// reply that could not be parsed also leaves the body untouched, for the opposite reason,
// and the next round is a real retry. These tests hold both halves, and hold what is
// delivered: the same body, with the same issues written to @unresolved.
import { test } from "node:test";
import assert from "node:assert/strict";
import { mkdtempSync, rmSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { runReview } from "../src/pipeline/review.ts";
import type { PipelineContext } from "../src/pipeline/context.ts";
import type { Paths } from "../src/store/paths.ts";
const BODY = "<h2>Operation</h2><p>Fill the hopper.</p><h2>Operation</h2><p>Press start.</p>";
// The issue this loop is designed not to resolve: the editor is told to leave an
// undecidable heading pair alone, so it hands the document straight back.
const ISSUES = [
{
issue: "two [Heading 2] Operation headings, and the excerpts do not say which case it is",
severity: "medium",
suggested_action: "leave both headings alone unless the pages decide it",
pages: [],
},
];
interface Round {
readers: number;
editors: number;
// What each Reader pass was given, in order. Kept because the question "was the document that
// shipped ever read" cannot be answered from a call count (#264): the loop reads at the top of
// every round, so the LAST of these is the body the exit delivered — on every exit but a
// truncation, where the editor changed the body after it.
readerPrompts: string[];
events: { type: string; data: Record<string, unknown> }[];
result: Awaited<ReturnType<typeof runReview>>;
}
// Run the loop against an editor that answers with `editorReply` every time. The Reader
// answers with the same unfixable issue unless a test says otherwise — which only the
// stopping-reason tests below need, because the two exits that end a run BEFORE the editor
// is called are decided entirely by what the Reader said.
async function loop(
editorReply: (body: string) => string,
maxReviewIterations = 3,
readerReply: () => string = () => JSON.stringify({ issues: ISSUES }),
): Promise<Round> {
const dir = mkdtempSync(join(tmpdir(), "iris-converge-"));
try {
let readers = 0;
let editors = 0;
const readerPrompts: string[] = [];
const events: { type: string; data: Record<string, unknown> }[] = [];
const ctx = {
sessionId: "ses_test",
images: [],
maxReviewIterations,
extractionConcurrency: 4,
recheckSampleSize: 1,
paths: {
agentsDir: join(dir, "agents"),
tmpAgentsDir: () => join(dir, "tmp-agents"),
agentMemory: () => join(dir, "memory", "page.json"),
} as unknown as Paths,
router: {
complete: async (agent: string, _cap: string, messages: { content: string }[]) => {
if (agent === "reader") {
readers++;
readerPrompts.push(messages.map((m) => m.content).join("\n"));
return { text: readerReply() };
}
editors++;
// The document the editor was actually handed, read back out of its own prompt — the
// numbered blocks, markers and all, so a reply about block 2 is a reply about the
// document this round was given rather than about a constant that happens to match.
const prompt = messages.map((m) => m.content).join("\n");
const given = prompt.match(/## Current document \(body content, in numbered blocks\)\n([\s\S]*?)\n\n## Issues to fix/);
assert.ok(given, "the editor prompt no longer carries the body where this test reads it");
return { text: editorReply(given[1]) };
},
},
log: {
event: (type: string, data: Record<string, unknown> = {}) => events.push({ type, data }),
agentCall: () => {},
},
} as unknown as PipelineContext;
const result = await runReview(ctx, { body: BODY, lint: { ok: true, violations: [] } });
return { readers, editors, readerPrompts, events, result };
} finally {
rmSync(dir, { recursive: true, force: true });
}
}
const typed = (round: Round, type: string) => round.events.filter((e) => e.type === type);
test("a round that changes nothing ends the loop", async () => {
// The editor answers, and answers that there is nothing to change. At a cap of 3 the
// old loop spent two more editor calls and two more full re-reads on the same request.
const round = await loop(() => JSON.stringify({ edits: [] }));
assert.equal(round.editors, 1, "one editor call, not three");
assert.equal(round.readers, 1, "and no re-read of a document that did not change");
const converged = typed(round, "review_converged");
assert.equal(converged.length, 1);
assert.deepEqual(converged[0].data, { iteration: 1, issues: 1, rounds_left: 2 });
});
test("an editor that answers with the whole document converges the same way", async () => {
// The contract the editor used to be given, still read (#250): a model that hands back a
// corrected body instead of a list of edits is answering, and a reply identical to its input
// is the same fact about the round as an empty edits list. Pinned because the equivalence is
// the whole reason the old shape is still accepted — a model that reverts to it under load
// must not cost the loop its stopping rule as well.
//
// The reply is the body itself rather than what the prompt showed, and the difference is worth
// naming: the numbered view puts each block on its own line, so a model that retypes what it
// was shown returns a body that differs from the original in whitespace and is a CHANGED round
// by every measure this loop has. That is a real cost of answering the old way and it belongs to
// the model, not to this code — what is pinned here is that answering the old way still works.
const round = await loop(() => JSON.stringify({ html: BODY }));
assert.equal(round.editors, 1, "one editor call, not three");
assert.equal(typed(round, "review_converged").length, 1);
assert.equal(typed(round, "editor_whole_body").length, 1, "and the log says which contract it answered");
});
test("what a converged round delivers is what the cap would have delivered", async () => {
const round = await loop(() => JSON.stringify({ edits: [] }));
assert.equal(round.result.body, BODY, "the document is the one the editor handed back");
assert.equal(round.result.unresolved.length, 1, "the issues it stopped on are reported");
assert.match(round.result.html, /@unresolved/);
assert.match(round.result.html, /two \[Heading 2\] Operation headings/);
assert.equal(round.result.iterationsCompleted, 1, "the round that ran is counted, and no more");
});
test("an unusable reply is a retry, not a decision", async () => {
// The body is unchanged here too, and for the opposite reason: the editor never said
// anything. Stopping on it would turn one bad response into a skipped correction.
const round = await loop(() => "not json at all");
assert.equal(round.editors, 3, "every round the cap allows is still spent");
assert.equal(typed(round, "review_converged").length, 0, "and none of them is a convergence");
assert.equal(typed(round, "editor_no_output").length, 3, "each is recorded as a call that said nothing");
assert.equal(round.result.body, BODY);
});
test("a round that changes something keeps the loop going", async () => {
// The guard has to be about what changed, not about how many issues came back: the
// Reader here keeps reporting, and the editor keeps editing, so the cap is what stops it.
let n = 0;
const round = await loop(() => JSON.stringify({ edits: [{ block: 0, html: `<p>edit ${n++}</p>` }] }));
assert.equal(round.editors, 3, "the cap, not convergence, is what stopped this");
assert.equal(typed(round, "review_converged").length, 0);
assert.equal(round.result.iterationsCompleted, 3);
});
// Which exit the loop left by (#264). The tests above establish that the exits behave
// differently; these pin the word each one records, because from outside the loop two of
// them are the same document — issues open, nothing truncated — and they ask for opposite
// fixes. `cap` is the only one more rounds can help; `converged` means the editor was shown
// the issues and answered "no change", so the remedy is a prompt. A report that cannot tell
// them apart can only guess, which is how #264 came to lead with `max_review_iterations`
// against a mean of 0.886 rounds out of 3. The `truncated` exits are pinned where they are
// driven, in test/review-truncation.test.ts.
const CLEAN_READ = () => JSON.stringify({ issues: [] });
test("the loop records which of its exits ended the round", async () => {
const converged = await loop(() => JSON.stringify({ edits: [] }));
assert.equal(converged.result.stoppedAt, "converged");
let n = 0;
const cap = await loop(() => JSON.stringify({ edits: [{ block: 0, html: `<p>edit ${n++}</p>` }] }));
assert.equal(cap.result.stoppedAt, "cap");
// Both of these end before the editor is ever called, and only the Reader's answer
// decides which: an empty issue list from a read that came back whole is `clean`, and the
// same empty list from a read with a window it could not parse is `unread` — a document
// nothing objected to and a document nothing finished reading.
const clean = await loop(() => assert.fail("a clean read must not reach the editor"), 3, CLEAN_READ);
assert.equal(clean.result.stoppedAt, "clean");
assert.equal(clean.result.iterationsCompleted, 0);
const unread = await loop(() => assert.fail("an unread window must not reach the editor"), 3, () => "not json at all");
assert.equal(unread.result.stoppedAt, "unread");
assert.equal(unread.result.unreviewedWindows, 1);
// And which of the five deliver a list of open issues, which is what makes the tally's exits a
// split of `unresolved_rate` rather than a breakdown standing beside it (#264, and the
// arithmetic is asserted in test/quality.test.ts). These two deliver none — `clean` because
// there was nothing and `unread` because nothing was found — so neither records an
// `iris:unresolved` row, and the other three each do.
assert.deepEqual(clean.result.unresolved, []);
assert.deepEqual(unread.result.unresolved, [], "an empty list because nothing was FOUND, not because none is there");
assert.equal(converged.result.unresolved.length, 1);
assert.equal(cap.result.unresolved.length, 1);
});
test("the list a converged or capped round ships was read on the bytes that ship", async () => {
// The distinction the split in the quality tally turns on (#264). The loop re-reads at the TOP
// of each round, and both of these exits are taken before the next editor call — so on them the
// issues in `@unresolved` are a statement about the delivered document, and a threshold on that
// part of the rate is a threshold on document quality.
//
// `truncated` is the exit where that is not true: the editor changed the body and the round
// that would have re-read it is the one that could not be made. That half is driven in
// test/editor-sections.test.ts, where the corrections that ship are ones no Reader ever saw.
const converged = await loop(() => JSON.stringify({ edits: [] }));
assert.equal(converged.result.body, BODY);
assert.equal(converged.readers, 1, "and the one read it has was of that body");
let n = 0;
const cap = await loop(() => JSON.stringify({ edits: [{ block: 0, html: `<p>edit ${n++}</p>` }] }));
assert.match(cap.result.body, /edit 2/, "the third round's edit is what shipped");
// The claim itself, and not the call count that suggests it: the delivered edit is in the
// prompt of the last Reader pass, so the issues left open were raised against these bytes.
assert.equal(cap.readers, 4);
assert.match(cap.readerPrompts[3], /edit 2/, "the read that found the cap was of the delivered body");
assert.ok(
cap.readerPrompts.slice(0, 3).every((p) => !/edit 2/.test(p)),
"and only that one — the earlier reads were of bodies that did not ship",
);
});
test("the cap and a round that changed nothing are one document with two remedies", async () => {
// Everything else the report can see about these two runs agrees: the same body, the same
// one issue left open, nothing truncated. `stoppedAt` is the only thing that distinguishes
// a budget that ran out from an editor that declined.
const converged = await loop(() => JSON.stringify({ edits: [] }));
let n = 0;
const cap = await loop(() => JSON.stringify({ edits: [{ block: 0, html: `<p>edit ${n++}</p>` }] }));
for (const round of [converged, cap]) {
assert.equal(round.result.unresolved.length, 1);
assert.equal(round.result.editorTruncated, false);
assert.equal(round.result.unreviewedWindows, 0);
}
assert.notEqual(converged.result.stoppedAt, cap.result.stoppedAt);
});
test("a run that spent every round on unusable replies reports the cap, not a convergence", async () => {
// The body never changed, so this run looks converged from the body alone — and it is the
// opposite: three rounds spent, none of them answered. Reporting it as `converged` would
// attribute a broken editor to a prompt that the editor never disagreed with.
const round = await loop(() => "not json at all");
assert.equal(round.result.stoppedAt, "cap");
assert.equal(round.result.iterationsCompleted, 3);
});
// The counts of an empty structure, so the two assertions below can say which of the thirteen the
// round moved and be read as the whole line at the same time.
const NO_STRUCTURE = {
headings: 0, paragraphs: 0, lists: 0, items: 0, terms: 0, definitions: 0,
tables: 0, captions: 0, rows: 0, header_cells: 0, cells: 0, images: 0, links: 0,
};
test("each round says whether it changed the document, and by how much", async () => {
// The sizes and the structure counts are on the same line as `changed`, and they are the whole
// line: nothing else about a round answered whole. Here a round replaced the body with something
// a fifth of its size and both its headings with nothing, and it is delivered — this body holds
// 49 characters of prose, which is under `EDITOR_FLOOR_MIN_TEXT`, and a proportion of 49
// characters is not a measurement (#174). What the floor does above that size is pinned in
// test/editor-round-size.test.ts; what this pins is that the numbers are reported either way.
// Four blocks in, one replaced and three emptied — which is the patch contract's way of saying
// what a whole-body reply of "<p>edited</p>" used to say, and it exercises the deletion path
// while it is here.
const changed = await loop(
() =>
JSON.stringify({
edits: [
{ block: 0, html: "<p>edited</p>" },
{ block: 1, html: "" },
{ block: 2, html: "" },
{ block: 3, html: "" },
],
}),
1,
);
assert.deepEqual(typed(changed, "editor")[0].data, {
iteration: 1,
changed: true,
chars_before: BODY.length,
chars_after: "<p>edited</p>".length,
text_chars_before: "Operation Fill the hopper. Operation Press start.".length,
text_chars_after: "edited".length,
structure_before: { ...NO_STRUCTURE, headings: 2, paragraphs: 2 },
structure_after: { ...NO_STRUCTURE, paragraphs: 1 },
});
// A converged round handed the document back, so every one of these is a measurement of the body
// it was given. Equal numbers do not say the round converged, though — a reply with nothing
// usable in it reports the same ones, and `editor_no_output` is what tells those apart.
const unchanged = await loop(() => JSON.stringify({ edits: [] }), 1);
assert.deepEqual(typed(unchanged, "editor")[0].data, {
iteration: 1,
changed: false,
chars_before: BODY.length,
chars_after: BODY.length,
text_chars_before: "Operation Fill the hopper. Operation Press start.".length,
text_chars_after: "Operation Fill the hopper. Operation Press start.".length,
structure_before: { ...NO_STRUCTURE, headings: 2, paragraphs: 2 },
structure_after: { ...NO_STRUCTURE, headings: 2, paragraphs: 2 },
});
});