Keep mixed reviewer replies attached to the correct findings

CHANGELOG.md

@@ -2,6 +2,9 @@
 
 ## Unreleased
 
+- Keep agreed findings settled in mixed replies with explicit dispute IDs, and
+  stop rebuttal evidence before a separate new finding while preserving restatements
+  of the disputed finding.
 - Support a saved `--language` preference for reviewer findings, judge explanations,
   replies and code-fix summaries, including language switches on resumed runs (#124).
 

docs/usage.md

@@ -184,6 +184,11 @@ JSON keys, verdict values and exact protocol/stop markers remain unchanged.
 Code comments, string literals and test names follow the repository's conventions
 and task requirements rather than the response-language preference.
 
+Replies with explicit `DISPUTE: <finding-id>` markers reopen only the named
+findings. Agreements with other findings remain settled. Each complete rebuttal
+ends at the next live dispute or separate new finding; restatements of the disputed
+finding and quoted examples stay in its evidence.
+
 ## Token usage
 
 The CLI and saved-run console show input, output and total tokens for every review,

lib/loop.js

@@ -13,7 +13,7 @@ import { findingPr } from "./review-group.js";
 // effect of agreeing.
 import path from "node:path";
 import { randomUUID } from "node:crypto";
-import { runAgent } from "./agents.js";
+import { runAgent, parseFindings } from "./agents.js";
 import { buildReply, threadFor, replyArgv } from "./reply.js";
 import { findingsIn, gate, settledList } from "./findings.js";
 import { appendEvent, writeArtifact } from "./store.js";
@@ -388,13 +388,14 @@ export async function recordReplyDisputes(dir, agent, thread, { report, verdict
   // claim: one contested item must not spend the whole run's budget.
   let explicitlyDisputed = false;
   const replyLines = (report ?? "").split("\n");
+  const hasDisputeMarkers = replyMarkers(replyLines).some(marker => marker.id !== null);
   for (const f of thread) {
     if (f.status === "open") continue;
     const disputed = disputeLine(replyLines, f) >= 0;
     if (disputed) explicitlyDisputed = true;
-    // A sign-off may name the title or path while agreeing with the verdict.
-    // Only an explicit dispute can contradict that approval.
-    if (disputed || (verdict !== "clean" && mentions(report, f))) {
+    // Explicit IDs distinguish objections from agreements in a mixed reply.
+    // Keep title/path matching only for legacy replies without live markers.
+    if (disputed || (!hasDisputeMarkers && verdict !== "clean" && mentions(report, f))) {
       await appendEvent(dir, {
         t: "finding.turn", id: f.id, agent, who: agent,
         text: excerpt(report, f),
@@ -418,10 +419,11 @@ function same(f, nf) {
 
 function disputeLine(lines, f) {
   if (!f.id) return -1;
-  return disputeMarkers(lines).find(marker => marker.id === f.id)?.line ?? -1;
+  return replyMarkers(lines).find(marker => marker.id === f.id)?.line ?? -1;
 }
 
-function disputeMarkers(lines) {
+// Both new findings and disputes delimit evidence; quoted examples do not.
+function replyMarkers(lines) {
   const markers = [];
   let fence = null;
   for (const [i, line] of lines.entries()) {
@@ -435,8 +437,8 @@ function disputeMarkers(lines) {
       else if (marker[1][0] === fence[0] && marker[1].length >= fence.length && !marker[2].trim()) fence = null;
       continue;
     }
-    const dispute = !fence && line.trim().match(/^DISPUTE: (.+)$/);
-    if (dispute) markers.push({ id: dispute[1], line: i });
+    const protocol = !fence && line.trim().match(/^(DISPUTE|FINDING): (.+)$/);
+    if (protocol) markers.push({ id: protocol[1] === "DISPUTE" ? protocol[2] : null, line: i });
   }
   return markers;
 }
@@ -457,7 +459,13 @@ function mentions(report, f) {
 
 function excerpt(report, f) {
   const lines = (report ?? "").split("\n");
-  const markers = disputeMarkers(lines);
+  const markers = replyMarkers(lines).filter((marker, i, all) => {
+    if (marker.id !== null) return true;
+    // A restatement is deduplicated by replyRound, so its evidence must stay
+    // in the rebuttal. Use the same identity check for both paths.
+    const [finding] = parseFindings(lines.slice(marker.line, all[i + 1]?.line).join("\n"));
+    return !finding || !same(f, finding);
+  });
   const sections = [];
   for (const [i, marker] of markers.entries()) {
     if (marker.id === f.id) {

test/language-replies.test.js

@@ -228,3 +228,87 @@ for (const fence of ['```', '~~~']) {
     }
   });
 }
+
+for (const [language, claim, agreement, objection] of [
+  ['zh-CN', '[P2] 数值错误', '关于 B 的数值错误，我同意已经修复。', '匿名请求仍然能执行受保护操作。'],
+  ['en', '[P2] Numeric overflow', 'I agree that B: Numeric overflow is fixed.', 'Anonymous requests can still perform the protected action.'],
+]) test(`explicit dispute IDs keep mixed agreements settled: ${language}`, async t => {
+  const dir = await mkdtemp(path.join(tmpdir(), 'jury-mixed-reply-'));
+  t.after(() => rm(dir, {recursive: true, force: true}));
+  const report = `DISPUTE: A\n${objection}\n\n${agreement}`;
+  const agent = {name: 'fixture', argv: [process.execPath, '-e', `console.log(${JSON.stringify(report)})`]};
+  // The agreed finding comes first so marker handling cannot depend on thread order.
+  for (const [id, title, status] of [['B', claim, 'accepted'], ['A', '[P2] Missing authorization', 'rejected']]) {
+    await appendEvent(dir, {t: 'finding.raised', id, agent: agent.name, round: 1, claim: title, loc: `${id}.js:1`});
+    await appendEvent(dir, {t: 'finding.resolved', id, verdict: status, reason: 'Checked'});
+  }
+  const [answer] = await replyRound({dir, pool: [agent], cfg: {stopToken: 'NO NEW FINDINGS'},
+    worktree: dir, sha: 'abc', round: 1, language, findings: await findingsIn(dir)});
+  assert.equal(answer.failed, false);
+  assert.equal(answer.clean, false, 'the disputed finding still needs the judge');
+  const events = await readEvents(dir), findings = await findingsIn(dir);
+  assert.deepEqual({
+    turns: events.filter(e => e.t === 'finding.turn').map(e => e.id),
+    pending: outstanding(findings, events).map(f => f.id),
+    agreed: {status: findings.get('B').status, contested: findings.get('B').contested, turns: findings.get('B').turns},
+  }, {turns: ['A'], pending: ['A'], agreed: {status: 'accepted', contested: false, turns: 0}});
+  assert.ok(findings.get('A').rebuttals[0].includes(objection));
+  assert.deepEqual(findings.get('B').rebuttals, []);
+});
+
+test('quoted dispute markers do not disable legacy title matching', async t => {
+  const report = 'Earlier example:\n```text\nDISPUTE: unknown\n```\n我仍然不同意：缺少权限检查的问题依然存在。';
+  const {events, findings} = await reply(t, '[P2] 缺少权限检查', report);
+  assert.deepEqual(events.filter(e => e.t === 'finding.turn').map(e => e.id), ['A']);
+  assert.deepEqual(outstanding(findings, events).map(f => f.id), ['A']);
+});
+
+test('complete rebuttal sections preserve code examples and repeated disputes without other findings', async t => {
+  const first = [
+    'DISPUTE: A', 'The quoted example is part of the reproduction:', '',
+    '```text', 'DISPUTE: B', '```', '',
+    'Important evidence after the example: the protected write succeeds.',
+  ].join('\n');
+  const second = 'DISPUTE: A\nAn additional request confirms the same unauthorized write.';
+  const report = `Introduction.\n\n${first}\n\nDISPUTE: B\nUnrelated dispute evidence.\n\n${second}\n\n`
+    + 'FINDING: A separate defect\nWHERE: other.js:2\nUnrelated new finding evidence.';
+  const {events, findings} = await reply(t, '[P2] Missing authorization validation', report, 'en');
+  const saved = events.find(e => e.t === 'finding.turn' && e.id === 'A').text;
+  assert.equal(saved, `${first}\n\n${second}`, 'keep all sections for the exact ID and respect live boundaries');
+  const prompt = triagePrompt({finding: findings.get('A'), trunk: 'master', stopToken: 'NO NEW FINDINGS'});
+  assert.ok(prompt.includes('Important evidence after the example: the protected write succeeds.'));
+  assert.ok(prompt.includes('An additional request confirms the same unauthorized write.'));
+  assert.ok(!prompt.includes('Unrelated dispute evidence.'));
+  assert.ok(!prompt.includes('Unrelated new finding evidence.'));
+});
+
+for (const [label, claim, loc] of [
+  ['same title and location', 'Missing authorization', 'auth.js:20'],
+  ['same location with a revised title', 'Alternate route permits anonymous writes', 'auth.js:20'],
+  ['normalized title with a different location', 'Missing authorization!', 'alternate.js:30'],
+]) test(`restated disputed findings retain evidence and stop at separate findings: ${label}`, async t => {
+  const dir = await mkdtemp(path.join(tmpdir(), 'jury-restated-reply-'));
+  t.after(() => rm(dir, {recursive: true, force: true}));
+  const evidence = 'New evidence: anonymous POST to the alternate route returns 200 and changes protected data.';
+  const section = `DISPUTE: A\nFINDING: ${claim}\nWHERE: ${loc}\n${evidence}`;
+  const separate = 'FINDING: Numeric overflow\nWHERE: balance.js:8\nUnrelated new finding evidence.';
+  const report = `${section}\n\n${separate}\n\nDISPUTE: B\nUnrelated dispute evidence.`;
+  const agent = {name: 'fixture', argv: [process.execPath, '-e', `console.log(${JSON.stringify(report)})`]};
+  await appendEvent(dir, {t: 'finding.raised', id: 'A', agent: agent.name, round: 1,
+    claim: 'Missing authorization', loc: 'auth.js:20'});
+  await appendEvent(dir, {t: 'finding.resolved', id: 'A', verdict: 'rejected', reason: 'Checked'});
+  const [answer] = await replyRound({dir, pool: [agent], cfg: {stopToken: 'NO NEW FINDINGS'},
+    worktree: dir, sha: 'abc', round: 1, language: 'en', findings: await findingsIn(dir)});
+  assert.equal(answer.failed, false);
+  assert.equal(answer.clean, false);
+  assert.equal(answer.raised, 1, 'only the separate finding is raised');
+  const events = await readEvents(dir), findings = await findingsIn(dir);
+  assert.deepEqual(events.filter(e => e.t === 'finding.raised' && e.id !== 'A')
+    .map(({claim, loc}) => ({claim, loc})), [{claim: 'Numeric overflow', loc: 'balance.js:8'}]);
+  assert.equal(events.find(e => e.t === 'finding.turn' && e.id === 'A').text, section);
+  assert.deepEqual(findings.get('A').rebuttals, [section]);
+  const prompt = triagePrompt({finding: findings.get('A'), trunk: 'master', language: 'en'});
+  assert.ok(prompt.includes(evidence));
+  assert.ok(!prompt.includes('Unrelated new finding evidence.'));
+  assert.ok(!prompt.includes('Unrelated dispute evidence.'));
+});