feat(files-changed): editable CM6 diff view + in-place updates + lock chip - #300
Open
jeonghun-jj-lee wants to merge 25 commits into
Open
feat(files-changed): editable CM6 diff view + in-place updates + lock chip#300jeonghun-jj-lee wants to merge 25 commits into
jeonghun-jj-lee wants to merge 25 commits into
Conversation
- New editable-diff-view-core.ts: language loader, theme builder,
createDiffEditor factory (split via MergeView, unified via
unifiedMergeView fallback)
- New editable-diff-view.tsx: SolidJS component wrapper
- 20 tests covering loadLanguage, buildThemeExtension, baseExtensions,
createDiffEditor (split + unified), readOnly, onChange, revert, destroy
- CM6 deps: @codemirror/{state,view,merge,language} + lang-* grammars
- readOnly enforced via transactionFilter (blocks programmatic dispatch too)
- Theme maps app CSS custom properties to CM6 selectors (light/dark)
Part of #766
- New diff-gutter-extension.ts: CM6 gutter with added (green), removed (red), changed (yellow) markers via StateField + ViewPlugin - Uses 'diff' npm package (diffLines) for line-level diffing - 300ms debounce after edits for performance on large files - Replaces unifiedMergeView fallback in createDiffEditor unified mode - 9 tests: marker kinds, debounced updates, empty/identical content - All 180 session-ui tests pass Part of #766
…772) Apply overlay changes to the fork so the binary build picks them up: - Replace <Dynamic> with EditableDiffView for A/M files in review panel - Debounced auto-save (1s) + Cmd/Ctrl+S via /file/write - Save indicator (yellow/green/red) in file header - Revert button + concurrent edit detection banner (Reload/Keep) - Markdown preview toggle (eye icon, .md files only) - Widen ReviewDiffStyle to include "preview" Part of #766
Three visual fixes for the EditableDiffView component: 1. Dark mode colors: rewrite buildThemeExtension to use the app v2 design tokens (--v2-background-bg-base, --v2-text-text-base, etc.) and add buildSyntaxHighlightStyle mapping lezer tags to --syntax-* CSS vars so highlighting matches the user selected theme (all 38 themes). Replaces non-existent --amc-* variables + defaultHighlightStyle. 2. Line wrapping: add EditorView.lineWrapping to baseExtensions so long lines wrap instead of requiring horizontal scroll. 3. Vertical scroll: set mergeView.dom.style.height = "100%" directly on the DOM (EditorView.theme selectors can t target MergeView.dom because it is a PARENT of .cm-editor, not a descendant). Change container overflow from hidden to auto. Same for unified mode editorView.dom. Also update diff-gutter-extension to use --v2-state-fg-* tokens. 4 new tests (lineWrapping, split scroll, unified scroll, syntax style). All 184 session-ui tests pass.
Unified mode now shows removed lines as faded strikethrough text with a red-tinted background, inserted between the editor real lines via CM6 widget decorations (DeletedLinesWidget). Each widget renders the deleted lines with a minus-sign prefix, line-through text-decoration, and 0.7 opacity — similar to GitHub unified diff. Also soften the full-line diff backgrounds in both modes: the v2 state-bg tokens are designed for small badges, so mix them down to 30-60% opacity via color-mix() for full-line tints that do not obscure text. 5 new tests for deleted-line decorations (export, state entries, DOM widgets, multi-line). All 189 session-ui tests pass.
…geView Replace the custom diff-gutter + deleted-line-widget implementation in unified mode with @codemirror/merge unifiedMergeView — the standard CM6 inline diff that interleaves deleted lines with the editable document. Trade-off: cursor navigates through deleted text, but the rendering is more polished (native line styling, gutter alignment, syntax highlighting on deleted text). The custom diff-gutter-extension remains available as a module but is no longer wired into the editor. diff-gutter-extension tests still pass (the module is intact). New test verifies deleted lines appear in the unified view DOM. All 190 tests pass.
Set mergeControls: false on unifiedMergeView — the accept/reject chunk buttons ship enabled by default and are not wanted in our editing context. Also explicitly enable gutter, highlightChanges, and syntaxHighlightDeletions.
Reset session.tsx and review-panel-v2.tsx to local/amicode versions, then add serverUrl. Restores: - keepPreviousData for diff query flicker prevention - serverResponded trust logic for empty server responses - watcherDebounce for file watcher diff_version bump (#743) - onRefresh in reviewPanelV2Props (refresh button) Plus our addition: serverUrl getter for auto-save.
The write tool computed a diff but never packaged it as a filediff object in its metadata — unlike the edit tool which did. This meant: - The Files Changed panel never saw files created/overwritten by the write tool - The event-reducer never bumped diff_version (no filediff → no refetch) - The server snapshot diff excluded write-tool files from its agentFiles set Now the write tool: 1. Computes additions/deletions via diffLines (same as edit.ts) 2. Pushes filediff via ctx.metadata() mid-execution for live UI updates 3. Includes filediff + diff in the completion return metadata 3 new tests: overwrite filediff, new-file filediff, mid-execution push.
… in Files Changed The cross-project filter assumed that any tool-metadata diff absent from the server set was cross-project. But in-project files the server correctly excluded (net-zero diff: created then deleted) also leaked through. Now mergeServerAndToolDiffs checks whether the file path starts with the project prefix — in-project files trust the server's authority; only genuinely cross-project files pass through from tool metadata. Also extracts the merge logic into a tested pure function (mergeServerAndToolDiffs in accumulate-diffs.ts) instead of inlining it in session.tsx's createMemo. 6 new tests for the merge function.
When a file is moved or renamed via the sidebar, its tool-metadata diff still records the old path. The server correctly excludes the old path (net zero), and the new location may be cross-project (invisible to the server). Result: the file vanished from Files Changed. Now session.tsx listens for file-op-notify messages from the extension, maintains a rename map, and applies it to tool-metadata diffs via the new applyRenames() pure function before merging with server diffs. The renamed file appears at its new location in Files Changed. 5 new tests for applyRenames.
The browser iframe has no process.env.HOME, so toHomePath couldn't normalize absolute paths to ~/... form. The rename map keys stayed absolute while tool diffs were normalized — no match, file vanished. Now the extension sends os.homedir() in the file-op-notify message and session.tsx uses it for normalization.
When a file is moved cross-project then moved back, the rename map resolves to the original in-project path. But the server still has a stale response (no diff for that file). The cross-project filter drops it (in-project files trust the server). Result: the file vanishes. Now the file-op-notify handler bumps diff_version immediately after updating the rename map, forcing a server refetch. The fresh server response includes the file at its current location. 2 new tests documenting the round-trip move scenario.
…browser) The listener used d.home from the extension (producing ~/... keys) while the memo used globalThis.process.env.HOME (undefined in browser, producing absolute paths). Rename map keys never matched tool diff paths. Both now use the same home source — undefined in the browser, so both produce absolute paths. Consistent normalization = keys match.
The file-picker dropdown overflowed the side panel because it used a fixed max-width: 400px with position: absolute. Items used width: 100% so overflow-x: auto never fired. - Cap dropdown max-width to min(400px, calc(100vw - 16px)) - Add scroll-inner wrapper (flex column, width: max-content, min-width: 100%) so all items align to the widest one and trigger horizontal scroll - Remove width: 100% from items (flex-column stretch handles alignment) - Remove overflow: hidden / text-overflow: ellipsis from root label so it contributes full width to the scrollable area Test: new case in file-name-picker.test.tsx verifies the scroll-inner wrapper is present and contains the filePicker content.
…roll The previous flex-column + width: max-content approach didn't produce actual overflow in WebKit — the block-level flex container resolved to the dropdown's width, leaving nothing to scroll. Switch to a plain block div with min-width: max-content — this reliably expands the wrapper to the widest child, triggering overflow-x: auto. Also add overscroll-behavior: contain on the dropdown to keep the trackpad gesture from leaking to parent containers.
The dropdown used left: 0 + max-width: min(400px, calc(100vw - 16px)), but 100vw in a VS Code WebView is the full window, not the side panel. The dropdown rendered at full width and bled past the panel edge — no overflow inside, so nothing to scroll. Pin both edges (left: 0; right: 0) to the wrapper, which has flex: 1 inside the file header and IS panel-constrained. The dropdown is now exactly the wrapper's width; the scroll-inner (min-width: max-content) produces real overflow when file paths are wider, and overflow-x: auto provides the horizontal scrollbar.
The dropdown bled past the side panel edge because 100vw in a VS Code WebView is the full window, not the panel, and left: 0; right: 0 made it too narrow (pinned to the wrapper width only). Instead, measure the dropdown's actual left offset on mount via a ref callback + requestAnimationFrame, then set max-width to the remaining panel space (window.innerWidth - left - 8px, floor 200px). The dropdown shrink-wraps to its content up to this cap; wider content scrolls horizontally via the scroll-inner (min-width: max-content). Test: new async case mocks getBoundingClientRect and window.innerWidth, verifies max-width is computed correctly.
The 3-way toggle in Files Changed used Icon name="eye" but the v2 Icon component had no eye entry — it silently fell back to plus. Copy the SVG from the v1 Icon (20x20 viewBox, stroke eye + circle pupil).
- Add `lock` icon to the v2 Icon map (16x16 stroke padlock)
- Thread `isAgentBusy` (from session_working) through the prop chain:
session.tsx → ReviewPanelV2 → SessionReviewFilePreviewV2
- Wire `readOnly={!!props.isAgentBusy}` on EditableDiffView — the editor
is read-only while the agent is churning, preventing the infuriating
'edits lost to diff refresh' problem
- Render a lock indicator chip (solid bg, bordered, design-system tokens)
in the upper-right corner of the diff view via a zero-height overlay;
fades in on mount, removed from DOM when agent finishes
- Add `scrollDOM` accessor to DiffEditorHandle — returns the scrollable
element for both split (mergeView.dom) and unified (editorView.scrollDOM)
- Preserve scroll position across CM6 teardown/recreate cycles: save
scrollTop before destroy, restore via requestAnimationFrame after create
- TDD: 3 new tests for scrollDOM accessor (split, unified, post-destroy)
- Move lock indicator from top-right to top-left (right→left in CSS) - Fix scroll jumping to top on diff refresh: the CM6-level scrollDOM save/restore was at the wrong level — the actual visible scroll happens on the parent session-review-v2-diff-scroll container. Add onScroll listener on scrollRef to continuously track position, and a createEffect on view() to restore after diff data changes. Reset saved position on file switch so new files start at top.
…tent or readOnly changes Replace the destroy/recreate createEffect in EditableDiffView with three separate effects that update the CM6 editor in place: 1. Creation effect — fires only on diffStyle or language changes (structural) 2. Content effect — dispatches minimal changesets via updateOriginal/updateModified 3. ReadOnly effect — reconfigures a Compartment via setReadOnly Key changes in editable-diff-view-core.ts: - Add minimalChanges() — common-prefix/suffix scan for focused change descriptions that preserve CM6 scroll mapping (TDD: 10 tests) - Add updateOriginal/updateModified to DiffEditorHandle — in-place document updates via MergeView.a/b.dispatch (split) and originalDocChangeEffect (unified), with Transaction.addToHistory.of(false) (TDD: 15 tests) - Add setReadOnly via Compartment.reconfigure — toggles editable/readOnly/ onChange extensions without editor teardown (TDD: 6 tests) - Extract editableExtensions() from baseExtensions — mutable extensions live in a Compartment, structural extensions stay flat - Remove custom transactionFilter that blocked programmatic dispatches when readOnly (CM6 built-in readOnly + editable facets are sufficient) Result: agent edits stream into the diff view without any DOM teardown, scroll displacement, or visible flicker. The editor is created once and lives for the lifetime of the file selection. 61 fork-side tests pass (was 30). 2252 amicode tests pass.
Move lock indicator from top-right (right: 12px) to top-center (left: 50%; transform: translateX(-50%)). Pure CSS, zero JS — no DOM coupling to CM6 gutter width.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CM6-powered editable diff view for Files Changed with in-place update architecture.
Core: EditableDiffView component (split/unified), minimalChanges dispatch, Compartment-based readOnly toggle, lock chip. 61 tests.
Companion: harmoniqs/amicode#774