Merge pull request 'F10 #31: render resolved proposals inline at their anchor block' (#36) from feature/31-inline-anchor-proposals into main
This commit was merged in pull request #36.
This commit is contained in:
@@ -422,8 +422,13 @@ function renderReviewOp(
|
|||||||
* On-state body (INV-33): the F7 baseline diff — added/changed PROSE author-colored
|
* On-state body (INV-33): the F7 baseline diff — added/changed PROSE author-colored
|
||||||
* via colorByAuthor (F9 sentinels), deletions struck — overlaid with F4 pending
|
* via colorByAuthor (F9 sentinels), deletions struck — overlaid with F4 pending
|
||||||
* proposals as blue cw-proposal blocks (✓/✗). One pass, pure, vscode-free.
|
* 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(
|
export function renderReview(
|
||||||
baselineText: string,
|
baselineText: string,
|
||||||
@@ -435,17 +440,43 @@ export function renderReview(
|
|||||||
const render = opts.render ?? defaultRender;
|
const render = opts.render ?? defaultRender;
|
||||||
const ranges = splitBlocksWithRanges(currentText);
|
const ranges = splitBlocksWithRanges(currentText);
|
||||||
const ops = diffBlocks(baselineText, 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<number, ProposalView[]>();
|
||||||
|
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
|
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 blk = op.kind === "removed" ? undefined : ranges[ci++];
|
||||||
const colored = (raw: string): string =>
|
const colored = (raw: string): string =>
|
||||||
blk ? colorByAuthor(raw, blk.start, authorSpans, render) : render(raw);
|
blk ? colorByAuthor(raw, blk.start, authorSpans, render) : render(raw);
|
||||||
return renderReviewOp(op, render, colored);
|
bodyParts.push(renderReviewOp(op, render, colored));
|
||||||
});
|
const here = blockIndex >= 0 ? byBlock.get(blockIndex) : undefined;
|
||||||
const anchored = proposals.filter((p) => p.anchorStart !== null);
|
if (here) for (const p of here) bodyParts.push(proposalBlockHtml(p, render));
|
||||||
const unanchored = proposals.filter((p) => p.anchorStart === null);
|
}
|
||||||
const proposalParts = [...anchored, ...unanchored].map((p) => proposalBlockHtml(p, render));
|
for (const p of trailing) bodyParts.push(proposalBlockHtml(p, render));
|
||||||
return [...bodyParts, ...proposalParts].join("\n");
|
return bodyParts.join("\n");
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Pure entry point: annotated HTML body for the preview (INV-22). */
|
/** Pure entry point: annotated HTML body for the preview (INV-22). */
|
||||||
|
|||||||
@@ -104,6 +104,11 @@ suite("F10 interactive review (host E2E — preview is the single review surface
|
|||||||
const html = api.trackChangesPreviewController.renderHtmlFor(key);
|
const html = api.trackChangesPreviewController.renderHtmlFor(key);
|
||||||
assert.ok(html.includes(`data-proposal-id="${id}"`), "the preview renders the proposal block by id");
|
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");
|
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.
|
// INV-10: proposing never touches the document.
|
||||||
assert.ok(doc.getText().includes(T1), "document unchanged by propose");
|
assert.ok(doc.getText().includes(T1), "document unchanged by propose");
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -231,6 +231,63 @@ describe("renderReview", () => {
|
|||||||
const count = (html.match(/cw-by-human/g) ?? []).length;
|
const count = (html.match(/cw-by-human/g) ?? []).length;
|
||||||
expect(count).toBe(1);
|
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";
|
import { renderTrackChanges as rtc2 } from "../src/trackChangesModel";
|
||||||
|
|||||||
Reference in New Issue
Block a user