diff --git a/src/trackChangesModel.ts b/src/trackChangesModel.ts index 1c467b1..13989d9 100644 --- a/src/trackChangesModel.ts +++ b/src/trackChangesModel.ts @@ -422,8 +422,13 @@ function renderReviewOp( * On-state body (INV-33): the F7 baseline diff — added/changed PROSE author-colored * via colorByAuthor (F9 sentinels), deletions struck — overlaid with F4 pending * proposals as blue cw-proposal blocks (✓/✗). One pass, pure, vscode-free. - * Resolved proposals append after the diff body; unresolved ones append as trailing - * cw-proposal-unanchored blocks (never dropped — INV-34). + * + * A resolved proposal renders INLINE, right after the current-side block its anchor + * falls in (#31) — so "what is Claude proposing, and where?" is answered by + * position. A proposal whose anchor does not resolve (or falls before the first + * block) renders as a trailing cw-proposal-unanchored block (never dropped — + * INV-34). Deterministic: proposals in the same block are ordered by anchorStart + * then id; trailing proposals keep input order. */ export function renderReview( baselineText: string, @@ -435,17 +440,43 @@ export function renderReview( const render = opts.render ?? defaultRender; const ranges = splitBlocksWithRanges(currentText); const ops = diffBlocks(baselineText, currentText); + + // Associate each resolved proposal with the current-side block index whose range + // it anchors into: the largest block with start <= anchorStart (the containing + // block, or the nearest preceding block when the anchor sits in a gap). A + // resolved anchor before all blocks, and every unresolved proposal, trails. + const blockOf = (a: number): number => { + let j = -1; + for (let k = 0; k < ranges.length && ranges[k].start <= a; k++) j = k; + return j; + }; + const byBlock = new Map(); + const trailing: ProposalView[] = []; + for (const p of proposals) { + const j = p.anchorStart === null ? -1 : blockOf(p.anchorStart); + if (j < 0) { + trailing.push(p); + continue; + } + (byBlock.get(j) ?? byBlock.set(j, []).get(j)!).push(p); + } + for (const arr of byBlock.values()) { + arr.sort((a, b) => a.anchorStart! - b.anchorStart! || (a.id < b.id ? -1 : a.id > b.id ? 1 : 0)); + } + let ci = 0; // pointer into ranges; advances for every op with a current-side block - const bodyParts = ops.map((op) => { + const bodyParts: string[] = []; + for (const op of ops) { + 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); - return renderReviewOp(op, render, colored); - }); - const anchored = proposals.filter((p) => p.anchorStart !== null); - const unanchored = proposals.filter((p) => p.anchorStart === null); - const proposalParts = [...anchored, ...unanchored].map((p) => proposalBlockHtml(p, render)); - return [...bodyParts, ...proposalParts].join("\n"); + bodyParts.push(renderReviewOp(op, render, colored)); + const here = blockIndex >= 0 ? byBlock.get(blockIndex) : undefined; + if (here) for (const p of here) bodyParts.push(proposalBlockHtml(p, render)); + } + for (const p of trailing) bodyParts.push(proposalBlockHtml(p, render)); + return bodyParts.join("\n"); } /** Pure entry point: annotated HTML body for the preview (INV-22). */ diff --git a/test/e2e/suite/f10Review.test.ts b/test/e2e/suite/f10Review.test.ts index 94b719a..03fd037 100644 --- a/test/e2e/suite/f10Review.test.ts +++ b/test/e2e/suite/f10Review.test.ts @@ -104,6 +104,11 @@ suite("F10 interactive review (host E2E — preview is the single review surface const html = api.trackChangesPreviewController.renderHtmlFor(key); assert.ok(html.includes(`data-proposal-id="${id}"`), "the preview renders the proposal block by id"); assert.match(html, /class="cw-actions"/, "the proposal block carries ✓/✗ actions"); + // #31: the proposal renders INLINE at its anchor (right after T1's block), not + // trailing the whole document — so it appears BEFORE the following T2 block. + const pIdx = html.indexOf(`data-proposal-id="${id}"`); + const t2Idx = html.indexOf("second claude target"); + assert.ok(t2Idx >= 0 && pIdx < t2Idx, "the proposal renders in place, before the following block (#31)"); // INV-10: proposing never touches the document. assert.ok(doc.getText().includes(T1), "document unchanged by propose"); }); diff --git a/test/trackChangesModel.test.ts b/test/trackChangesModel.test.ts index 5a2c306..72c06e7 100644 --- a/test/trackChangesModel.test.ts +++ b/test/trackChangesModel.test.ts @@ -231,6 +231,63 @@ describe("renderReview", () => { const count = (html.match(/cw-by-human/g) ?? []).length; expect(count).toBe(1); }); + + // #31 — a resolved proposal renders INLINE at its anchor's block, not as a trailing block. + test("renderReview: a resolved proposal renders in place after its anchor's block, not trailing", () => { + // Two paragraphs; "Alpha here" at 0..10, "Beta there" at 12..22. + const doc = "Alpha here\n\nBeta there"; + const proposals: ProposalView[] = [{ id: "p1", anchorStart: 0, anchorEnd: 5, replaced: "Alpha", replacement: "Omega" }]; + const html = renderReview(doc, doc, [], proposals); + const pIdx = html.indexOf('data-proposal-id="p1"'); + const alphaIdx = html.indexOf("Alpha here"); + const betaIdx = html.indexOf("Beta there"); + expect(pIdx).toBeGreaterThan(alphaIdx); // after its own block + expect(pIdx).toBeLessThan(betaIdx); // and BEFORE the next block (in place, not trailing) + }); + test("renderReview: two proposals in distinct blocks each render after their own block", () => { + const doc = "Alpha here\n\nBeta there"; + const proposals: ProposalView[] = [ + { id: "pA", anchorStart: 0, anchorEnd: 5, replaced: "Alpha", replacement: "Omega" }, + { id: "pB", anchorStart: 12, anchorEnd: 16, replaced: "Beta", replacement: "Gamma" }, + ]; + const html = renderReview(doc, doc, [], proposals); + const betaIdx = html.indexOf("Beta there"); + expect(html.indexOf('data-proposal-id="pA"')).toBeLessThan(betaIdx); // before second block + expect(html.indexOf('data-proposal-id="pB"')).toBeGreaterThan(betaIdx); // after second block + }); + test("renderReview: two proposals in the same block are ordered by anchorStart (deterministic)", () => { + // single block "one two three": one@0, two@4, three@8 + const doc = "one two three"; + const proposals: ProposalView[] = [ + { id: "late", anchorStart: 8, anchorEnd: 13, replaced: "three", replacement: "trois" }, + { id: "early", anchorStart: 0, anchorEnd: 3, replaced: "one", replacement: "uno" }, + ]; + const html = renderReview(doc, doc, [], proposals); + expect(html.indexOf('data-proposal-id="early"')).toBeLessThan(html.indexOf('data-proposal-id="late"')); + }); + test("renderReview: a resolved proposal renders before a trailing unanchored one (INV-34)", () => { + const doc = "Alpha here\n\nBeta there"; + const proposals: ProposalView[] = [ + { id: "anc", anchorStart: 0, anchorEnd: 5, replaced: "Alpha", replacement: "Omega" }, + { id: "unanc", anchorStart: null, anchorEnd: null, replaced: "x", replacement: "y" }, + ]; + const html = renderReview(doc, doc, [], proposals); + const uIdx = html.indexOf('data-proposal-id="unanc"'); + expect(html.indexOf('data-proposal-id="anc"')).toBeLessThan(uIdx); // resolved in-place first + expect(uIdx).toBeGreaterThan(html.indexOf("Beta there")); // unanchored still trails the body + expect(html).toContain("cw-proposal-unanchored"); + }); + test("renderReview is deterministic with mixed anchored/unanchored proposals", () => { + const doc = "one two three"; + const proposals: ProposalView[] = [ + { id: "late", anchorStart: 8, anchorEnd: 13, replaced: "three", replacement: "trois" }, + { id: "early", anchorStart: 0, anchorEnd: 3, replaced: "one", replacement: "uno" }, + { id: "floating", anchorStart: null, anchorEnd: null, replaced: "z", replacement: "w" }, + ]; + const a = renderReview(doc, doc, [], proposals); + const b = renderReview(doc, doc, [], proposals); + expect(a).toBe(b); + }); }); import { renderTrackChanges as rtc2 } from "../src/trackChangesModel";