From fdd743490d4b0622e8553ce88140a2509e1bcda1 Mon Sep 17 00:00:00 2001 From: Ben Stull Date: Sat, 13 Jun 2026 08:28:52 -0700 Subject: [PATCH] #48: pinning the baseline leaves the review panel fully un-annotated MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A pinned baseline with no changes since should read as a clean starting point, but the F10 on-render still author-colored every block (colorByAuthor), painting the whole document green/blue right after a pin. Now, when the baseline reason is "pinned" and there are zero changes since (every diffBlocks op unchanged), the on-render is fully clean — no change marks and no authorship coloring — while the data-src block mapping (INV-36) and any pending proposals (review actions, not annotations) are kept. Scoped to the PIN specifically, not all zero-diff: a baseline advanced by a machine-landing (accept) is also zero-diff but keeps its authorship coloring so accepted Claude text stays blue (F10 INV-33). renderReview takes a `pinned` option; the controller passes baseline.reason === "pinned" from both refresh and the renderHtmlFor test seam. - trackChangesModel.ts: renderReview gains the `pinned` RenderOption; when pinned + zero-diff, blocks render plain (no colorByAuthor). - trackChangesPreview.ts: pass { pinned } through refresh + renderHtmlFor. - unit: pinned+zero-diff → no cw-by-*; pinned+zero-diff still shows proposals; zero-diff WITHOUT pin (machine-landing) keeps coloring (INV-33); real changes after a pin re-color. - s48PinClean host E2E: type → colored; pin → clean; edit → annotations return. 218 unit + 74/5 host E2E green. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/trackChangesModel.ts | 18 ++++++++- src/trackChangesPreview.ts | 3 +- test/e2e/suite/s48PinClean.test.ts | 63 ++++++++++++++++++++++++++++++ test/trackChangesModel.test.ts | 48 +++++++++++++++++++++++ 4 files changed, 130 insertions(+), 2 deletions(-) create mode 100644 test/e2e/suite/s48PinClean.test.ts diff --git a/src/trackChangesModel.ts b/src/trackChangesModel.ts index f96da2c..32cc777 100644 --- a/src/trackChangesModel.ts +++ b/src/trackChangesModel.ts @@ -441,6 +441,14 @@ function wordMergedMarkdown(beforeRaw: string, afterRaw: string): string { export interface RenderOptions { /** Per-block markdown→HTML renderer (test seam). Defaults to the bundled markdown-it. */ render?: (src: string) => string; + /** + * #48: the baseline was just PINNED (`reason === "pinned"`). With zero changes + * since that pin, the on-render is fully clean — no authorship coloring — so a + * pin reads as "this is my clean starting point". Distinct from a baseline + * advanced by a machine-landing (accept), which keeps its authorship coloring + * (F10 INV-33). Only consulted by `renderReview`. + */ + pinned?: boolean; } function defaultRender(src: string): string { @@ -673,6 +681,14 @@ export function renderReview( const render = opts.render ?? defaultRender; const ranges = splitBlocksWithRanges(currentText); const ops = diffBlocks(baselineText, currentText); + // #48: right after a PIN (baseline reason "pinned") with no changes since, the + // panel is fully clean: no change marks (already absent) AND no authorship + // coloring, so the pin reads as "this is my clean starting point". Skip + // colorByAuthor for every block in this case; data-src mapping and any pending + // proposals (review actions, not annotations) are kept below. A baseline advanced + // by a machine-landing (accept) is ALSO zero-diff but is NOT pinned — it keeps + // its authorship coloring (F10 INV-33), so this is gated on the pin specifically. + const clean = opts.pinned === true && ops.every((o) => o.kind === "unchanged"); // Associate each resolved proposal with the current-side block index whose range // it anchors into: the largest block with start <= anchorStart (the containing @@ -703,7 +719,7 @@ export function renderReview( const blockIndex = op.kind === "removed" ? -1 : ci; const blk = op.kind === "removed" ? undefined : ranges[ci++]; const colored = (raw: string): string => - blk ? colorByAuthor(raw, blk.start, authorSpans, render) : render(raw); + blk && !clean ? colorByAuthor(raw, blk.start, authorSpans, render) : render(raw); bodyParts.push(renderReviewOp(op, render, colored, srcAttr(blk))); const here = blockIndex >= 0 ? byBlock.get(blockIndex) : undefined; if (here) for (const p of here) bodyParts.push(proposalBlockHtml(p, render)); diff --git a/src/trackChangesPreview.ts b/src/trackChangesPreview.ts index f3be21e..f7c7f38 100644 --- a/src/trackChangesPreview.ts +++ b/src/trackChangesPreview.ts @@ -337,7 +337,7 @@ export class TrackChangesPreviewController implements vscode.Disposable { void panel.webview.postMessage({ type: "render", mode, - html: renderReview(baselineText, current, spans, proposals), + html: renderReview(baselineText, current, spans, proposals, { pinned: baseline?.reason === "pinned" }), epoch: this.epochLabel(baseline), summary, authorable, @@ -464,6 +464,7 @@ export class TrackChangesPreviewController implements vscode.Disposable { current, this.attribution.spansFor(doc), this.proposals.listProposals(doc), + { pinned: baseline?.reason === "pinned" }, ); } /** F10: current annotations mode for a panel (default on). */ diff --git a/test/e2e/suite/s48PinClean.test.ts b/test/e2e/suite/s48PinClean.test.ts new file mode 100644 index 0000000..c34a6c3 --- /dev/null +++ b/test/e2e/suite/s48PinClean.test.ts @@ -0,0 +1,63 @@ +import * as assert from "assert"; +import * as fs from "fs"; +import * as path from "path"; +import * as vscode from "vscode"; +import type { CowritingApi } from "../../../src/extension"; + +const WS = process.env.E2E_WORKSPACE!; +const settle = () => new Promise((r) => setTimeout(r, 400)); + +async function getApi(): Promise { + const ext = vscode.extensions.getExtension("benstull.vscode-cowriting-plugin")!; + const api = (await ext.activate()) as CowritingApi; + assert.ok(api?.trackChangesPreviewController, "exports preview controller"); + return api; +} + +async function freshDoc(rel: string, body: string): Promise<{ doc: vscode.TextDocument; key: string }> { + const abs = path.join(WS, rel); + fs.mkdirSync(path.dirname(abs), { recursive: true }); + fs.writeFileSync(abs, body, "utf8"); + const uri = vscode.Uri.file(abs); + const doc = await vscode.workspace.openTextDocument(uri); + await vscode.window.showTextDocument(doc); + await settle(); + return { doc, key: uri.toString() }; +} + +// #48 host E2E (no LLM): pinning the baseline leaves the review panel fully clean +// — no authorship coloring on unchanged blocks — while re-divergence brings the +// annotations back. The author colors are read from the on-state renderReview HTML. +suite("S48 — pin → fully clean review panel (host E2E, no LLM)", () => { + test("pin clears authorship coloring; a later edit brings annotations back", async () => { + const { doc, key } = await freshDoc("docs/s48pin.md", "# S48\n\nAn original baseline paragraph.\n"); + const api = await getApi(); + const ctl = api.trackChangesPreviewController; + await vscode.commands.executeCommand("cowriting.showTrackChangesPreview"); + await settle(); + + // Type a paragraph → a human attribution span + author coloring in the on-state. + const edit = new vscode.WorkspaceEdit(); + edit.insert(doc.uri, doc.positionAt(doc.getText().length), "\n\nA freshly typed human paragraph.\n"); + assert.ok(await vscode.workspace.applyEdit(edit), "operator edit applied"); + await settle(); + assert.match(ctl.renderHtmlFor(key), /cw-by-human/, "typed text is author-colored before the pin"); + + // Pin the baseline → zero diff → the panel must be fully clean. + ctl.receiveMessage(key, { type: "pinBaseline" }); + await settle(); + const pinned = ctl.renderHtmlFor(key); + assert.ok(!pinned.includes("cw-by-human"), "no human authorship coloring after pin"); + assert.ok(!pinned.includes("cw-by-claude"), "no Claude authorship coloring after pin"); + assert.ok(!pinned.includes("cw-add") && !pinned.includes("cw-del"), "no change marks after pin"); + assert.match(pinned, /data-src-start/, "blocks still carry data-src offsets (INV-36 mapping kept)"); + assert.ok(pinned.includes("freshly typed human paragraph"), "the body text is still rendered, just plain"); + + // Edit again → there are changes since the pinned baseline → annotations return. + const edit2 = new vscode.WorkspaceEdit(); + edit2.insert(doc.uri, doc.positionAt(doc.getText().length), "\n\nA second typed paragraph diverges again.\n"); + assert.ok(await vscode.workspace.applyEdit(edit2), "second operator edit applied"); + await settle(); + assert.match(ctl.renderHtmlFor(key), /cw-by-human/, "authorship coloring returns once the doc diverges from the pin"); + }); +}); diff --git a/test/trackChangesModel.test.ts b/test/trackChangesModel.test.ts index 9f83363..cb2e5c5 100644 --- a/test/trackChangesModel.test.ts +++ b/test/trackChangesModel.test.ts @@ -277,6 +277,54 @@ describe("renderReview", () => { expect(uIdx).toBeGreaterThan(html.indexOf("Beta there")); // unanchored still trails the body expect(html).toContain("cw-proposal-unanchored"); }); + // #48: a PINNED baseline with zero changes leaves the panel fully un-annotated — + // no authorship coloring on unchanged blocks — while pending proposals still show. + test("renderReview: pinned + zero diff with author spans renders NO authorship coloring", () => { + const doc = "Human wrote this.\n\nClaude wrote that."; + const spans: AuthorSpan[] = [ + { start: 0, end: 17, author: "human" }, + { start: 19, end: doc.length, author: "claude" }, + ]; + const html = renderReview(doc, doc, spans, [], { pinned: true }); + expect(html).not.toContain("cw-by-human"); + expect(html).not.toContain("cw-by-claude"); + expect(html).not.toContain("cw-add"); + expect(html).not.toContain("cw-del"); + // still a selection→source surface (INV-36): blocks carry data-src offsets. + expect(html).toContain("data-src-start"); + // the body text is still there, just plain. + expect(html).toContain("Human wrote this."); + expect(html).toContain("Claude wrote that."); + }); + test("renderReview: pinned + zero diff still renders a pending proposal block", () => { + const doc = "Human wrote this.\n\nClaude wrote that."; + const spans: AuthorSpan[] = [{ start: 0, end: 17, author: "human" }]; + const proposals: ProposalView[] = [ + { id: "p1", anchorStart: 0, anchorEnd: 5, replaced: "Human", replacement: "Person" }, + ]; + const html = renderReview(doc, doc, spans, proposals, { pinned: true }); + expect(html).not.toContain("cw-by-human"); // body still clean + expect(html).toContain('data-proposal-id="p1"'); // proposal still shows (it is an action) + expect(html).toContain("Person"); + }); + test("renderReview: zero diff WITHOUT a pin (e.g. machine-landing) keeps authorship coloring (INV-33)", () => { + // accepting a Claude edit advances the baseline (zero diff) but is NOT a pin — + // the landed author coloring must remain. + const doc = "Human wrote this.\n\nClaude wrote that."; + const spans: AuthorSpan[] = [{ start: 19, end: doc.length, author: "claude" }]; + const html = renderReview(doc, doc, spans, []); // no pinned flag + expect(html).toContain("cw-by-claude"); + }); + test("renderReview: with REAL changes since a pin, author coloring returns", () => { + // a genuine added block is still author-colored even when pinned (only the + // zero-diff-after-pin state is clean). + const baseline = "Hello world"; + const current = "Hello world\n\nHello world"; + const spans: AuthorSpan[] = [{ start: 19, end: 24, author: "human" }]; + const html = renderReview(baseline, current, spans, [], { pinned: true }); + expect((html.match(/cw-by-human/g) ?? []).length).toBe(1); + }); + test("renderReview is deterministic with mixed anchored/unanchored proposals", () => { const doc = "one two three"; const proposals: ProposalView[] = [