diff --git a/src/trackChangesModel.ts b/src/trackChangesModel.ts index 3a5a6ce..c8afc1c 100644 --- a/src/trackChangesModel.ts +++ b/src/trackChangesModel.ts @@ -211,13 +211,23 @@ export interface EditHunk { * unchanged parts reconstruct it exactly, so advancing on those yields true * source offsets. Adjacent added/removed runs coalesce into one hunk; an * unchanged part flushes the open hunk. + * + * A PURE insertion (added text between two unchanged regions) would otherwise be + * a zero-width hunk (start==end) — and F4's fingerprint of empty text can never + * resolve (anchorer.resolve orphans an empty needle), so the proposal could + * never be accepted. Every zero-width hunk is therefore anchored to an adjacent + * source token (`anchorInsertion`): its range absorbs that token and its + * replacement keeps it, so the net text is identical but `fp.text` is non-empty + * and resolvable. A zero-width hunk is always bordered by unchanged text (or a + * doc edge), so the absorbed token is genuinely present in `currentText` and is + * never part of another hunk. */ export function diffToHunks(currentText: string, rewrittenText: string): EditHunk[] { const hunks: EditHunk[] = []; let offset = 0; let open: EditHunk | null = null; const flush = () => { - if (open) hunks.push(open); + if (open) hunks.push(open.start === open.end ? anchorInsertion(open, currentText) : open); open = null; }; for (const part of diffWordsWithSpace(currentText, rewrittenText)) { @@ -237,6 +247,31 @@ export function diffToHunks(currentText: string, rewrittenText: string): EditHun return hunks; } +const isWs = (c: string): boolean => /\s/.test(c); + +/** + * Grow a zero-width insertion hunk to absorb an adjacent source token so its F4 + * fingerprint has real text to resolve. Prefer the FOLLOWING token (optional + * leading whitespace + a run of non-whitespace); at end-of-document, fall back to + * the PRECEDING token. The absorbed text is kept in the replacement, so the + * applied result is unchanged. + */ +function anchorInsertion(hunk: EditHunk, text: string): EditHunk { + const p = hunk.start; + if (p < text.length) { + let e = p; + while (e < text.length && isWs(text[e])) e++; + while (e < text.length && !isWs(text[e])) e++; + if (e === p) e = Math.min(text.length, p + 1); + return { start: p, end: e, replacement: hunk.replacement + text.slice(p, e) }; + } + let s = p; + while (s > 0 && isWs(text[s - 1])) s--; + while (s > 0 && !isWs(text[s - 1])) s--; + if (s === p) s = Math.max(0, p - 1); + return { start: s, end: p, replacement: text.slice(s, p) + hunk.replacement }; +} + const md = new MarkdownIt({ html: true, linkify: false, breaks: false }); // mermaid fences →
SRCfor client-side rendering; all // other fences fall through to markdown-it's default (escaped
).
@@ -427,6 +462,12 @@ export function colorByAuthor(
* preview is still a selection→source mapping surface (INV-36) while staying
* visually clean — no `cw-` annotation classes. A doc with no blocks (empty /
* whitespace-only) renders whole. Pure + deterministic.
+ *
+ * Tradeoff of the locked block-level mapping (§6.7): rendering PER BLOCK (so each
+ * carries its offsets) means a markdown construct split across blank-line-
+ * separated blocks — a reference-link use and its definition — doesn't resolve
+ * across blocks. `renderReview` already rendered per-block; this keeps the two
+ * modes consistent rather than faithful-but-different. Characterized in tests.
*/
export function renderPlain(currentText: string, opts: RenderOptions = {}): string {
const render = opts.render ?? defaultRender;
diff --git a/src/trackChangesPreview.ts b/src/trackChangesPreview.ts
index 93b5a02..85afc8e 100644
--- a/src/trackChangesPreview.ts
+++ b/src/trackChangesPreview.ts
@@ -56,6 +56,11 @@ export class TrackChangesPreviewController implements vscode.Disposable {
const { runEditTurn } = await import("./liveTurn");
return runEditTurn(instruction, text);
};
+ /** Monotonic per-session counter minting a stable turnId for each Ask-Claude gesture. */
+ private turnSeq = 0;
+ private nextTurnSeq(): number {
+ return ++this.turnSeq;
+ }
constructor(
private readonly diffView: DiffViewController,
@@ -219,6 +224,9 @@ export class TrackChangesPreviewController implements vscode.Disposable {
instruction: string,
): Promise {
const full = document.getText();
+ // One turnId per gesture — the document case's N hunk-proposals all share it,
+ // so a single rewrite groups as one agent turn (parity with editSelection).
+ const turnId = `turn-${this.nextTurnSeq()}`;
const provenance = (turn: EditTurnResult) =>
({ kind: "agent" as const, id: "claude", agent: { sdk: "@cline/sdk", model: turn.model, sessionId: turn.sessionId } });
if (target.kind === "range") {
@@ -226,14 +234,14 @@ export class TrackChangesPreviewController implements vscode.Disposable {
const turn = await this.editTurn(instruction, selected);
if (turn.replacement === "" || turn.replacement === selected) return [];
const fp = buildFingerprint(full, { start: target.start, end: target.end });
- const id = await this.proposals.propose(document, fp, turn.replacement, provenance(turn), { instruction });
+ const id = await this.proposals.propose(document, fp, turn.replacement, provenance(turn), { turnId, instruction });
return id ? [id] : [];
}
const turn = await this.editTurn(instruction, full);
const ids: string[] = [];
for (const h of diffToHunks(full, turn.replacement)) {
const fp = buildFingerprint(full, { start: h.start, end: h.end });
- const id = await this.proposals.propose(document, fp, h.replacement, provenance(turn), { instruction });
+ const id = await this.proposals.propose(document, fp, h.replacement, provenance(turn), { turnId, instruction });
if (id) ids.push(id);
}
return ids;
diff --git a/test/e2e/suite/f11Toolbar.test.ts b/test/e2e/suite/f11Toolbar.test.ts
index 3042fa3..f85ae61 100644
--- a/test/e2e/suite/f11Toolbar.test.ts
+++ b/test/e2e/suite/f11Toolbar.test.ts
@@ -144,6 +144,33 @@ suite("F11 preview toolbar (host E2E — message → seam wiring, no LLM)", () =
assert.strictEqual(ids.length, 0, "an unchanged replacement proposes nothing");
});
+ // SLICE-3 (review follow-up): a document rewrite that INSERTS text → an
+ // acceptable proposal (not a born-orphaned zero-width hunk), and accepting all
+ // hunks of a multi-hunk rewrite lands the intended document.
+ test("document rewrite with an insertion → acceptable proposals; accept-all reaches the rewrite", async () => {
+ const original = "# F11 accept\n\nThe brown fox sleeps.\n";
+ const rewrite = "# F11 accept\n\nThe brown fox QUIETLY sleeps today.\n";
+ const { doc } = await freshDoc("docs/f11accept.md", original);
+ const api = await getApi();
+ const ctl = api.trackChangesPreviewController;
+ await vscode.commands.executeCommand("cowriting.showTrackChangesPreview");
+ await settle();
+
+ ctl.setEditTurnForTest(async () => ({ replacement: rewrite, model: "sonnet", sessionId: "e2e-accept" }));
+ const ids = await ctl.runEditAndPropose(doc, { kind: "document" }, "expand the sentence");
+ await settle();
+ assert.ok(ids.length >= 1, "the rewrite produced at least one proposal");
+
+ // Every proposal must be acceptable (the insertion-anchoring fix): accept each.
+ for (const id of ids) {
+ const ok = await api.proposalController.acceptById("docs/f11accept.md", id);
+ assert.ok(ok, `proposal ${id} is acceptable (not born-orphaned)`);
+ await settle();
+ }
+ assert.strictEqual(doc.getText(), rewrite, "accepting all hunks reconstructs the intended rewrite");
+ assert.strictEqual(api.proposalController.listProposals(doc).length, 0, "no proposals left pending");
+ });
+
// SLICE-3: the document-scoped command exists for #42 reuse, guarded on markdown.
test("cowriting.editDocument is a registered command, palette-guarded on markdown", async () => {
const all = await vscode.commands.getCommands(true);
diff --git a/test/trackChangesModel.test.ts b/test/trackChangesModel.test.ts
index 7389fa6..6c2b721 100644
--- a/test/trackChangesModel.test.ts
+++ b/test/trackChangesModel.test.ts
@@ -360,6 +360,46 @@ describe("F11 diffToHunks (INV-37)", () => {
const b = diffToHunks("a b c d", "a B c D");
expect(a).toEqual(b);
});
+
+ /** Apply hunks (right→left so earlier offsets stay valid) to reconstruct the rewrite. */
+ const applyHunks = (current: string, hunks: ReturnType): string => {
+ let out = current;
+ for (const h of [...hunks].sort((a, b) => b.start - a.start)) {
+ out = out.slice(0, h.start) + h.replacement + out.slice(h.end);
+ }
+ return out;
+ };
+
+ test("applying the hunks always reconstructs the rewrite (substitute / delete / insert / multi)", () => {
+ const cases: Array<[string, string]> = [
+ ["The quick brown fox.", "The quick red fox."],
+ ["one two three four", "one TWO three FOUR"],
+ ["alpha", "omega"],
+ ["keep this and drop that", "keep this"],
+ ["one two three", "one INSERTED two three"],
+ ["start middle end", "PREFIX start middle end SUFFIX"],
+ ["unchanged body", "unchanged body"],
+ ];
+ for (const [current, rewrite] of cases) {
+ expect(applyHunks(current, diffToHunks(current, rewrite))).toBe(rewrite);
+ }
+ });
+
+ test("an inserted run anchors to adjacent text — never a zero-width, unacceptable hunk (INV-37)", () => {
+ // A pure insertion would otherwise produce start==end → an empty fingerprint
+ // → resolve() orphans it → the proposal can never be accepted. Each hunk must
+ // span real source text so its F4 fingerprint resolves.
+ for (const [current, rewrite] of [
+ ["one two three", "one INSERTED two three"],
+ ["tail anchor", "tail anchor APPENDED"],
+ ["lead", "PREPENDED lead"],
+ ] as Array<[string, string]>) {
+ for (const h of diffToHunks(current, rewrite)) {
+ expect(h.end).toBeGreaterThan(h.start); // non-zero-width
+ expect(current.slice(h.start, h.end).length).toBeGreaterThan(0); // real fp.text
+ }
+ }
+ });
});
// F11 SLICE-2 (INV-36): the pure render layer emits data-src-start/data-src-end
@@ -408,4 +448,23 @@ describe("F11 data-src emission (INV-36)", () => {
const r2 = renderReview("x", "x\n\ny", [], []);
expect(r1).toBe(r2);
});
+
+ // CHARACTERIZATION (conscious tradeoff of the locked block-level mapping, §6.7
+ // fork 1): both modes render markdown PER BLOCK so each block can carry its
+ // data-src offsets. A consequence is that markdown constructs whose parts span
+ // blank-line-separated blocks — a reference-link USE and its DEFINITION — do not
+ // resolve across blocks (markdown-it sees each block in isolation). renderReview
+ // already had this; F11 brings the off/clean preview into line with it (both
+ // per-block) rather than leaving the two modes rendering differently. If
+ // cross-block fidelity is wanted later, it is a follow-up (source-map driven
+ // wrapping), not a change to the locked block-level decision.
+ test("a reference-link definition in a separate block does not resolve (block-level rendering)", () => {
+ const doc = "See [the spec][ref] for details.\n\n[ref]: https://example.com/spec\n";
+ const html = renderPlain(doc);
+ // the link is NOT resolved to an — the [text][ref] is rendered literally.
+ expect(html).not.toContain('href="https://example.com/spec"');
+ expect(html).toContain("[the spec][ref]");
+ // both modes agree (renderReview is likewise per-block) — the consistency point.
+ expect(renderReview(doc, doc, [], [])).not.toContain('href="https://example.com/spec"');
+ });
});