fix(proposal): drop only a still-ambiguous edit instead of the whole proposal

README.md

@@ -516,7 +516,10 @@ keeps the merged change rather than guessing.
 
 A malformed answer or gate violation triggers a re-prompt naming the exact breach (at
 most two). If those also fail, backpass **fails loudly** and preserves the latest parseable
-rejected proposal, if one was produced. It never silently truncates. A harness that writes
+rejected proposal, if one was produced. It never silently truncates. The one edit it will
+drop is a change whose anchor text still occurs more than once after the re-prompts
+(an earlier edit repeats it); that edit alone is dropped, named in a warning and in the
+proposal's notes, and the rest of the proposal goes to review. A harness that writes
 past the staging copy into the repo is an error, never an apply.
 
 Not every annotate turn is an answer, and the three cases are kept apart because they call

src/nested.js

@@ -380,7 +380,7 @@ export function owningFile(sessionIds, weights, attribution) {
 
 /**
  * One proposal over every trained file, reviewed in one apply: the root proposal, then
- * each nested file's edits renumbered after it and labeled with their file. A nested
+ * each nested file's edits numbered after it, never reusing an id, and labeled with their file. A nested
  * file's image joins `targetFiles`, so apply refuses it if it changed since, the same
  * freshness contract every non-root target has; `nested` carries each file's own budget,
  * which apply re-checks against the accepted subset. With `sampledSessions` and
@@ -391,11 +391,20 @@ export function mergeNestedProposals(root, passes, { sampledSessions = null, min
   const targetFiles = [...(root.targetFiles || [])];
   const usage = [...(root.usage || [])];
   const notes = [...(root.notes || [])];
+  // Review and apply key decisions by id, so an id is never reused. A root proposal can have
+  // a gap (a dropped ambiguous edit), which makes "one past the count" a live neighbour's id.
+  const taken = new Set(edits.map((edit) => edit.id));
+  const nextId = () => {
+    let serial = edits.length + 1;
+    while (taken.has(`e${serial}`)) serial += 1;
+    taken.add(`e${serial}`);
+    return `e${serial}`;
+  };
   const nested = passes.map((pass) => {
     const { weight, proposal } = pass;
     const ids = [];
     for (const edit of proposal?.edits || []) {
-      const id = `e${edits.length + 1}`;
+      const id = nextId();
       edits.push({ ...edit, id, targetsMemoryFile: false, nestedMemoryFile: weight.path });
       ids.push(id);
     }

src/proposal.js

@@ -211,10 +211,37 @@ function occurrences(haystack, needle) {
   return count;
 }
 
+/** `Error.code` of a `find` that matches more than once, so callers can tell it from a stale one. */
+export const AMBIGUOUS_FIND = "AMBIGUOUS_FIND";
+
+/** 1-based line where each occurrence of `needle` starts (overlapping ones included), capped for display. */
+function occurrenceLines(haystack, needle, cap = 5) {
+  const lines = [];
+  let index = haystack.indexOf(needle);
+  while (index !== -1 && lines.length < cap) {
+    // A tail `find` carries the newline before its first line (see `span` in diff.js).
+    const start = index + (needle.startsWith("\n") ? 1 : 0);
+    lines.push(haystack.slice(0, start).split("\n").length);
+    index = haystack.indexOf(needle, index + 1);
+  }
+  return lines;
+}
+
+function ambiguousFind(text, find, found, label, file) {
+  const lines = occurrenceLines(text, find);
+  const more = found > lines.length ? `, and ${found - lines.length} more` : "";
+  return Object.assign(
+    new Error(
+      `${label}: "find" text appears ${found} times in ${file} (lines ${lines.join(", ")}${more}); must be unique`,
+    ),
+    { code: AMBIGUOUS_FIND },
+  );
+}
+
 function replaceOnce(text, find, replace, label, file) {
   const found = occurrences(text, find);
   if (found === 0) throw new Error(`${label}: "find" text does not appear in ${file}`);
-  if (found > 1) throw new Error(`${label}: "find" text appears ${found} times in ${file}; must be unique`);
+  if (found > 1) throw ambiguousFind(text, find, found, label, file);
   return text.replace(find, () => replace);
 }
 
@@ -380,6 +407,7 @@ export function buildProposal(rawResult, context) {
     scope = null,
     target = { kind: "surface" },
     routing = null,
+    dropAmbiguous = false,
   } = context;
 
   const violations = [];
@@ -754,35 +782,52 @@ export function buildProposal(rawResult, context) {
   // description line, or the description line(s) a created skill adds. Bodies stay
   // free-until-triggered and never enter it.
   const running = new Map();
-  for (const edit of accepted) {
+  /** @type {{ id: string, title: string, reason: string }[]} */
+  const dropped = [];
+  for (const edit of [...accepted]) {
     const targets = filesOfEdit(edit);
     let memoryDelta = 0;
     let otherDelta = 0;
     let descriptionDelta = edit.kind === "extract" ? createdDescriptionCost(edit) : 0;
+    // Staged until the whole edit applies, so a dropped edit leaves no half-applied file behind.
+    const next = new Map();
     try {
       for (const target of targets) {
         const slice = sliceEditForFile(edit, target);
         const before =
           running.get(target) ??
           (target === memoryFile.path ? memoryFile.text : (measured.originals?.get(target) ?? ""));
-        const next = applyEdit(before, slice);
-        running.set(target, next);
-        const delta = estimateTokens(next) - estimateTokens(before);
+        const after = applyEdit(before, slice);
+        next.set(target, after);
+        const delta = estimateTokens(after) - estimateTokens(before);
         if (target === memoryFile.path) memoryDelta += delta;
         else {
           otherDelta += delta;
           if (isSkillFilePath(target, config.skillDirs || config.skillsDir)) {
-            descriptionDelta += descriptionLineDelta(before, next) * loadedCopies(repo.root, skillFiles, target);
+            descriptionDelta += descriptionLineDelta(before, after) * loadedCopies(repo.root, skillFiles, target);
           }
         }
       }
     } catch (err) {
-      violations.push(err.message);
+      const ambiguous = err.code === AMBIGUOUS_FIND;
+      if (ambiguous && dropAmbiguous) {
+        // Only the still-ambiguous edit goes; every other edit already cleared its gates.
+        accepted.splice(accepted.indexOf(edit), 1);
+        dropped.push({ id: edit.id, title: edit.title, reason: err.message });
+        notes.push(`dropped edit ${edit.id} (${edit.title}): ${err.message}`);
+        continue;
+      }
+      violations.push(
+        ambiguous
+          ? `${err.message}; an earlier change in this proposal repeats it (line numbers count the file as the changes before it leave it) - reword or remove the repeated copy, or merge the two changes into one edit`
+          : err.message,
+      );
       edit.applicable = false;
       edit.deltaTokens = 0;
       edit.descriptionDelta = 0;
       continue;
     }
+    for (const [target, text] of next) running.set(target, text);
     edit.applicable = true;
     edit.deltaTokens = edit.targetsMemoryFile ? memoryDelta : otherDelta;
     edit.descriptionDelta = descriptionDelta;
@@ -901,7 +946,7 @@ export function buildProposal(rawResult, context) {
     notes,
   };
 
-  return { proposal, violations };
+  return { proposal, violations, dropped };
 }
 
 // Display counter for the apply surface's funnel: the instructions that drew a negative

src/synthesize.js

@@ -493,7 +493,17 @@ async function annotateLoop({
       continue;
     }
 
-    const { proposal, violations } = buildProposal(parsed, { ...context, measured });
+    let built = buildProposal(parsed, { ...context, measured });
+    // The re-prompts are spent: an edit whose `find` is still ambiguous goes alone, named in
+    // the notes, instead of discarding every other edit that cleared its gates (#200).
+    if (built.violations.length && attempts >= ANNOTATE_TURNS) {
+      const retried = buildProposal(parsed, { ...context, measured, dropAmbiguous: true });
+      if (retried.dropped.length) {
+        built = retried;
+        for (const { id, title, reason } of retried.dropped) warn(`dropping edit ${id} (${title}): ${reason}`);
+      }
+    }
+    const { proposal, violations } = built;
     proposal.notes = [...proposal.notes, ...notes];
     proposal.usage = usage;
     proposal.overflowTarget = overflow;

test/nested.test.js

@@ -812,6 +812,103 @@ test("one proposal covers every file: nested edits are renumbered, labeled by fi
   );
 });
 
+test("a root edit dropped from the middle never leaves its id to a nested edit", () => {
+  // The model's second edit rewrites a line the first one copied, so its `find` is ambiguous
+  // once the copy lands and the last run of synthesis drops it: the root is left with [e1, e3].
+  const rootText = [
+    ...Array.from({ length: 6 }, (_, i) => `- Rule number ${i + 1} is unique.`),
+    "- Keep transactions short.",
+    "- Rule number 7 is unique.",
+    "- Rule number 8 is unique.",
+    "",
+  ].join("\n");
+  const repo = makeRepo({ "AGENTS.md": rootText, [API.path]: "# API\n\n- Handlers live in src/routes.\n" });
+  const config = { budgetTokens: 5000, maxEditsPerRun: 5, minGapEvidence: 2, skillsDir: ".agents/skills" };
+  const evidence = [
+    { polarity: "negative", text: "it hit this twice", source: "a" },
+    { polarity: "negative", text: "and again", source: "b" },
+  ];
+  const edit = (change, title) => ({ changes: [change], kind: "rewrite", title, evidence });
+
+  const rootStaged = stageAndMeasure({
+    repo,
+    edit: (dir) =>
+      writeIn(dir, "AGENTS.md", (text) =>
+        text
+          .replace("- Rule number 1 is unique.\n", "- Rule number 1 is unique.\n- Keep transactions short.\n")
+          .replace("- Keep transactions short.\n- Rule number 7", "- Keep every transaction short.\n- Rule number 7")
+          .replace("- Rule number 8 is unique.", "- Rule number 8 is unique, and checked."),
+      ),
+  });
+  const root = buildProposal(
+    { edits: [edit("H1", "copy"), edit("H2", "rewrite"), edit("H3", "tail")] },
+    {
+      memoryFile: rootStaged.memoryFile,
+      config,
+      repo,
+      summary: null,
+      measured: rootStaged.measured,
+      dropAmbiguous: true,
+    },
+  );
+  assert.deepEqual(root.violations, []);
+  assert.deepEqual(
+    root.proposal.edits.map((e) => [e.id, e.title]),
+    [
+      ["e1", "copy"],
+      ["e3", "tail"],
+    ],
+  );
+
+  const apiStaged = stageAndMeasure({
+    repo,
+    memoryPath: API.path,
+    edit: (dir) => writeIn(dir, API.path, (text) => `${text}- Run the api contract tests before editing a handler.\n`),
+  });
+  const api = buildProposal(
+    { edits: [edit("H1", "contract tests")] },
+    {
+      memoryFile: apiStaged.memoryFile,
+      config,
+      repo,
+      summary: null,
+      measured: apiStaged.measured,
+      target: { kind: "memory", path: API.path, nested: true },
+    },
+  );
+  const proposal = mergeNestedProposals(root.proposal, [
+    {
+      weight: { ...API, file: apiStaged.memoryFile },
+      transcripts: [{}, {}],
+      proposal: api.proposal,
+      summary: null,
+      cap: 5000,
+      skipped: null,
+    },
+  ]);
+
+  const ids = proposal.edits.map((e) => e.id);
+  assert.equal(new Set(ids).size, ids.length, `edit ids must be unique: ${ids.join(", ")}`);
+  assert.deepEqual(ids, ["e1", "e3", "e4"]);
+  assert.deepEqual(proposal.nested[0].edits, ["e4"]);
+
+  // Rejecting the root's e3 must not take the nested edit with it.
+  const result = applyDecisions({
+    proposal,
+    decisions: { e1: "accepted", e3: "rejected", e4: "accepted" },
+    repo,
+    state: rootStaged.state,
+    config: { ...config, nestedBudgetTokens: 5000, nestedMemoryFiles: [API.path] },
+  });
+  assert.deepEqual(result.failed, []);
+  assert.equal(result.accepted, 2);
+  assert.equal(result.rejected, 1);
+  const rootAfter = fs.readFileSync(path.join(repo.root, "AGENTS.md"), "utf8");
+  assert.match(rootAfter, /- Rule number 1 is unique\.\n- Keep transactions short\./);
+  assert.doesNotMatch(rootAfter, /and checked/);
+  assert.match(fs.readFileSync(path.join(repo.root, API.path), "utf8"), /Run the api contract tests/);
+});
+
 test("apply holds each nested file to its own budget, and writes nothing when the accepted subset breaks it", () => {
   // Made under a cap its one edit breaks: the writer is the gate of last resort even
   // for a proposal `cmdApply` would refuse for its violations.