Skip to content

[harvest] review-sweep: prompt-carrying finders, file+line dedup, a logged roster throw, read-only agents, interrupted runs #72

Description

@j4th

Five gaps in review-sweep.js and the rules around it, found on three targets' runs after harvest 4. Each is applied in crease-data/crease#281 (open) with a stub scenario in review-sweep-accounting.mjs that was red on the unfixed script.

1. A caller-named finder cannot carry a prompt

pr-review.md says the caller names the finders a diff needs: "a targeted concern such as a schema change, a timing invariant". The slot takes only {key, agentType}:

const finders = (params.finders ?? []).map((f) => ({ key: f.key, agentType: f.agentType }));

So a concern with no defined agent cannot ride in the sweep. On j4th/you-are-hear#89 the statistics finder the caller named "was dispatched directly beside the workflow, because the workflow's finder slot carries no prompt".

Fix. A finder is {key, prompt?, agentType?}. A prompt becomes the find prompt's focus. agentType is passed only when named, so a prompt-only finder runs on the default workflow agent. A finder with neither is dropped coverage, named on the gate line, and never dispatched blind.

const finders = [];
for (const f of params.finders ?? []) {
  if (f && f.key && (f.prompt || f.agentType)) finders.push({ key: f.key, agentType: f.agentType, focus: f.prompt });
  else droppedCoverage.push(`${(f && f.key) || "(unnamed)"} (caller finder with neither prompt nor agentType)`);
}
// in findOnce:
`…${dim.focus ? `Your review focus, from the caller: ${dim.focus}\nReport findings on this focus only.\n\n` : "Apply your standard review discipline. "}…`
// and the opts:
...(dim.agentType ? { agentType: dim.agentType } : {}),

pr-review.md § The orchestrated sweep names the shape. Scenario 25 checks it: the focus reaches the prompt, there is no agentType, the finding is attributed to the finder, and the empty finder is not dispatched and is named as dropped. Crease commit 673bbd0.

2. Paraphrased duplicates each take a verify slot

The dedup key includes the normalized title (file:line:title). Three dimensions paraphrasing one defect make three findings and three verifies. On j4th/you-are-hear#88, "The sweep's verifiers refuted the same finding three times".

Fix. Key on file:line. A finding with no line keeps its title in the key, or every line-less finding in a file would merge. A merged finding carries every distinct title and its detail, keeps the strongest severity, and counts a dimension in alsoFoundBy once, never its own original. The verify prompt lists every report, says they may be paraphrases of one defect or several, and asks a real=true verdict to name which report the evidence demonstrates.

const key = f.line != null ? `${f.file}:${f.line}` : `${f.file}:?:${norm(f.title)}`;
// on a merge:
if (f.dimension !== prior.dimension && !prior.alsoFoundBy.includes(f.dimension)) prior.alsoFoundBy.push(f.dimension);
if (!prior.titles.some((t) => norm(t) === norm(f.title))) { prior.titles.push(f.title); prior.details.push(f.detail); }

pr-review.md invariant (3) now reads "keyed on file and line (and on the normalized title only when a finding names no line) … carrying every distinct title". Scenario 26 checks it: three paraphrases on one line take one verify slot whose prompt lists all three titles, and two different line-less findings stay two. Crease commit 090cde9.

This changes the invariant's wording, so it is a decision for the kit to make, not only a patch. The trade-off is that two different defects on one line now share one verdict. The prompt asks the verifier to say which report its evidence supports.

3. The roster read's catch discards why it threw

).catch(() => null);

A thrown roster read (a budget ceiling, say) degrades exactly like a null read, which is right. But the degrade path keeps no record of the reason (j4th/echosphere#55 deferred it upstream). Fix:

).catch((e) => {
  log(`review-sweep: the roster read threw — ${e && e.message ? e.message : String(e)}`);
  return null;
});

The throwing-roster scenario now also asserts that the reason is logged. Crease commit b3787ef.

4. Nothing tells a finder or verifier not to touch the tree

An echosphere design-workflow agent (j4th/echosphere#54) edited a tracked source file in place and restored it with its old mtime. Cargo judged a stale artifact fresh, and the next gate failed. A refute-by-default verifier is exactly the agent that wants to "just try" a patch.

Fix. Every find and verify prompt carries:

const READ_ONLY = "Never modify the working tree — not even to restore a file afterwards. To probe (run code, try a patch), copy what you need into a scratch directory and give it its own build cache.";

orchestration.md § Fan-out discipline gains Read-only agents stay read-only: a probe runs on a copy with its own build cache. It also says a content check cannot certify the tree afterwards. In echosphere's incident every tracked file matched HEAD by content hash while the build state was stale, and only a forced rebuild (touch) cleared it. So when an agent may have touched the tree anyway, rebuild from clean before the next gate. Scenario 27 asserts the clause is in every find and verify prompt. Crease commits 84b4331, 567d4fe.

5. An interrupted floor or sweep

pr-review.md invariant (2) covers a failed agent (dropped coverage, retried) but not a run that was interrupted or stopped part-way. j4th/echosphere#51 re-ran its interrupted sweep fresh, used none of the stopped run's output, and discarded the reviewer memory it had written. Invariant (2) now says: "An interrupted or stopped floor or sweep is re-run fresh: none of its partial output is used, and reviewer memory it wrote is discarded, never committed". § Reviewer precedent memory points at it. Crease commit 9ff9b22.

Where it lands in the kit

Kit file Items crease commits (on crease-data/crease#281)
.claude/workflows/review-sweep.js 1–4 b3787ef, 673bbd0, 090cde9, 84b4331 (red scenarios first: e6a434d)
.claude/workflows/tests/review-sweep-accounting.mjs scenarios 25–27 and the logged throw; the stub records each call's prompt e6a434d
.claude/rules/pr-review.md the finder shape (1), invariant (3) (2), invariant (2) (5) 673bbd0, 090cde9, 9ff9b22
.claude/rules/orchestration.md § Fan-out discipline read-only agents (4) 84b4331, 567d4fe

All four commits were checked against the harness in sequence: each turns its own scenario green and leaves the next red.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    harvestHarvested from a real cascade runsource:echosphereEvidence from the echosphere run (Linear axis)source:you-are-hearEvidence from the you-are-hear run (GitHub axis)

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions