F12: inline editable proposed-change diff in the Markdown editor + Accept/Reject control parity (#64) (#66)
This commit was merged in pull request #66.
This commit is contained in:
@@ -0,0 +1,445 @@
|
||||
---
|
||||
status: graduated
|
||||
---
|
||||
# Solution Design: Inline editable proposed-change diff in the Markdown editor (#64)
|
||||
|
||||
| | |
|
||||
| --- | --- |
|
||||
| **Author(s)** | Ben Stull (with Claude) |
|
||||
| **Reviewers / approvers** | Ben Stull |
|
||||
| **Status** | `graduated` |
|
||||
| **Version** | v0.1.1 |
|
||||
| **Source artifacts** | Feature `benstull/vscode-cowriting-plugin#64` (Inline editable proposed-change diff in the Markdown editor + Accept/Reject controls in both surfaces, `type/feature`, `priority/P2`) · Brainstorming session `vscode-cowriting-plugin-0058` (2026-06-26) · Builds on (all shipped): F4 `#12` (propose/accept seam, INV-9/10/11/13), F6 `#17` (baseline / machine-landing), F7 `#21` (rendered preview, pure render engine INV-22), F10 `#29` (interactive review preview — the **clean-editor** decision INV-32; ✓/✗ accept-reject), F11 `#43` (preview toolbar, `data-src` block→source mapping INV-36), document-edit-flow `#42/#47/#46` (block proposals INV-39/40, accept-all INV-42) · Parent specs (graduated): `coauthoring-propose-accept.md`, `coauthoring-diff-view.md`, `coauthoring-rendered-preview.md`, `coauthoring-interactive-review.md`, `coauthoring-document-edit-flow.md` · Lineage: `ben.stull/rfc-app#48` |
|
||||
|
||||
**Change log**
|
||||
|
||||
| Date | Version | Change | By |
|
||||
| --- | --- | --- | --- |
|
||||
| 2026-06-26 | v0.1.1 | Implementation refinement (planning-and-executing session 0059): made the **re-anchor** explicit. Optimistic apply (§3.2) must store the pre-apply text on a new `Proposal.original` field **and** re-fingerprint the proposal's anchor to the *applied* text — F4's `fp.text` is the *original* target, which leaves the buffer once the replacement lands, so `resolve()` would orphan every applied proposal otherwise. `finalizeInPlace`/`revertInPlace`/decorate all key off the re-anchored fp; revert restores `original`. Reload-safety (INV-51/54): a proposal already carrying `original` is never re-captured (the in-memory applied-set is empty after a window reload), so the revert target survives save+reload. Shipped #64 (PR, session 0059). |
|
||||
| 2026-06-26 | v0.1.0 | Initial draft — brainstorming session 0058. Four forks locked with the operator: **(1) editor model** = *optimistic apply + decorations* — on propose, the editor buffer becomes the would-be-accepted text (insertions real & editable & tinted; deletions shown as struck-red non-editable hints); accept = finalize-in-place, reject = revert. **(2) timing** = *on proposal* (turn complete), not a live token-stream into the editor (that stays in #60's notification/OutputChannel). **(3) editor affordance** = CodeLens `Accept ▾ / Reject ▾` above each block, `▾` → QuickPick (this / all); the webview keeps HTML buttons with the same dropdown. **(4) controls parity** = Accept / Reject / Accept-all / Reject-all reachable from **both** the editor and the webview. Two sub-decisions confirmed: a **dedicated `EditorProposalController`** owns optimistic-apply + decorations + CodeLens (keeps `ProposalController` the pure F4 state/seam owner); **saving while pending persists** the proposed (accepted-result) text. Reverses INV-32 and INV-10; supersedes the ✓/✗ glyph controls. | Ben Stull + Claude |
|
||||
|
||||
---
|
||||
|
||||
## 1. Business Context
|
||||
|
||||
### 1.1 Executive Summary
|
||||
|
||||
Today, when Claude proposes an edit, the change is shown **only in the rendered
|
||||
review webview** (`<del>old</del><ins>new</ins>` with ✓/✗ controls); the Markdown
|
||||
**editor stays deliberately clean** (F10 / INV-32 — "the preview is the single
|
||||
review surface"). The writer who lives in the editor sees nothing: to review what
|
||||
Claude proposed they must open the preview panel, and they cannot *edit* the
|
||||
proposed text in place — they can only accept or reject it wholesale in the
|
||||
webview.
|
||||
|
||||
This design brings the proposed change **into the Markdown editor itself**:
|
||||
editable, with a track-changes diff that matches the webview and the post-accept
|
||||
result **exactly**. It also replaces the per-proposal `✓`/`✗` glyphs with labelled
|
||||
**Accept** / **Reject** controls — each carrying a `▾` dropdown for **Accept all**
|
||||
/ **Reject all** — and makes all four actions reachable from **both** the editor
|
||||
and the webview. The writer can now read, tweak, and resolve a proposal without
|
||||
leaving the document.
|
||||
|
||||
### 1.2 Background
|
||||
|
||||
The plugin's review model (graduated specs `coauthoring-propose-accept.md`,
|
||||
`coauthoring-rendered-preview.md`, `coauthoring-interactive-review.md`):
|
||||
|
||||
- A machine edit becomes an **F4 proposal** — a pending-only record
|
||||
(`Proposal{ id, anchorId, replacement, granularity }` + a `Fingerprint` anchor
|
||||
whose `text` is the exact target) that **never mutates the document** (INV-10),
|
||||
re-resolved against the live text at accept (INV-11) and cleared when
|
||||
accepted/rejected (INV-13).
|
||||
- The **render engine** (`trackChangesModel.ts`) is a pure, deterministic,
|
||||
`vscode`-free unit (INV-22): it diffs at **block** granularity (INV-39), word-
|
||||
merges prose blocks, and emits webview HTML — `renderReview` (annotated) /
|
||||
`renderPlain` (clean) — with proposal blocks (`proposalBlockHtml`) carrying the
|
||||
✓/✗ buttons and `data-src` offsets for selection mapping (INV-36).
|
||||
- The **F10 decision (INV-32)** removed *all* in-editor decorations
|
||||
(`grep` confirms zero `TextEditorDecorationType` / `setDecorations` in the tree)
|
||||
so the webview is the sole review surface; `ProposalController` carries the note
|
||||
*"no in-editor UI — INV-32 makes the rendered preview the single review surface."*
|
||||
- The sealed webview is **intent-only** (INV-35): it posts `{accept|reject|
|
||||
acceptAll|…}` to the host, which routes to `ProposalController.acceptById` /
|
||||
`rejectById` / `acceptAllProposals` (#46 / INV-42). Accept **applies** the
|
||||
replacement via the F4 seam (`applyAgentEdit`), which advances the F6 baseline
|
||||
(machine-landing, INV-18).
|
||||
|
||||
This feature **deliberately reverses two of those decisions** for the editor
|
||||
surface, and supersedes the ✓/✗ glyph controls in both surfaces.
|
||||
|
||||
### 1.3 Who feels it & why
|
||||
|
||||
The **human coauthor working in the editor** — the writer who asked Claude to edit
|
||||
and wants to see, adjust, and resolve the result without context-switching to the
|
||||
preview panel. The pain: the proposed change is invisible in the place they are
|
||||
actually writing, and it is not editable at all (only accept/reject-able). P2
|
||||
(issue #64): it materially deepens the inner-loop "edit-in-place" experience and
|
||||
unifies the review controls, but the webview review path already works, so it is
|
||||
an enhancement rather than a gap.
|
||||
|
||||
---
|
||||
|
||||
## 2. Product Design
|
||||
|
||||
### 2.1 The experience
|
||||
|
||||
When Claude's turn lands a proposal (or, for a document edit, N block proposals),
|
||||
the **editor** shows the change as track-changes, in place:
|
||||
|
||||
```
|
||||
Accept ▾ Reject ▾ ← CodeLens, above each proposed block
|
||||
The quick ~~brown~~ red fox jumps over the lazy dog.
|
||||
└ struck red ┘└ green/blue, editable ┘
|
||||
```
|
||||
|
||||
- **Insertions** are **real, editable buffer text**, tinted by origin
|
||||
(green = human-origin, blue = LLM) — the writer can click in and edit them like
|
||||
any other text.
|
||||
- **Deletions** appear as a **struck-red, non-editable hint** rendered adjacent to
|
||||
the insertion (a decoration, not buffer text) — visually mirroring the webview's
|
||||
`<del>…</del>`.
|
||||
- The buffer content **is exactly the would-be-accepted result**, so what the
|
||||
writer sees (and can edit) is precisely what accepting produces.
|
||||
|
||||
Above each proposed block sit two **CodeLens** actions — **`Accept ▾`** and
|
||||
**`Reject ▾`**. Clicking opens a small QuickPick:
|
||||
|
||||
```
|
||||
Accept ▾ → ┌───────────────────────┐ Reject ▾ → ┌───────────────────────┐
|
||||
│ Accept this proposal │ │ Reject this proposal │
|
||||
│ Accept ALL proposals │ │ Reject ALL proposals │
|
||||
└───────────────────────┘ └───────────────────────┘
|
||||
```
|
||||
|
||||
The **webview** shows the same proposal with the same diff, and its controls are
|
||||
the parallel HTML buttons — **`Accept ▾`** / **`Reject ▾`** — where the `▾`
|
||||
reveals *Accept all* / *Reject all*. The legacy ✓/✗ glyphs are **replaced** by
|
||||
these labelled controls in both surfaces.
|
||||
|
||||
All four actions — **Accept**, **Reject**, **Accept all**, **Reject all** — are
|
||||
reachable from **either** surface and route to the same controller logic
|
||||
(INV-53).
|
||||
|
||||
### 2.2 Accept / Reject semantics (editable-in-place)
|
||||
|
||||
- **Accept** finalizes the change *that is already in the buffer*: it records the
|
||||
span's attribution as landed, advances the F6 baseline (machine-landing), and
|
||||
clears the proposal. It does **not** re-apply text (the text is already there) —
|
||||
see §3.3.
|
||||
- **Reject** reverts that block's region back to the stored original
|
||||
(`Fingerprint.text`), and clears the proposal.
|
||||
- **If the writer edited the inserted text before deciding:** **Accept** keeps
|
||||
their edited text (their keystrokes layer on as human authorship over the
|
||||
proposed span); **Reject** still reverts the whole block to the original.
|
||||
- **Accept all / Reject all** operate on the current document's pending proposals,
|
||||
in descending anchor order, skipping orphans (the #46 / INV-42 shape; **Reject
|
||||
all is new**).
|
||||
|
||||
### 2.3 Save semantics
|
||||
|
||||
Because the proposed text is *in the buffer*, **saving while a proposal is pending
|
||||
persists the proposed (accepted-result) text** to disk. The deletion hints are
|
||||
decorations (never buffer text), so the **saved file is clean** — it contains the
|
||||
accepted-result text, not yet "finalized" only in the sense of attribution /
|
||||
baseline. "Pending" therefore means *provisional attribution, baseline not yet
|
||||
advanced* — **not** "document unchanged" (INV-54). We deliberately **allow** the
|
||||
save rather than block it or strip on save; once the editor shows the live
|
||||
document, blocking saves would be the surprising behavior.
|
||||
|
||||
### 2.4 What this is *not* (non-goals)
|
||||
|
||||
- **Not live token-streaming into the editor.** The diff appears when the turn
|
||||
produces a proposal; watching Claude type into the document is out of scope (the
|
||||
live token stream is #60's notification + OutputChannel).
|
||||
- **Not a new review *model*.** F4 proposals, anchoring (INV-11), block
|
||||
granularity (INV-39/40), accept-all (INV-42) are reused; this adds an editor
|
||||
*surface* and unifies the controls.
|
||||
- **Not non-Markdown.** Same Markdown-gated scope as the rest of the review UI.
|
||||
- **Not intra-diagram mermaid editing.** Atomic fences stay atomic (INV-23); a
|
||||
mermaid block proposal is editable as its whole fence, not sub-diagram.
|
||||
- **Not removing the webview.** The rendered preview remains a full review surface;
|
||||
it gains label/dropdown parity, not a demotion.
|
||||
|
||||
### 2.5 Surfaces considered & rejected
|
||||
|
||||
| Editor approach | Why not |
|
||||
| --- | --- |
|
||||
| **Read-only decoration overlay** (doc unchanged; ghost insertions via decoration) | The inserted text would **not be editable** — fails the core "human-editable" requirement. |
|
||||
| **Inline track-changes markup in the buffer** (`{~~old~~|++new++}`) | Pollutes the file on disk with markup until resolved; complex; conflicts with clean save. |
|
||||
| **Native `vscode.diff` two-pane** | The two-pane diff was *removed* in #34 (F10) precisely to make one review surface; a separate diff editor is not "in the Markdown file." |
|
||||
|
||||
Chosen: **optimistic apply + decorations** — the only model that yields an
|
||||
editable diff that is *exactly* the accepted result, in the document itself.
|
||||
|
||||
---
|
||||
|
||||
## 3. Engineering Design
|
||||
|
||||
### 3.1 Architecture
|
||||
|
||||
```
|
||||
┌──────────────────────────────────────────────┐
|
||||
one F4 turn ───▶ │ ProposalController (F4 state/seam owner) │
|
||||
(#12 propose) │ propose() · finalizeInPlace() · revertInPlace│
|
||||
│ · rejectAll() · onDidChangeProposals │
|
||||
└───────────────┬──────────────────────────────┘
|
||||
│ proposals (pending records + anchors)
|
||||
┌─────────────────────┴───────────────────────┐
|
||||
▼ ▼
|
||||
┌───────────────────────────┐ ┌────────────────────────────┐
|
||||
│ EditorProposalController │ shared │ TrackChangesPreview- │
|
||||
│ (NEW, vscode host) │ pure diff │ Controller (webview) │
|
||||
│ · optimistic buffer write │◀──────────────▶ │ · renderReview HTML │
|
||||
│ · insertion tint deco │ trackChangesModel│ · Accept▾/Reject▾ buttons │
|
||||
│ · deletion-hint deco │ (diff → plan) │ · dropdown → acceptAll/ │
|
||||
│ · CodeLens Accept▾/Reject▾│ │ rejectAll │
|
||||
└───────────────────────────┘ └────────────────────────────┘
|
||||
▲ ▲
|
||||
└──────────── both route actions to ───────────┘
|
||||
ProposalController.{finalizeInPlace,revertInPlace,
|
||||
acceptAll,rejectAll}
|
||||
```
|
||||
|
||||
The decisive rule: **one pure diff is the single source of truth** for both
|
||||
surfaces (INV-49). The editor renders it as *buffer text + decorations + CodeLens*;
|
||||
the webview renders it as *HTML*. They cannot diverge because they consume the
|
||||
same hunks.
|
||||
|
||||
### 3.2 Optimistic apply (on propose) — INV-48
|
||||
|
||||
`propose()` gains a buffer-write step, owned by `EditorProposalController` (so
|
||||
`ProposalController` stays the pure state owner):
|
||||
|
||||
1. Record the pending proposal(s) + anchor(s) **exactly as today** (the sidecar
|
||||
record is unchanged: `replacement`, `Fingerprint{text,before,after,lineHint}`,
|
||||
`granularity`).
|
||||
2. Write the proposed text into the **live editor buffer**: replace the resolved
|
||||
anchor span with `replacement` (for a document turn, this makes the buffer
|
||||
Claude's full proposed document; for a selection turn, the single span). This
|
||||
write **does not advance the F6 baseline** — the baseline stays at the
|
||||
pre-proposal text, so the change reads as *pending*, not *landed*.
|
||||
3. Compute the **decoration plan** from the same per-block word/line diff used by
|
||||
`acceptBlock` (`wordEditHunks(fp.text, replacement)`): insertion segments →
|
||||
tinted ranges (buffer coords, offset by the live anchor); deletion segments →
|
||||
struck-red hint injections (decoration `before`/`after` content) at the segment
|
||||
boundary.
|
||||
|
||||
Critically, the optimistic write must **not** fire the F4 seam's
|
||||
`onDidApplyAgentEdit` (which would advance the baseline / register a landed edit).
|
||||
Optimistic apply is a *distinct* buffer-mutation path from accept; only **accept**
|
||||
advances the baseline (§3.3). This is the load-bearing distinction that keeps the
|
||||
change *pending* while *present*.
|
||||
|
||||
### 3.3 Accept = finalize-in-place; Reject = revert-in-place — INV-51
|
||||
|
||||
Because the proposed text is already in the buffer, the old "accept ⇒ apply via
|
||||
seam" path would **double-apply**. The accept/reject paths are reworked:
|
||||
|
||||
- **`finalizeInPlace(id)`** — resolve the proposal's anchor against the live text;
|
||||
record the span's attribution as **landed** (provenance from the proposal's
|
||||
`author`, word-precise per INV-40 for block granularity); **advance the F6
|
||||
baseline** for that region (the machine-landing the seam used to do on apply);
|
||||
`removeProposal(id)`. No text is re-applied. If the writer edited the inserted
|
||||
span, the *current* buffer text is what gets finalized (their edits become
|
||||
layered human authorship).
|
||||
- **`revertInPlace(id)`** — resolve the proposal's live span; replace it with the
|
||||
stored original `Fingerprint.text`; `removeProposal(id)`. Reverts the whole
|
||||
block regardless of any in-place edits.
|
||||
- **`acceptAll()` / `rejectAll()`** — iterate the current document's pending
|
||||
proposals in **descending** anchor order (so earlier resolutions don't shift
|
||||
later offsets), skipping orphans with a tally (INV-42 shape). `acceptAll` reuses
|
||||
the #46 batch ordering; **`rejectAll` is new** and symmetric.
|
||||
|
||||
Both surfaces (editor CodeLens/QuickPick and webview buttons/dropdown) call these
|
||||
**same four methods** (INV-53). The webview's existing intent messages are renamed/
|
||||
extended (`accept`/`reject`/`acceptAll` + new `rejectAll`) but stay intent-only
|
||||
(INV-35).
|
||||
|
||||
### 3.4 Single diff source & render-once — INV-49 / INV-50
|
||||
|
||||
- **INV-49 (single diff source).** The per-proposal diff (block hunks → word/line
|
||||
edit hunks) is produced by one pure function in `trackChangesModel.ts` (extends
|
||||
the existing `diffToBlockHunks` / `wordEditHunks`, INV-39/40). The webview HTML
|
||||
path and the editor decoration-plan path both consume it. Same proposal ⇒
|
||||
identical diff in both surfaces (this is the testable parity guarantee).
|
||||
- **INV-50 (render once).** With optimistic apply, the baseline→buffer delta now
|
||||
*contains* the proposed change. The renderer must attribute any baseline→buffer
|
||||
delta that is covered by a **pending proposal** to that proposal (rendered as a
|
||||
proposal block), and must **not** also render it as an already-landed
|
||||
machine-diff. Mechanism: the webview render reconciles baseline-diff spans
|
||||
against pending-proposal anchors; a span covered by a pending proposal is
|
||||
rendered exactly once, as the proposal. *(This reconciliation is the primary
|
||||
implementation risk; the implementation plan owns the exact span-mapping, but the
|
||||
invariant — rendered once — is fixed here.)*
|
||||
|
||||
### 3.5 Editor decorations & CodeLens (the editor surface)
|
||||
|
||||
`EditorProposalController` (new, `vscode` host module) owns:
|
||||
|
||||
- **Two `TextEditorDecorationType`s** — an *insertion tint* (green/blue background,
|
||||
by provenance) over real buffer ranges, and a *deletion hint* (a
|
||||
`before`/`after` render-option carrying the struck original text, red,
|
||||
strikethrough, non-editable). Both are recomputed from the decoration plan on
|
||||
every `onDidChangeProposals` and on active-editor change.
|
||||
- **A `CodeLensProvider`** — for each pending proposal whose anchor resolves in the
|
||||
active document, emit two lenses positioned above the block: `Accept ▾`
|
||||
(command `cowriting.proposalAcceptMenu` with the proposal id) and `Reject ▾`
|
||||
(`cowriting.proposalRejectMenu`). Each command opens a `vscode.window
|
||||
.showQuickPick(["… this proposal","… ALL proposals"])` and dispatches to
|
||||
`finalizeInPlace`/`acceptAll` or `revertInPlace`/`rejectAll`.
|
||||
- **Lifecycle wiring** — subscribes to `proposals.onDidChangeProposals` (the same
|
||||
event the preview uses) and `window.onDidChangeActiveTextEditor`; clears its
|
||||
decorations + lenses for documents with no pending proposals (so a clean editor
|
||||
stays clean — INV-32's spirit holds *when there is nothing pending*).
|
||||
|
||||
Deletion hints are **decoration-only** (never buffer text, never saved) and
|
||||
insertion tints decorate **real** buffer text (INV-52).
|
||||
|
||||
### 3.6 Webview controls (label + dropdown parity)
|
||||
|
||||
`proposalBlockHtml` (`trackChangesModel.ts`) swaps the ✓/✗ glyph buttons for an
|
||||
**`Accept ▾`** / **`Reject ▾`** control pair. The `▾` opens a small CSS/JS dropdown
|
||||
in the sealed webview (`preview.ts`/`preview.css`, no network — INV-21) offering
|
||||
*Accept all* / *Reject all*; selecting posts the corresponding intent
|
||||
(`acceptAll` / `rejectAll`) or the per-proposal `accept`/`reject`. The host routes
|
||||
all four to the controller methods of §3.3 (INV-35 preserved).
|
||||
|
||||
### 3.7 Components & files
|
||||
|
||||
| File | Change |
|
||||
| --- | --- |
|
||||
| `src/editorProposalController.ts` | **NEW** — optimistic apply, decoration types, deletion-hint injection, `CodeLensProvider`, QuickPick menus; subscribes to `onDidChangeProposals` + active-editor change. |
|
||||
| `src/proposalController.ts` | Add `finalizeInPlace(id)`, `revertInPlace(id)`, `rejectAll(docKey)`; `propose()` delegates the optimistic buffer write (so the controller stays state-pure); accept/reject routing now finalizes/reverts in place (no seam re-apply). |
|
||||
| `src/trackChangesModel.ts` | Factor the shared per-proposal diff into one pure producer feeding both surfaces; reconcile pending-proposal spans in `renderReview` (INV-50); swap ✓/✗ → `Accept ▾`/`Reject ▾` in `proposalBlockHtml`. |
|
||||
| `src/trackChangesPreview.ts` | Route new `rejectAll` intent; keep `accept`/`reject`/`acceptAll`; relabel toolbar/proposal controls. |
|
||||
| `src/preview.ts` / `preview.css` | Dropdown control for `Accept ▾`/`Reject ▾`; same diff CSS reused. |
|
||||
| `src/extension.ts` | Register `EditorProposalController`, its CodeLens provider, and the menu commands; wire it alongside `ProposalController`/`TrackChangesPreviewController`. |
|
||||
| `package.json` | Add `cowriting.rejectAllProposals`, `cowriting.proposalAcceptMenu`, `cowriting.proposalRejectMenu`; CodeLens contribution; markdown-gated `when` clauses. |
|
||||
|
||||
### 3.8 Invariants
|
||||
|
||||
- **INV-48 — Optimistic apply (with re-anchor).** On propose, the proposed text is
|
||||
written into the live editor buffer (the buffer becomes the would-be-accepted
|
||||
result); this write does **not** advance the F6 baseline and does **not** fire the
|
||||
F4 machine-landing seam. Because F4's `fp.text` is the *original* target — which
|
||||
leaves the buffer once the replacement lands — optimistic apply **stores the
|
||||
pre-apply text on `Proposal.original` and re-fingerprints the anchor to the applied
|
||||
text**, so `resolve()` keeps finding the proposal in the mutated buffer; finalize /
|
||||
revert / decorate all key off the re-anchored fp, and revert restores `original`.
|
||||
`original` is captured **exactly once** (first apply): a proposal that survives a
|
||||
save+reload already carries it, so a fresh session never re-captures it from the
|
||||
already-applied buffer (reload-safety, INV-51/54). **Reverses INV-10** (propose no
|
||||
longer leaves the document untouched) and **INV-32** (the editor is no longer kept
|
||||
unconditionally clean — it shows pending proposals).
|
||||
- **INV-49 — Single diff source.** Editor decorations and webview HTML both derive
|
||||
from one pure, deterministic per-proposal diff (extends `diffToBlockHunks` /
|
||||
`wordEditHunks`, INV-22/39/40). The same proposal yields the same diff in both
|
||||
surfaces.
|
||||
- **INV-50 — Rendered once.** A span covered by a pending proposal is rendered
|
||||
exactly once — as that proposal — never additionally as an already-landed
|
||||
baseline diff.
|
||||
- **INV-51 — Finalize / revert in place.** Accept finalizes the span already in the
|
||||
buffer (record attribution, advance baseline, clear proposal) without
|
||||
re-applying text; Reject reverts the block region to the stored
|
||||
`Fingerprint.text`. No double-apply.
|
||||
- **INV-52 — Decoration roles.** Deletion hints are decoration-only (non-editable
|
||||
`before`/`after` content, never buffer text, never saved); insertion tints
|
||||
decorate real, editable buffer text.
|
||||
- **INV-53 — Control parity.** Accept / Reject / Accept-all / Reject-all are
|
||||
reachable from **both** the editor (CodeLens + QuickPick) and the webview
|
||||
(buttons + dropdown) and route to the same `ProposalController` methods; the ✓/✗
|
||||
glyph controls are superseded by labelled `Accept ▾`/`Reject ▾`.
|
||||
- **INV-54 — Save persists pending.** Saving while a proposal is pending persists
|
||||
the proposed (accepted-result) text; "pending" denotes provisional attribution /
|
||||
un-advanced baseline, not document divergence. (Decorations are not persisted, so
|
||||
the saved file is clean text.)
|
||||
|
||||
### 3.9 Error & edge handling
|
||||
|
||||
- **Orphaned proposal** (anchor no longer resolves) — no editor decorations/lenses
|
||||
for it; it still surfaces in the webview at end with a dashed border (INV-34);
|
||||
accept-all/reject-all skip it with a tally (INV-42).
|
||||
- **Writer edits the inserted span, then accepts** — current buffer text is
|
||||
finalized (their edits layer as human authorship); **then rejects** — block
|
||||
reverts to original regardless.
|
||||
- **External / concurrent edit shifts geometry** — anchors re-resolve on
|
||||
`onDidChangeProposals` (existing resolve-or-flag); decoration plan + lenses
|
||||
recompute.
|
||||
- **Multiple proposals from one turn** — all optimistically applied (buffer = full
|
||||
proposed document); each block independently finalizable/revertible; descending
|
||||
order on accept-all/reject-all keeps offsets valid.
|
||||
- **Reject of an in-buffer span after partial accept of siblings** — each
|
||||
proposal's revert uses its own live-resolved span, independent of siblings.
|
||||
- **Non-authorable document** (read-only / not on disk) — controls disabled exactly
|
||||
as today (`authorable` gating).
|
||||
|
||||
---
|
||||
|
||||
## 4. Testing & E2E
|
||||
|
||||
Per the §9 pipeline and `coauthoring-*` precedent (this is a VS Code extension —
|
||||
**no flotilla/PPE**; the gate is unit + host-E2E green).
|
||||
|
||||
- **Unit (the core):**
|
||||
- The shared per-proposal diff producer: same hunks → insertion/deletion
|
||||
segments; parity assertion that the decoration plan and the webview HTML derive
|
||||
identical add/del spans for the same proposal (INV-49).
|
||||
- `renderReview` render-once reconciliation: a pending-proposal span is emitted
|
||||
as a proposal block and **not** as a landed baseline diff (INV-50).
|
||||
- Decoration-plan computation from `wordEditHunks(fp.text, replacement)`:
|
||||
insertion ranges in buffer coords; deletion-hint positions (INV-52).
|
||||
- **Host E2E:**
|
||||
- Propose (via the injectable `editTurn` stub) → editor shows insertion tint +
|
||||
deletion hint + `Accept ▾`/`Reject ▾` CodeLens; buffer equals the accepted
|
||||
result (INV-48).
|
||||
- Edit the inserted text → **finalizeInPlace** keeps the edited text; baseline
|
||||
advanced; proposal cleared (INV-51).
|
||||
- **revertInPlace** restores `Fingerprint.text`; proposal cleared.
|
||||
- Accept / Reject / Accept-all / Reject-all from **both** the editor command path
|
||||
and the webview intent path produce the same end state (INV-53); `rejectAll`
|
||||
clears all pending and reverts all blocks.
|
||||
- Save while a proposal is pending → file on disk has the proposed text; no
|
||||
decoration markup in the saved bytes (INV-54).
|
||||
- Webview/editor parity: same proposal renders the same diff in both (INV-49).
|
||||
- **Manual smoke:** ask Claude to edit a paragraph; confirm the editor diff is
|
||||
editable, the deletion hint reads correctly, both surfaces' controls resolve it,
|
||||
and a clean editor returns once nothing is pending.
|
||||
|
||||
---
|
||||
|
||||
## 5. Delivery Plan (rollout — not a task list)
|
||||
|
||||
One increment, single design-then-build (#64). Ships through branch → PR → `main`;
|
||||
no migration and no new persisted sidecar shape (the `Proposal` record is
|
||||
unchanged). The implementation plan (downstream `wgl-planning-and-executing`
|
||||
session) owns the Task breakdown; a natural cut:
|
||||
|
||||
1. **Shared diff + render-once** — factor the pure per-proposal diff producer;
|
||||
reconcile pending spans in `renderReview` (INV-49/50) + unit tests.
|
||||
2. **Accept/reject rework** — `finalizeInPlace` / `revertInPlace` / `rejectAll` on
|
||||
`ProposalController` (no seam re-apply); unit tests.
|
||||
3. **Editor surface** — `EditorProposalController`: optimistic apply, decorations,
|
||||
deletion hints, CodeLens + QuickPick menus; wire in `extension.ts`.
|
||||
4. **Control parity** — relabel ✓/✗ → `Accept ▾`/`Reject ▾` + dropdown in the
|
||||
webview; route `rejectAll`; `package.json` commands/`when`.
|
||||
5. **Host E2E + manual smoke** across both surfaces.
|
||||
|
||||
Reversible by reverting the PR (restores the clean-editor / ✓-✗ webview behavior).
|
||||
|
||||
---
|
||||
|
||||
## 6. Open Questions
|
||||
|
||||
- **OQ-1 — Provenance tint exactness.** Insertion tint uses green=human-origin /
|
||||
blue=LLM by the proposal `author`; whether to additionally word-tint *within* a
|
||||
block by the F3 attribution of edited-in-place text (vs a single block tint) can
|
||||
follow the existing F9/F10 coloring; v1 tints the proposed insertion by the
|
||||
proposal author. *(Not blocking.)*
|
||||
- **OQ-2 (inherited)** — The F11 design (`#43`) remains un-graduated (lives in code
|
||||
+ an issue draft); unrelated to #64 but noted in the shared lineage. Track
|
||||
separately.
|
||||
- **OQ-3 — Editor dropdown fidelity.** The editor uses a QuickPick to stand in for
|
||||
the webview's true `▾` dropdown (CodeLens cannot render a caret menu inline); if
|
||||
a more button-like affordance is wanted later, a webview-style overlay is a
|
||||
possible increment. *(Accepted for v1.)*
|
||||
Reference in New Issue
Block a user