#47 (SLICE-2, review): per-block document proposals + word-precise accept
Document-edit flow SLICE-2 (specs/coauthoring-document-edit-flow.md §7.2,
INV-39/40/41 — P1 "too much to review"). A whole-document rewrite now proposes
ONE F4 proposal per CHANGED BLOCK (the unit a human reviews), but accepting a
block reconciles attribution at WORD granularity (the unit F3 records). Block =
decision unit; word = attribution unit. Supersedes INV-37's per-word cut for
document edits; selection edits unchanged.
- trackChangesModel.ts: new pure diffToBlockHunks(current, rewritten) — block-key
alignment (reusing diffArrays/diffBlocks keying): an isolated changed block →
one block-aligned hunk → the rewritten block raw (a code/mermaid fence is one
atomic whole-fence hunk, INV-23); insert/delete runs → one gap-span hunk over
the inter-anchor region (separators included) so reconstruction stays exact; a
zero-width gap-span is anchored (INV-41). Also split diffToHunks into the raw,
un-anchored wordEditHunks + the anchoring wrapper (the anchoring could grow an
insertion to overlap an adjacent hunk, corrupting a batch apply — a latent bug
that only surfaced once hunks are applied as a batch).
- trackChangesPreview.ts: runEditAndPropose document branch uses diffToBlockHunks
and tags each proposal granularity:"block".
- model.ts / proposalModel.ts: additive optional Proposal.granularity
("block"|"single"; absent ⇒ single, back-compat — no migration).
- proposalController.ts: accept of a block proposal runs an intra-block word
sub-diff (wordEditHunks, disjoint) and applies one applyAgentEdit per changed
run, descending offset — only the words Claude changed land Claude-attributed;
unchanged spans keep prior authorship (INV-40).
- Tests: diffToBlockHunks unit (reconstruction + fence atomic + add/remove);
f12Review host E2E (M blocks→M proposals, unchanged→none, fence atomic, INV-40
attribution, INV-41 insertion accept); updated the f11 document-path E2E to
per-block (INV-39 supersedes INV-37); MANUAL-SMOKE-F12 §2.
Seam note: pendingEdits.matchEvent resolves one registration per change event, so
INV-40's per-run attribution is sequential applyAgentEdit calls (N undo steps),
not one multi-replace WorkspaceEdit — see transcript Deferred decisions.
214 unit + 69/5 host E2E green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -35,3 +35,35 @@ The body and tab menus show exactly one Ask-Claude edit entry, matching the live
|
|||||||
selection state (selection → Edit Selection; none → Edit Document); the tab
|
selection state (selection → Edit Selection; none → Edit Document); the tab
|
||||||
gesture targets the clicked tab's document, not the active editor; both are absent
|
gesture targets the clicked tab's document, not the active editor; both are absent
|
||||||
on non-markdown docs; no console errors.
|
on non-markdown docs; no console errors.
|
||||||
|
|
||||||
|
## SLICE-2 — #47 (review): per-block proposals + word-precise attribution (INV-39/40/41)
|
||||||
|
|
||||||
|
On a markdown doc with several paragraphs, **Ask Claude to Edit Document** with a
|
||||||
|
light copy-edit instruction (e.g. "tighten the prose, fix typos").
|
||||||
|
|
||||||
|
1. **One proposal per changed block (INV-39).** Claude's pass surfaces as **one
|
||||||
|
✓/✗ block per changed paragraph/header/bullet**, not a flurry of word-level
|
||||||
|
blocks. A paragraph with several word edits is a **single** proposal; the
|
||||||
|
word-level `<ins>`/`<del>` still shows *inside* it. Untouched paragraphs show no
|
||||||
|
proposal.
|
||||||
|
2. **Changed fence is atomic (INV-23).** If Claude edits a code/mermaid fence, it
|
||||||
|
is **one** whole-fence proposal.
|
||||||
|
3. **Accept attributes only the changed words (INV-40).** Accept a block proposal,
|
||||||
|
then toggle the preview to **Authorship**/colors (or re-open in the on-state):
|
||||||
|
only the words Claude actually changed are Claude-colored; the unchanged words
|
||||||
|
in that block keep their prior author. The block is the decision unit; the word
|
||||||
|
is the attribution unit.
|
||||||
|
4. **Inserted block accepts cleanly (INV-41).** If Claude adds a new paragraph,
|
||||||
|
its proposal **accepts** (it is anchored to an adjacent block, never a
|
||||||
|
born-orphaned/zero-width proposal).
|
||||||
|
5. **Undo.** Accepting a block is currently **N undo steps** (one per changed run
|
||||||
|
inside the block) — `Ctrl+Z` repeatedly restores it. (Spec deferred note:
|
||||||
|
single-undo-step grouping is a possible follow-up.)
|
||||||
|
6. **Selection edits unchanged.** Edit *Selection* still produces exactly one
|
||||||
|
proposal over the selection (no block fan-out).
|
||||||
|
|
||||||
|
### Pass criteria
|
||||||
|
|
||||||
|
A document edit yields one in-context proposal per changed block (fences atomic);
|
||||||
|
accepting a block lands the whole block but Claude-attributes only the words it
|
||||||
|
changed; inserted blocks accept; selection edits are unaffected; no console errors.
|
||||||
|
|||||||
@@ -90,6 +90,13 @@ export interface Proposal {
|
|||||||
turnId?: string;
|
turnId?: string;
|
||||||
/** what the human asked for (review context). */
|
/** what the human asked for (review context). */
|
||||||
instruction?: string;
|
instruction?: string;
|
||||||
|
/**
|
||||||
|
* F12/#47 (INV-39/40): the review-decision unit this proposal represents.
|
||||||
|
* `"block"` ⇒ the anchor spans a whole document block and accept reconciles
|
||||||
|
* attribution per WORD inside it (INV-40); `"single"` (or absent, for
|
||||||
|
* back-compat with older sidecars) ⇒ a single-range proposal accepted whole.
|
||||||
|
*/
|
||||||
|
granularity?: "block" | "single";
|
||||||
}
|
}
|
||||||
|
|
||||||
export interface Artifact {
|
export interface Artifact {
|
||||||
|
|||||||
@@ -18,7 +18,7 @@ import { addProposal, removeProposal } from "./proposalModel";
|
|||||||
import type { AttributionController } from "./attributionController";
|
import type { AttributionController } from "./attributionController";
|
||||||
import type { VersionGuard } from "./versionGuard";
|
import type { VersionGuard } from "./versionGuard";
|
||||||
import { isAuthorable } from "./workspacePath";
|
import { isAuthorable } from "./workspacePath";
|
||||||
import type { ProposalView } from "./trackChangesModel";
|
import { wordEditHunks, type ProposalView } from "./trackChangesModel";
|
||||||
|
|
||||||
/** Test-facing snapshot of what is currently rendered for a document. */
|
/** Test-facing snapshot of what is currently rendered for a document. */
|
||||||
export interface RenderedProposal {
|
export interface RenderedProposal {
|
||||||
@@ -122,7 +122,7 @@ export class ProposalController implements vscode.Disposable {
|
|||||||
fp: Fingerprint,
|
fp: Fingerprint,
|
||||||
replacement: string,
|
replacement: string,
|
||||||
author: Provenance,
|
author: Provenance,
|
||||||
opts?: { turnId?: string; instruction?: string },
|
opts?: { turnId?: string; instruction?: string; granularity?: "block" | "single" },
|
||||||
): Promise<string | undefined> {
|
): Promise<string | undefined> {
|
||||||
if (!this.isTracked(document)) return undefined;
|
if (!this.isTracked(document)) return undefined;
|
||||||
if (this.guard.isReadOnly(this.keyOf(document))) return undefined;
|
if (this.guard.isReadOnly(this.keyOf(document))) return undefined;
|
||||||
@@ -164,12 +164,19 @@ export class ProposalController implements vscode.Disposable {
|
|||||||
this.renderAll(document);
|
this.renderAll(document);
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
const range = new vscode.Range(document.positionAt(resolved.start), document.positionAt(resolved.end));
|
// #47 (INV-40): a BLOCK proposal applies the whole block but attributes only
|
||||||
|
// the words Claude actually changed; a single proposal applies its whole range.
|
||||||
|
const ok =
|
||||||
|
proposal.granularity === "block"
|
||||||
|
? await this.acceptBlock(document, resolved, proposal)
|
||||||
|
: await this.attribution.applyAgentEdit(
|
||||||
|
document,
|
||||||
|
new vscode.Range(document.positionAt(resolved.start), document.positionAt(resolved.end)),
|
||||||
|
proposal.replacement,
|
||||||
|
proposal.author,
|
||||||
// No awaits between resolve and the seam call: document.version is current.
|
// No awaits between resolve and the seam call: document.version is current.
|
||||||
const ok = await this.attribution.applyAgentEdit(document, range, proposal.replacement, proposal.author, {
|
{ expectedVersion: document.version, turnId: proposal.turnId },
|
||||||
expectedVersion: document.version,
|
);
|
||||||
turnId: proposal.turnId,
|
|
||||||
});
|
|
||||||
if (!ok) {
|
if (!ok) {
|
||||||
void vscode.window.showWarningMessage(
|
void vscode.window.showWarningMessage(
|
||||||
"Cowriting: the editor rejected the accept — the proposal is still pending.",
|
"Cowriting: the editor rejected the accept — the proposal is still pending.",
|
||||||
@@ -181,6 +188,41 @@ export class ProposalController implements vscode.Disposable {
|
|||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* #47 (INV-40): accept a BLOCK proposal — apply Claude's whole block, but
|
||||||
|
* attribute ONLY the runs Claude actually changed. The block is the DECISION
|
||||||
|
* unit; the word is the ATTRIBUTION unit. An intra-block word sub-diff
|
||||||
|
* (`wordEditHunks`, the raw engine INV-37 used, repurposed — un-anchored so the
|
||||||
|
* runs stay disjoint for a batch apply) yields the changed runs; each lands
|
||||||
|
* through the F4 seam (Claude-attributed), applied last-position-first so an
|
||||||
|
* earlier run's offsets stay valid under the later ones. Unchanged spans within
|
||||||
|
* the block are never touched, so their prior authorship stands. (Each run is
|
||||||
|
* one seam edit / one undo step — see the spec's deferred note on undo grouping.)
|
||||||
|
*/
|
||||||
|
private async acceptBlock(
|
||||||
|
document: vscode.TextDocument,
|
||||||
|
resolved: OffsetRange,
|
||||||
|
proposal: Proposal,
|
||||||
|
): Promise<boolean> {
|
||||||
|
const blockText = document.getText(
|
||||||
|
new vscode.Range(document.positionAt(resolved.start), document.positionAt(resolved.end)),
|
||||||
|
);
|
||||||
|
const subHunks = wordEditHunks(blockText, proposal.replacement);
|
||||||
|
if (subHunks.length === 0) return true; // block already equals the proposal — nothing to attribute
|
||||||
|
for (const h of [...subHunks].sort((a, b) => b.start - a.start)) {
|
||||||
|
const range = new vscode.Range(
|
||||||
|
document.positionAt(resolved.start + h.start),
|
||||||
|
document.positionAt(resolved.start + h.end),
|
||||||
|
);
|
||||||
|
const ok = await this.attribution.applyAgentEdit(document, range, h.replacement, proposal.author, {
|
||||||
|
expectedVersion: document.version,
|
||||||
|
turnId: proposal.turnId,
|
||||||
|
});
|
||||||
|
if (!ok) return false;
|
||||||
|
}
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
private reject(state: DocState, proposal: Proposal): void {
|
private reject(state: DocState, proposal: Proposal): void {
|
||||||
if (this.guard.isReadOnly(state.docPath)) return;
|
if (this.guard.isReadOnly(state.docPath)) return;
|
||||||
this.store.update(state.docPath, (a) => removeProposal(a, proposal.id));
|
this.store.update(state.docPath, (a) => removeProposal(a, proposal.id));
|
||||||
|
|||||||
@@ -12,7 +12,7 @@ export function addProposal(
|
|||||||
fp: Fingerprint,
|
fp: Fingerprint,
|
||||||
replacement: string,
|
replacement: string,
|
||||||
author: Provenance,
|
author: Provenance,
|
||||||
opts?: { turnId?: string; instruction?: string },
|
opts?: { turnId?: string; instruction?: string; granularity?: "block" | "single" },
|
||||||
): { proposalId: string; anchorId: string } {
|
): { proposalId: string; anchorId: string } {
|
||||||
const anchorId = newId("a");
|
const anchorId = newId("a");
|
||||||
const proposalId = newId("pr");
|
const proposalId = newId("pr");
|
||||||
@@ -25,6 +25,7 @@ export function addProposal(
|
|||||||
createdAt: new Date().toISOString(),
|
createdAt: new Date().toISOString(),
|
||||||
...(opts?.turnId !== undefined ? { turnId: opts.turnId } : {}),
|
...(opts?.turnId !== undefined ? { turnId: opts.turnId } : {}),
|
||||||
...(opts?.instruction !== undefined ? { instruction: opts.instruction } : {}),
|
...(opts?.instruction !== undefined ? { instruction: opts.instruction } : {}),
|
||||||
|
...(opts?.granularity !== undefined ? { granularity: opts.granularity } : {}),
|
||||||
});
|
});
|
||||||
return { proposalId, anchorId };
|
return { proposalId, anchorId };
|
||||||
}
|
}
|
||||||
|
|||||||
+119
-1
@@ -223,11 +223,27 @@ export interface EditHunk {
|
|||||||
* never part of another hunk.
|
* never part of another hunk.
|
||||||
*/
|
*/
|
||||||
export function diffToHunks(currentText: string, rewrittenText: string): EditHunk[] {
|
export function diffToHunks(currentText: string, rewrittenText: string): EditHunk[] {
|
||||||
|
return wordEditHunks(currentText, rewrittenText).map((h) =>
|
||||||
|
h.start === h.end ? anchorInsertion(h, currentText) : h,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The raw word-level diff underlying `diffToHunks`: disjoint, ordered EditHunks
|
||||||
|
* that exactly partition the change (a pure insertion stays ZERO-WIDTH at its
|
||||||
|
* offset — not anchored). `diffToHunks` adds anchoring on top so each hunk's F4
|
||||||
|
* fingerprint resolves; callers that apply hunks directly through the seam
|
||||||
|
* (INV-40's intra-block accept) want THESE raw, non-overlapping hunks instead —
|
||||||
|
* anchoring can grow an insertion to absorb a token an adjacent hunk also edits,
|
||||||
|
* which corrupts a batch apply. Pure, vscode-free, deterministic. Applying these
|
||||||
|
* right→left reconstructs `rewrittenText` exactly.
|
||||||
|
*/
|
||||||
|
export function wordEditHunks(currentText: string, rewrittenText: string): EditHunk[] {
|
||||||
const hunks: EditHunk[] = [];
|
const hunks: EditHunk[] = [];
|
||||||
let offset = 0;
|
let offset = 0;
|
||||||
let open: EditHunk | null = null;
|
let open: EditHunk | null = null;
|
||||||
const flush = () => {
|
const flush = () => {
|
||||||
if (open) hunks.push(open.start === open.end ? anchorInsertion(open, currentText) : open);
|
if (open) hunks.push(open);
|
||||||
open = null;
|
open = null;
|
||||||
};
|
};
|
||||||
for (const part of diffWordsWithSpace(currentText, rewrittenText)) {
|
for (const part of diffWordsWithSpace(currentText, rewrittenText)) {
|
||||||
@@ -272,6 +288,108 @@ function anchorInsertion(hunk: EditHunk, text: string): EditHunk {
|
|||||||
return { start: s, end: p, replacement: text.slice(s, p) + hunk.replacement };
|
return { start: s, end: p, replacement: text.slice(s, p) + hunk.replacement };
|
||||||
}
|
}
|
||||||
|
|
||||||
|
type BlockAlignKind = "unchanged" | "changed" | "removed" | "added";
|
||||||
|
interface BlockAlignOp {
|
||||||
|
kind: BlockAlignKind;
|
||||||
|
ci?: number; // index into the current blocks
|
||||||
|
ni?: number; // index into the rewritten blocks
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Align two block sequences by normalized key (jsdiff `diffArrays`), pairing an
|
||||||
|
* adjacent removed-then-added run element-wise into `changed` ops (the surplus
|
||||||
|
* staying `removed` / `added`) — the same pairing `diffBlocks` uses, but yielding
|
||||||
|
* index ops so the caller can read each side's source range. Pure.
|
||||||
|
*/
|
||||||
|
function alignBlocks(cur: BlockWithRange[], next: BlockWithRange[]): BlockAlignOp[] {
|
||||||
|
const changes = diffArrays(
|
||||||
|
cur.map((b) => b.key),
|
||||||
|
next.map((b) => b.key),
|
||||||
|
);
|
||||||
|
const ops: BlockAlignOp[] = [];
|
||||||
|
let bi = 0; // current index
|
||||||
|
let ci = 0; // rewritten index
|
||||||
|
for (let n = 0; n < changes.length; n++) {
|
||||||
|
const ch = changes[n];
|
||||||
|
const count = ch.count ?? ch.value.length;
|
||||||
|
if (!ch.added && !ch.removed) {
|
||||||
|
for (let k = 0; k < count; k++) ops.push({ kind: "unchanged", ci: bi++, ni: ci++ });
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
if (ch.removed) {
|
||||||
|
const nx = changes[n + 1];
|
||||||
|
const addCount = nx?.added ? (nx.count ?? nx.value.length) : 0;
|
||||||
|
const paired = Math.min(count, addCount);
|
||||||
|
for (let k = 0; k < paired; k++) ops.push({ kind: "changed", ci: bi++, ni: ci++ });
|
||||||
|
for (let k = paired; k < count; k++) ops.push({ kind: "removed", ci: bi++ });
|
||||||
|
for (let k = paired; k < addCount; k++) ops.push({ kind: "added", ni: ci++ });
|
||||||
|
if (nx?.added) n++;
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
for (let k = 0; k < count; k++) ops.push({ kind: "added", ni: ci++ });
|
||||||
|
}
|
||||||
|
return ops;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* #47 (INV-39, §6.4): diff a whole-document rewrite into ONE EditHunk per CHANGED
|
||||||
|
* BLOCK — the unit a human reviews — rather than per word (which INV-37/`diffToHunks`
|
||||||
|
* did and this supersedes for document edits). Built by aligning both sides into
|
||||||
|
* the existing block units (`splitBlocksWithRanges`) and keying with the same diff
|
||||||
|
* as `diffBlocks`:
|
||||||
|
* - an ISOLATED changed block (1:1, unchanged blocks on both sides) → a
|
||||||
|
* block-aligned hunk `[block.start, block.end)` → the rewritten block's raw
|
||||||
|
* text. A code/mermaid fence is one such whole-block hunk (atomic, INV-23).
|
||||||
|
* - any other changed RUN (block insertions/deletions, or adjacent changes) → one
|
||||||
|
* gap-span hunk covering the source between the bounding unchanged blocks → the
|
||||||
|
* matching rewritten span (separators included), so reconstruction stays exact.
|
||||||
|
* A zero-width gap-span (insert at a seamless boundary) is anchored to an
|
||||||
|
* adjacent token (INV-41) so its F4 fingerprint resolves and it accepts.
|
||||||
|
* - unchanged blocks (same key AND same raw) → no hunk.
|
||||||
|
* Pure, vscode-free, deterministic; same `EditHunk` shape as `diffToHunks` (which
|
||||||
|
* is RETAINED as the intra-block sub-diff engine for word-precise accept
|
||||||
|
* attribution — INV-40). Applying all hunks right→left reconstructs `rewrittenText`
|
||||||
|
* exactly.
|
||||||
|
*/
|
||||||
|
export function diffToBlockHunks(currentText: string, rewrittenText: string): EditHunk[] {
|
||||||
|
const cur = splitBlocksWithRanges(currentText);
|
||||||
|
const next = splitBlocksWithRanges(rewrittenText);
|
||||||
|
const ops = alignBlocks(cur, next);
|
||||||
|
// Same key but different raw (whitespace/case) is still a real content change.
|
||||||
|
for (const op of ops) {
|
||||||
|
if (op.kind === "unchanged" && cur[op.ci!].raw !== next[op.ni!].raw) op.kind = "changed";
|
||||||
|
}
|
||||||
|
const hunks: EditHunk[] = [];
|
||||||
|
let i = 0;
|
||||||
|
while (i < ops.length) {
|
||||||
|
if (ops[i].kind === "unchanged") {
|
||||||
|
i++;
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
let j = i;
|
||||||
|
while (j < ops.length && ops[j].kind !== "unchanged") j++;
|
||||||
|
const run = ops.slice(i, j);
|
||||||
|
if (run.length === 1 && run[0].kind === "changed") {
|
||||||
|
const c = cur[run[0].ci!];
|
||||||
|
hunks.push({ start: c.start, end: c.end, replacement: next[run[0].ni!].raw });
|
||||||
|
} else {
|
||||||
|
// A run with insertions/deletions: replace the whole inter-anchor gap so the
|
||||||
|
// separators reconstruct exactly. Bound by the unchanged blocks on each side
|
||||||
|
// (or the document edges).
|
||||||
|
const prev = i > 0 ? ops[i - 1] : null;
|
||||||
|
const after = j < ops.length ? ops[j] : null;
|
||||||
|
const curStart = prev ? cur[prev.ci!].end : 0;
|
||||||
|
const curEnd = after ? cur[after.ci!].start : currentText.length;
|
||||||
|
const newStart = prev ? next[prev.ni!].end : 0;
|
||||||
|
const newEnd = after ? next[after.ni!].start : rewrittenText.length;
|
||||||
|
const hunk: EditHunk = { start: curStart, end: curEnd, replacement: rewrittenText.slice(newStart, newEnd) };
|
||||||
|
hunks.push(hunk.start === hunk.end ? anchorInsertion(hunk, currentText) : hunk);
|
||||||
|
}
|
||||||
|
i = j;
|
||||||
|
}
|
||||||
|
return hunks;
|
||||||
|
}
|
||||||
|
|
||||||
const md = new MarkdownIt({ html: true, linkify: false, breaks: false });
|
const md = new MarkdownIt({ html: true, linkify: false, breaks: false });
|
||||||
// mermaid fences → <pre class="mermaid">SRC</pre> for client-side rendering; all
|
// mermaid fences → <pre class="mermaid">SRC</pre> for client-side rendering; all
|
||||||
// other fences fall through to markdown-it's default (escaped <pre><code>).
|
// other fences fall through to markdown-it's default (escaped <pre><code>).
|
||||||
|
|||||||
@@ -13,7 +13,7 @@ import * as vscode from "vscode";
|
|||||||
import type { DiffViewController } from "./diffViewController";
|
import type { DiffViewController } from "./diffViewController";
|
||||||
import type { AttributionController } from "./attributionController";
|
import type { AttributionController } from "./attributionController";
|
||||||
import type { ProposalController } from "./proposalController";
|
import type { ProposalController } from "./proposalController";
|
||||||
import { renderReview, renderPlain, diffBlocks, diffToHunks, type BlockOp } from "./trackChangesModel";
|
import { renderReview, renderPlain, diffBlocks, diffToBlockHunks, type BlockOp } from "./trackChangesModel";
|
||||||
import { buildFingerprint } from "./anchorer";
|
import { buildFingerprint } from "./anchorer";
|
||||||
import { isAuthorable } from "./workspacePath";
|
import { isAuthorable } from "./workspacePath";
|
||||||
import type { EditTurnResult } from "./liveTurn";
|
import type { EditTurnResult } from "./liveTurn";
|
||||||
@@ -226,11 +226,12 @@ export class TrackChangesPreviewController implements vscode.Disposable {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* F11 (INV-35/37): run one host edit turn and record the result as F4
|
* F11/F12 (INV-35/39): run one host edit turn and record the result as F4
|
||||||
* proposal(s) — a SELECTION yields one single-range proposal over the resolved
|
* proposal(s) — a SELECTION yields one single-range proposal over the resolved
|
||||||
* block-union; a DOCUMENT rewrite is `diffToHunks`'d into one single-range
|
* block-union; a DOCUMENT rewrite is `diffToBlockHunks`'d into one proposal per
|
||||||
* proposal per changed hunk (reusing the F4 single-range model N times, no new
|
* changed BLOCK (#47, INV-39 supersedes INV-37's per-word cut), each tagged
|
||||||
* model). Never mutates the document (INV-10). Returns the created proposal ids.
|
* `granularity:"block"` so accept reconciles attribution per word (INV-40).
|
||||||
|
* Never mutates the document (INV-10). Returns the created proposal ids.
|
||||||
*/
|
*/
|
||||||
async runEditAndPropose(
|
async runEditAndPropose(
|
||||||
document: vscode.TextDocument,
|
document: vscode.TextDocument,
|
||||||
@@ -253,9 +254,17 @@ export class TrackChangesPreviewController implements vscode.Disposable {
|
|||||||
}
|
}
|
||||||
const turn = await this.editTurn(instruction, full);
|
const turn = await this.editTurn(instruction, full);
|
||||||
const ids: string[] = [];
|
const ids: string[] = [];
|
||||||
for (const h of diffToHunks(full, turn.replacement)) {
|
// #47 (INV-39, supersedes INV-37): a document rewrite is cut at BLOCK
|
||||||
|
// granularity — one proposal per changed block (the unit a human reviews) —
|
||||||
|
// not per word. Each is tagged `granularity:"block"` so accept reconciles
|
||||||
|
// attribution per word inside the block (INV-40).
|
||||||
|
for (const h of diffToBlockHunks(full, turn.replacement)) {
|
||||||
const fp = buildFingerprint(full, { start: h.start, end: h.end });
|
const fp = buildFingerprint(full, { start: h.start, end: h.end });
|
||||||
const id = await this.proposals.propose(document, fp, h.replacement, provenance(turn), { turnId, instruction });
|
const id = await this.proposals.propose(document, fp, h.replacement, provenance(turn), {
|
||||||
|
turnId,
|
||||||
|
instruction,
|
||||||
|
granularity: "block",
|
||||||
|
});
|
||||||
if (id) ids.push(id);
|
if (id) ids.push(id);
|
||||||
}
|
}
|
||||||
return ids;
|
return ids;
|
||||||
|
|||||||
@@ -67,8 +67,10 @@ suite("F11 preview toolbar (host E2E — message → seam wiring, no LLM)", () =
|
|||||||
assert.match(entry!.when ?? "", /editorLangId == markdown/, "guarded on markdown");
|
assert.match(entry!.when ?? "", /editorLangId == markdown/, "guarded on markdown");
|
||||||
});
|
});
|
||||||
|
|
||||||
// SLICE-3: Edit Document → a whole-document rewrite diffed into N F4 proposals.
|
// #47 (was INV-37): Edit Document now cuts at BLOCK granularity — two word
|
||||||
test("runEditAndPropose(document) with a stubbed multi-hunk rewrite → N proposals matching the hunks (PUC-4, INV-37)", async () => {
|
// changes in ONE paragraph are ONE block proposal (INV-39 supersedes INV-37's
|
||||||
|
// per-word cut). Full block coverage lives in f12Review.test.ts.
|
||||||
|
test("runEditAndPropose(document) — two word edits in one paragraph → ONE block proposal (PUC-4, INV-39)", async () => {
|
||||||
const { doc, key } = await freshDoc(
|
const { doc, key } = await freshDoc(
|
||||||
"docs/f11doc.md",
|
"docs/f11doc.md",
|
||||||
"# F11 doc\n\nThe quick brown fox jumps over the lazy dog.\n",
|
"# F11 doc\n\nThe quick brown fox jumps over the lazy dog.\n",
|
||||||
@@ -87,15 +89,16 @@ suite("F11 preview toolbar (host E2E — message → seam wiring, no LLM)", () =
|
|||||||
|
|
||||||
const ids = await ctl.runEditAndPropose(doc, { kind: "document" }, "swap brown→RED and dog→CAT");
|
const ids = await ctl.runEditAndPropose(doc, { kind: "document" }, "swap brown→RED and dog→CAT");
|
||||||
await settle();
|
await settle();
|
||||||
assert.strictEqual(ids.length, 2, "two changed words → two independent proposals");
|
assert.strictEqual(ids.length, 1, "two changed words in one block → ONE block proposal (INV-39)");
|
||||||
|
|
||||||
const views = api.proposalController.listProposals(doc);
|
const views = api.proposalController.listProposals(doc);
|
||||||
assert.ok(
|
const view = views.find((v) => v.id === ids[0]);
|
||||||
ids.every((id) => views.some((v) => v.id === id)),
|
assert.ok(view, "the returned proposal id is a live pending proposal");
|
||||||
"every returned proposal id is a live pending proposal",
|
assert.strictEqual(
|
||||||
|
view!.replacement,
|
||||||
|
"The quick RED fox jumps over the lazy CAT.",
|
||||||
|
"the block proposal carries the whole rewritten paragraph",
|
||||||
);
|
);
|
||||||
const replacements = views.map((v) => v.replacement);
|
|
||||||
assert.ok(replacements.includes("RED") && replacements.includes("CAT"), "proposals carry the per-hunk replacements");
|
|
||||||
// INV-10: proposing never mutates the document.
|
// INV-10: proposing never mutates the document.
|
||||||
assert.ok(doc.getText().includes("brown fox") && doc.getText().includes("lazy dog"), "document unchanged by propose");
|
assert.ok(doc.getText().includes("brown fox") && doc.getText().includes("lazy dog"), "document unchanged by propose");
|
||||||
void key;
|
void key;
|
||||||
|
|||||||
@@ -0,0 +1,134 @@
|
|||||||
|
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<CowritingApi> {
|
||||||
|
const ext = vscode.extensions.getExtension("benstull.vscode-cowriting-plugin")!;
|
||||||
|
const api = (await ext.activate()) as CowritingApi;
|
||||||
|
assert.ok(api?.trackChangesPreviewController && api?.proposalController, "exports controllers");
|
||||||
|
return api;
|
||||||
|
}
|
||||||
|
|
||||||
|
async function freshDoc(rel: string, body: string): Promise<vscode.TextDocument> {
|
||||||
|
const abs = path.join(WS, rel);
|
||||||
|
fs.mkdirSync(path.dirname(abs), { recursive: true });
|
||||||
|
fs.writeFileSync(abs, body, "utf8");
|
||||||
|
const doc = await vscode.workspace.openTextDocument(vscode.Uri.file(abs));
|
||||||
|
await vscode.window.showTextDocument(doc);
|
||||||
|
await settle();
|
||||||
|
return doc;
|
||||||
|
}
|
||||||
|
|
||||||
|
// #47 SLICE-2 (review, P1): a document rewrite proposes ONE F4 proposal per
|
||||||
|
// CHANGED BLOCK (INV-39 supersedes INV-37's per-word cut), but accepting a block
|
||||||
|
// reconciles attribution at WORD granularity (INV-40 — block = decision unit,
|
||||||
|
// word = attribution unit). Host E2E, no LLM (the edit turn is stubbed).
|
||||||
|
suite("F12 SLICE-2 — block-granularity document proposals (#47, INV-39/40/41)", () => {
|
||||||
|
// PUC-4: M changed blocks → M proposals; unchanged blocks → none.
|
||||||
|
test("edits across two paragraphs → two proposals; the untouched paragraph yields none (INV-39)", async () => {
|
||||||
|
const doc = await freshDoc(
|
||||||
|
"docs/f12-multi.md",
|
||||||
|
"# Doc\n\nFirst paragraph alpha.\n\nSecond paragraph beta.\n\nThird paragraph gamma.\n",
|
||||||
|
);
|
||||||
|
const api = await getApi();
|
||||||
|
const ctl = api.trackChangesPreviewController;
|
||||||
|
ctl.setEditTurnForTest(async () => ({
|
||||||
|
replacement: "# Doc\n\nFirst paragraph ALPHA.\n\nSecond paragraph beta.\n\nThird paragraph GAMMA.\n",
|
||||||
|
model: "sonnet",
|
||||||
|
sessionId: "e2e-f12-multi",
|
||||||
|
}));
|
||||||
|
const ids = await ctl.runEditAndPropose(doc, { kind: "document" }, "uppercase the first/last nouns");
|
||||||
|
await settle();
|
||||||
|
assert.strictEqual(ids.length, 2, "two changed blocks → two proposals; the unchanged middle block → none");
|
||||||
|
|
||||||
|
// each proposal's anchor spans a whole block, and the replacement is that block's rewrite
|
||||||
|
const views = api.proposalController.listProposals(doc);
|
||||||
|
const replacements = views.map((v) => v.replacement).sort();
|
||||||
|
assert.deepStrictEqual(replacements, ["First paragraph ALPHA.", "Third paragraph GAMMA."]);
|
||||||
|
});
|
||||||
|
|
||||||
|
// PUC-4 + INV-23: a changed code fence is ONE atomic whole-fence proposal.
|
||||||
|
test("a changed code fence → one atomic proposal over the whole fence (INV-23)", async () => {
|
||||||
|
const doc = await freshDoc(
|
||||||
|
"docs/f12-fence.md",
|
||||||
|
"# Code\n\n```js\nconst a = 1;\nconst b = 2;\n```\n",
|
||||||
|
);
|
||||||
|
const api = await getApi();
|
||||||
|
const ctl = api.trackChangesPreviewController;
|
||||||
|
ctl.setEditTurnForTest(async () => ({
|
||||||
|
replacement: "# Code\n\n```js\nconst a = 10;\nconst b = 2;\n```\n",
|
||||||
|
model: "sonnet",
|
||||||
|
sessionId: "e2e-f12-fence",
|
||||||
|
}));
|
||||||
|
const ids = await ctl.runEditAndPropose(doc, { kind: "document" }, "bump a to 10");
|
||||||
|
await settle();
|
||||||
|
assert.strictEqual(ids.length, 1, "a changed fence is one atomic proposal");
|
||||||
|
const view = api.proposalController.listProposals(doc).find((v) => v.id === ids[0])!;
|
||||||
|
assert.strictEqual(view.replacement, "```js\nconst a = 10;\nconst b = 2;\n```", "whole-fence replacement");
|
||||||
|
});
|
||||||
|
|
||||||
|
// PUC-5 / INV-40: accepting a block attributes ONLY the words Claude changed —
|
||||||
|
// unchanged words in the block are NOT swept into Claude's authorship.
|
||||||
|
test("accepting a block proposal attributes only the changed words to Claude (INV-40)", async () => {
|
||||||
|
const doc = await freshDoc("docs/f12-attr.md", "# T\n\nThe quick brown fox jumps lazily.\n");
|
||||||
|
const api = await getApi();
|
||||||
|
const ctl = api.trackChangesPreviewController;
|
||||||
|
const key = api.proposalController.keyFor(doc);
|
||||||
|
|
||||||
|
ctl.setEditTurnForTest(async () => ({
|
||||||
|
replacement: "# T\n\nThe quick RED fox jumps SLOWLY.\n",
|
||||||
|
model: "sonnet",
|
||||||
|
sessionId: "e2e-f12-attr",
|
||||||
|
}));
|
||||||
|
const ids = await ctl.runEditAndPropose(doc, { kind: "document" }, "change two words");
|
||||||
|
await settle();
|
||||||
|
assert.strictEqual(ids.length, 1, "one changed paragraph → one block proposal");
|
||||||
|
|
||||||
|
const ok = await api.proposalController.acceptById(key, ids[0]);
|
||||||
|
assert.ok(ok, "the block proposal accepts");
|
||||||
|
await settle();
|
||||||
|
|
||||||
|
assert.strictEqual(doc.getText(), "# T\n\nThe quick RED fox jumps SLOWLY.\n", "the whole block landed");
|
||||||
|
|
||||||
|
const agentTexts = api.attributionController
|
||||||
|
.getSpans(key)
|
||||||
|
.filter((s) => s.authorKind === "agent")
|
||||||
|
.map((s) => doc.getText().slice(s.range.start, s.range.end).trim())
|
||||||
|
.filter((t) => t.length > 0);
|
||||||
|
// Only the two words Claude actually changed are Claude-attributed.
|
||||||
|
const joined = agentTexts.join(" ");
|
||||||
|
assert.ok(joined.includes("RED"), "the changed word RED is Claude-attributed");
|
||||||
|
assert.ok(joined.includes("SLOWLY"), "the changed word SLOWLY is Claude-attributed");
|
||||||
|
assert.ok(!/\bquick\b/.test(joined), "the unchanged word 'quick' is NOT swept into Claude's authorship");
|
||||||
|
assert.ok(!/\bfox\b/.test(joined), "the unchanged word 'fox' is NOT swept into Claude's authorship");
|
||||||
|
});
|
||||||
|
|
||||||
|
// PUC-4 / INV-41: a block-insertion rewrite produces acceptable proposals and
|
||||||
|
// accepting them all reconstructs the intended document (no born-orphaned hunk).
|
||||||
|
test("a rewrite that inserts a paragraph → acceptable proposals; accept-all reaches the rewrite (INV-41)", async () => {
|
||||||
|
const original = "# Ins\n\nAlpha block.\n\nBeta block.\n";
|
||||||
|
const rewrite = "# Ins\n\nAlpha block.\n\nBrand new middle block.\n\nBeta block.\n";
|
||||||
|
const doc = await freshDoc("docs/f12-insert.md", original);
|
||||||
|
const api = await getApi();
|
||||||
|
const ctl = api.trackChangesPreviewController;
|
||||||
|
const key = api.proposalController.keyFor(doc);
|
||||||
|
|
||||||
|
ctl.setEditTurnForTest(async () => ({ replacement: rewrite, model: "sonnet", sessionId: "e2e-f12-insert" }));
|
||||||
|
const ids = await ctl.runEditAndPropose(doc, { kind: "document" }, "insert a paragraph");
|
||||||
|
await settle();
|
||||||
|
assert.ok(ids.length >= 1, "the insertion produced at least one proposal");
|
||||||
|
|
||||||
|
for (const id of ids) {
|
||||||
|
const ok = await api.proposalController.acceptById(key, id);
|
||||||
|
assert.ok(ok, `proposal ${id} is acceptable (not born-orphaned, INV-41)`);
|
||||||
|
await settle();
|
||||||
|
}
|
||||||
|
assert.strictEqual(doc.getText(), rewrite, "accepting all proposals reconstructs the intended rewrite");
|
||||||
|
assert.strictEqual(api.proposalController.listProposals(doc).length, 0, "no proposals left pending");
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -1,5 +1,5 @@
|
|||||||
import { describe, it, test, expect } from "vitest";
|
import { describe, it, test, expect } from "vitest";
|
||||||
import { splitBlocks, splitBlocksWithRanges, diffBlocks, diffToHunks, renderTrackChanges, colorByAuthor, type AuthorSpan } from "../src/trackChangesModel";
|
import { splitBlocks, splitBlocksWithRanges, diffBlocks, diffToHunks, diffToBlockHunks, renderTrackChanges, colorByAuthor, type AuthorSpan } from "../src/trackChangesModel";
|
||||||
|
|
||||||
describe("splitBlocks", () => {
|
describe("splitBlocks", () => {
|
||||||
it("splits prose paragraphs on blank lines, dropping empties", () => {
|
it("splits prose paragraphs on blank lines, dropping empties", () => {
|
||||||
@@ -402,6 +402,90 @@ describe("F11 diffToHunks (INV-37)", () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// #47 SLICE-2 (INV-39): a whole-document rewrite is diffed into ONE EditHunk per
|
||||||
|
// CHANGED BLOCK (the unit a human reviews), superseding INV-37's per-word hunks.
|
||||||
|
// Built by coarsening diffToHunks to block boundaries; fences stay atomic
|
||||||
|
// (INV-23); inserted/deleted blocks keep their anchored gap hunks (INV-41).
|
||||||
|
// Applying all hunks right→left must still reconstruct the rewrite exactly.
|
||||||
|
describe("#47 diffToBlockHunks (INV-39/41)", () => {
|
||||||
|
const applyHunks = (current: string, hunks: ReturnType<typeof diffToBlockHunks>): 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("an identical rewrite → zero hunks", () => {
|
||||||
|
expect(diffToBlockHunks("# H\n\nSame body.\n", "# H\n\nSame body.\n")).toEqual([]);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("two word edits in ONE paragraph → ONE block hunk (supersedes INV-37's two)", () => {
|
||||||
|
const current = "# Doc\n\nThe quick brown fox jumps over the lazy dog.\n";
|
||||||
|
const rewrite = "# Doc\n\nThe quick RED fox jumps over the lazy CAT.\n";
|
||||||
|
const hunks = diffToBlockHunks(current, rewrite);
|
||||||
|
expect(hunks).toHaveLength(1);
|
||||||
|
// the hunk spans the whole changed paragraph block
|
||||||
|
const para = "The quick brown fox jumps over the lazy dog.";
|
||||||
|
const start = current.indexOf(para);
|
||||||
|
expect(hunks[0].start).toBe(start);
|
||||||
|
expect(hunks[0].end).toBe(start + para.length);
|
||||||
|
expect(hunks[0].replacement).toBe("The quick RED fox jumps over the lazy CAT.");
|
||||||
|
expect(applyHunks(current, hunks)).toBe(rewrite);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("edits across two paragraphs → two block hunks, one per changed block; unchanged → none", () => {
|
||||||
|
const current = "First para alpha.\n\nSecond para beta.\n\nThird para gamma.\n";
|
||||||
|
const rewrite = "First para ALPHA.\n\nSecond para beta.\n\nThird para GAMMA.\n";
|
||||||
|
const hunks = diffToBlockHunks(current, rewrite);
|
||||||
|
expect(hunks).toHaveLength(2);
|
||||||
|
// each hunk lands on a real block boundary in current
|
||||||
|
const blocks = splitBlocksWithRanges(current);
|
||||||
|
for (const h of hunks) {
|
||||||
|
expect(blocks.some((b) => b.start === h.start && b.end === h.end)).toBe(true);
|
||||||
|
}
|
||||||
|
expect(applyHunks(current, hunks)).toBe(rewrite);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("a changed code fence → ONE atomic whole-fence hunk (INV-23)", () => {
|
||||||
|
const current = "# Code\n\n```js\nconst a = 1;\nconst b = 2;\n```\n";
|
||||||
|
const rewrite = "# Code\n\n```js\nconst a = 10;\nconst b = 2;\n```\n";
|
||||||
|
const hunks = diffToBlockHunks(current, rewrite);
|
||||||
|
expect(hunks).toHaveLength(1);
|
||||||
|
const fence = "```js\nconst a = 1;\nconst b = 2;\n```";
|
||||||
|
const start = current.indexOf(fence);
|
||||||
|
expect(hunks[0].start).toBe(start);
|
||||||
|
expect(hunks[0].end).toBe(start + fence.length);
|
||||||
|
expect(hunks[0].replacement).toBe("```js\nconst a = 10;\nconst b = 2;\n```");
|
||||||
|
expect(applyHunks(current, hunks)).toBe(rewrite);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("every hunk is resolvable (non-zero-width, real source text) and reconstructs", () => {
|
||||||
|
const cases: Array<[string, string]> = [
|
||||||
|
["# H\n\nThe brown fox sleeps.\n", "# H\n\nThe brown fox QUIETLY sleeps today.\n"], // insert words mid-block
|
||||||
|
["A para.\n\nB para.\n\nC para.\n", "A para.\n\nC para.\n"], // delete a whole block
|
||||||
|
["A para.\n\nB para.\n", "A para.\n\nNEW para.\n\nB para.\n"], // insert a whole block
|
||||||
|
["Only one block here.\n", "A totally different single block.\n"], // wholesale
|
||||||
|
["Keep me.\n\nDrop this one.\n", "Keep me.\n"], // delete trailing block
|
||||||
|
["unchanged body\n", "unchanged body\n"], // no-op
|
||||||
|
];
|
||||||
|
for (const [current, rewrite] of cases) {
|
||||||
|
const hunks = diffToBlockHunks(current, rewrite);
|
||||||
|
for (const h of hunks) {
|
||||||
|
expect(h.end).toBeGreaterThan(h.start); // non-zero-width → F4 fp resolves
|
||||||
|
expect(current.slice(h.start, h.end).length).toBeGreaterThan(0);
|
||||||
|
}
|
||||||
|
expect(applyHunks(current, hunks)).toBe(rewrite);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
test("is deterministic — same inputs → identical hunks", () => {
|
||||||
|
const c = "P one.\n\nP two.\n";
|
||||||
|
const r = "P ONE.\n\nP two.\n";
|
||||||
|
expect(diffToBlockHunks(c, r)).toEqual(diffToBlockHunks(c, r));
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
// F11 SLICE-2 (INV-36): the pure render layer emits data-src-start/data-src-end
|
// F11 SLICE-2 (INV-36): the pure render layer emits data-src-start/data-src-end
|
||||||
// (source char offsets from BlockWithRange) on every LIVE-source rendered block,
|
// (source char offsets from BlockWithRange) on every LIVE-source rendered block,
|
||||||
// in BOTH modes. The webview's selection→source mapping walks the DOM to the
|
// in BOTH modes. The webview's selection→source mapping walks the DOM to the
|
||||||
|
|||||||
Reference in New Issue
Block a user