From 886bbf5512cc687531f1ac87978f2f209157c3e6 Mon Sep 17 00:00:00 2001 From: Ben Stull Date: Mon, 25 May 2026 11:00:45 -0700 Subject: [PATCH] =?UTF-8?q?Contribute=20rewrite=20Phase=204:=20=C2=A78.10?= =?UTF-8?q?=20gutter=20accent=20in=20CM6=20raw=20pane?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Restores the §8.10 paragraph-margin marker layer Phase 1 dropped when Tiptap left, this time against the CM6 raw pane on the left half of the Contribute split. The matching inline tracked-insert/tracked- delete overlay Phase 3 shipped lives on the preview pane to the right; the two visual layers answer different questions on the two halves of the split — "did anything change in this region?" (gutter, amber, scannable) vs. "what changed here?" (inline, green/red, precise) — and are deliberately separate signals. New file: frontend/src/components/diffGutterExtension.js. The extension exposes a `setBaseline` StateEffect and a gutter that marks every line whose text differs from the same-indexed line of the baseline (the last server-confirmed branch body). Per-line, not paragraph-block — CM6's natural unit; collapsing to paragraph ranges is more spec-faithful but adds code, and the per-line stance is the pre-fancy default. A TODO is left for a future paragraph-collapse pass if the result reads noisy. MarkdownSourceEditor.jsx changes: - Install the gutter extension in the editor's extension list. - Seed the baseline to `initialDoc` at construct time. - In the existing `initialDoc`-watching effect, dispatch the doc replacement AND a `setBaseline` effect in the SAME transaction so there's no one-frame window where the new doc reads as "divergent" against the old baseline. This carries through every server- confirmed branch refresh that RFCView already wires (accept, decline, manual flush, branch switch); no RFCView changes needed because all four paths already re-push `initialDoc` after pulling fresh state. Design calls per the Phase 4 prompt's open list: • Per-line marks, not paragraph-block ranges. Pre-fancy stance. • Amber (#f59e0b) thin 3px vertical bar, distinct from the preview pane's green-on-light / red-on-light tracked-change inline overlay. Reads as "in-flight / not yet on the server." • Baseline reset on every branchView refresh (accept / decline / manual flush / branch switch), matching RFCView's existing originalSourceLinesRef discipline. Gutter then reads as "what's in the buffer but not on the server." • No hover / no click. The inline overlay already carries the click-to-card binding; the gutter is scannable only. Known caveats kept deliberately: • Insert/delete shifts. Adding a line in the middle shifts every subsequent line's index and the naive compare lights up everything below — tolerated as the honest "you've touched stuff below this point" cue. A future LCS-anchored pass could fix it; Phase 4 doesn't need to. SPEC §8.10: - First paragraph rewritten to name the CM6 raw pane as the gutter's home and replace the stale "ProseMirror plugin" wording with the CodeMirror gutter framing. Mirrors how Phase 3 named the preview pane as the inline overlay's home. - DiffView retirement paragraph adjusted: gutter and inline overlay together (across the split) cover its review affordance, not "both layers in the same surface" — the layers are deliberately on different surfaces. Verification: - Vite preview sandbox eval — standalone CM6 mount, dispatch tests across construct (no marks on identity), per-line edit (mark on exactly the touched line, confirmed by Y-coordinate matching the line-number gutter element), baseline reset clears, atomic doc+baseline dispatch leaves zero marks (the RFCView accept-flow path), insert-in-middle exhibits the expected cascade, and computed-style proof for the amber bar (`#f59e0b`, 3px wide, positioned left of the line-numbers gutter at x=21 vs x=24). - Screenshot captures the bar on the touched line only. - 125 backend integration tests still green. - Live RFCView accept → branch refresh → gutter clear flow against a real branch was not driven (backend not running this session, carrying forward Phase 2/3's verification gap). The sandbox-level proof covers the atomic dispatch correctness; the next session with a backend up should drive that golden path before piling Phase 5 work on top. Co-Authored-By: Claude Opus 4.7 (1M context) --- SPEC.md | 25 ++--- frontend/src/App.css | 17 ++++ .../src/components/MarkdownSourceEditor.jsx | 11 +++ .../src/components/diffGutterExtension.js | 98 +++++++++++++++++++ 4 files changed, 139 insertions(+), 12 deletions(-) create mode 100644 frontend/src/components/diffGutterExtension.js diff --git a/SPEC.md b/SPEC.md index fc3d3c5..8886be7 100644 --- a/SPEC.md +++ b/SPEC.md @@ -829,18 +829,20 @@ declined predecessor still visible. ### 8.10 Tracked-change markup and the review-mode toggle -Two visual layers carry change information in the editor. The first -is a paragraph-margin marker — a thin gutter accent on any paragraph -that differs from the branch's open-session baseline, rendered by a -ProseMirror plugin against a baseline snapshot taken when the editor -opens. The second is inline `tracked-delete` / `tracked-insert` -markup at the exact range of an accepted change, with the deleted -text shown struck-through and the inserted text shown in an additive +Two visual layers carry change information in the Contribute split. +The first is a paragraph-margin marker on the left raw-source pane — +a thin gutter accent on any line that differs from the branch's +open-session baseline, rendered by a CodeMirror gutter extension +against a baseline reset on every server-confirmed branch refresh +(accept, decline, manual flush, branch switch). The second is inline +`tracked-delete` / `tracked-insert` markup on the right preview pane +at the exact range of an accepted change, with the deleted text +shown struck-through and the inserted text shown in an additive style; both carry the change's ID via a data attribute, enabling the click-to-card binding from §8.8. The margin marker is scannable ("did anything change in this region?"); the inline markup is precise ("what changed here?"). The two answer different questions -and both are kept. +on the two halves of the split and both are kept. The inline markup lives in the rendered preview surface, not on the writable editor. With the Contribute-mode split (§8.3) the raw markdown @@ -865,10 +867,9 @@ selection-quote that drove the change, and the AI's `reason`. DiffView is the legacy read-only render surface invoked via the Contribute-mode toolbar toggle (§8.15) — a full-editor swap-in that reads the same `changes` table. It is retained as an interim path while -the preview-pane overlay matures and is slated for retirement once -gutter markers (the §8.10 paragraph-margin layer) land in the preview -itself; at that point both visual layers live in the same surface and -the toolbar toggle collapses. +the split-pane layers mature, and is slated for retirement once +the raw-pane gutter and the preview-pane inline overlay together cover +its review affordance; at that point the toolbar toggle collapses. ### 8.11 Manual edits and collisions with AI proposals diff --git a/frontend/src/App.css b/frontend/src/App.css index f40a9a8..4227b82 100644 --- a/frontend/src/App.css +++ b/frontend/src/App.css @@ -580,6 +580,23 @@ .cm-source-editor .cm-activeLineGutter { background: #f3f4f6; color: #555; } .cm-source-editor .cm-activeLine { background: #fafbfc; } +/* §8.10 paragraph-margin marker (Phase 4) — gutter accent on lines + that differ from the open-session baseline (last server-confirmed + body). A scannable "did anything change in this region?" cue, + visually distinct from the preview pane's green/red tracked-insert + / tracked-delete overlay. Amber signals in-flight / uncommitted. */ +.cm-source-editor .cm-diff-gutter { + width: 3px; padding: 0; + background: transparent; +} +.cm-source-editor .cm-diff-gutter .cm-gutterElement { + padding: 0; +} +.cm-source-editor .cm-diff-gutter-mark { + width: 3px; height: 100%; + background: #f59e0b; +} + .readonly-bar { border-top: 1px solid #e5e5e5; padding: 10px 16px; text-align: center; diff --git a/frontend/src/components/MarkdownSourceEditor.jsx b/frontend/src/components/MarkdownSourceEditor.jsx index 6052a0e..b93a94b 100644 --- a/frontend/src/components/MarkdownSourceEditor.jsx +++ b/frontend/src/components/MarkdownSourceEditor.jsx @@ -16,6 +16,7 @@ import { EditorView, keymap, lineNumbers, highlightActiveLine, highlightActiveLi import { defaultKeymap, history, historyKeymap, indentWithTab } from '@codemirror/commands' import { markdown } from '@codemirror/lang-markdown' import { syntaxHighlighting, defaultHighlightStyle, indentOnInput, bracketMatching } from '@codemirror/language' +import { diffGutterExtension, setBaseline } from './diffGutterExtension.js' export default function MarkdownSourceEditor({ initialDoc = '', @@ -34,6 +35,7 @@ export default function MarkdownSourceEditor({ doc: initialDoc, extensions: [ history(), + diffGutterExtension(), lineNumbers(), highlightActiveLine(), highlightActiveLineGutter(), @@ -51,6 +53,11 @@ export default function MarkdownSourceEditor({ ], }) const view = new EditorView({ state, parent: hostRef.current }) + // Seed the §8.10 gutter baseline to the initial doc — nothing + // diverges at construct time. Subsequent baseline resets ride the + // initialDoc-watching effect below so they dispatch atomically + // with the doc replacement. + view.dispatch({ effects: setBaseline.of(initialDoc ?? '') }) viewRef.current = view if (editorRef) { editorRef.current = { @@ -75,12 +82,16 @@ export default function MarkdownSourceEditor({ }, []) // Reflect external doc changes (branch reload, server-side flush, etc). + // The §8.10 gutter baseline is reset in the same dispatch so the + // newly-pushed doc doesn't briefly read as "divergent" against the + // old baseline. useEffect(() => { const v = viewRef.current if (!v) return if (v.state.doc.toString() === initialDoc) return v.dispatch({ changes: { from: 0, to: v.state.doc.length, insert: initialDoc ?? '' }, + effects: setBaseline.of(initialDoc ?? ''), }) }, [initialDoc]) diff --git a/frontend/src/components/diffGutterExtension.js b/frontend/src/components/diffGutterExtension.js new file mode 100644 index 0000000..8d9edb8 --- /dev/null +++ b/frontend/src/components/diffGutterExtension.js @@ -0,0 +1,98 @@ +// diffGutterExtension.js — Phase 4 of the Contribute rewrite. Restores +// §8.10's paragraph-margin gutter accent against the CM6 raw pane in +// the Contribute split, the surface where editing happens. The matching +// inline tracked-change overlay (Phase 3) lives in the preview pane on +// the right; the two visual layers answer different questions — +// "did anything change in this region?" (gutter) vs. "what changed +// here?" (inline) — and live on the two halves of the split for that +// reason. Discuss mode has no editor, so no gutter. +// +// Surface model +// ------------- +// CM6's natural unit is the line, not the paragraph, so the marker is +// per-line: every line whose text differs from the same-indexed line of +// the open-session baseline gets a thin amber bar in its own gutter +// (sat left of the line-numbers gutter). The spec talks +// "paragraph-margin marker"; collapsing line-marks back to +// paragraph-block ranges is more spec-faithful but adds code and risks +// hiding small inline changes inside large paragraphs. Per-line is the +// pre-fancy stance — a future pass can collapse if it reads noisy. +// +// Baseline reset +// -------------- +// Baseline = the last server-confirmed branch body. Initialized to +// `initialDoc` at editor construct; re-set via the `setBaseline` effect +// on accept / decline / manual flush / branch switch — i.e. every time +// the parent re-pushes `initialDoc`. The marker thus reads as +// "what's in the buffer but not on the server." +// +// The reset must dispatch atomically with the doc replacement — +// otherwise there's a one-frame window where the new doc differs from +// the old baseline and every line lights up. MarkdownSourceEditor's +// initialDoc effect bundles both into a single dispatch. +// +// Caveats kept deliberately +// ------------------------- +// Insert/delete shifts. Adding a line in the middle shifts every +// subsequent line's index and the naive index-compare lights up +// everything below the insertion. We tolerate it: the gutter is a +// scannable cue, not a precision diff, and "you've touched stuff below +// this point" remains honest. A future pass against a proper LCS diff +// could anchor the marks to stable lines, but Phase 4 doesn't need it. + +import { StateEffect, StateField } from '@codemirror/state' +import { GutterMarker, gutter } from '@codemirror/view' + +export const setBaseline = StateEffect.define() + +class DiffMarker extends GutterMarker { + toDOM() { + const el = document.createElement('div') + el.className = 'cm-diff-gutter-mark' + return el + } +} + +const diffMarker = new DiffMarker() + +// Baseline doc as a frozen line array. We split once on reset rather +// than on every gutter query — the doc itself can change far more +// often than the baseline. +const baselineField = StateField.define({ + create: () => [], + update(value, tr) { + for (const e of tr.effects) { + if (e.is(setBaseline)) return splitLines(e.value || '') + } + return value + }, +}) + +function splitLines(text) { + return (text ?? '').split('\n') +} + +export function diffGutterExtension() { + return [ + baselineField, + gutter({ + class: 'cm-diff-gutter', + lineMarker(view, line) { + const baseline = view.state.field(baselineField, false) + if (!baseline || baseline.length === 0) return null + const lineNumber = view.state.doc.lineAt(line.from).number + const baselineLine = baseline[lineNumber - 1] + const currentLine = view.state.doc.lineAt(line.from).text + if (baselineLine === undefined) return diffMarker + return currentLine === baselineLine ? null : diffMarker + }, + lineMarkerChange(update) { + if (update.docChanged) return true + for (const tr of update.transactions) { + for (const e of tr.effects) if (e.is(setBaseline)) return true + } + return false + }, + }), + ] +}