[diffs/edit] AI attempt at edit with collapsed regions#970
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@codex review |
While an editor is attached to a FileDiff, hunk updates now preserve the current hunk structure instead of recomputing it per keystroke: each hunk is a frozen region, edits re-diff only the region they land in, a fully reverted region persists as a context-only hunk, edits spanning regions merge them, and edits inside an unchanged gap synthesize a new region. The real recompute runs once on genuine session exit, so post-session state is unchanged. This is groundwork for edit mode preserving collapsed unchanged regions; with expandUnchanged still forced during editing, rendering behavior is unchanged. Details: - New pure module `utils/editSessionHunks.ts` holds the region math (window mapping, region re-diff with zero context, expansion-key remapping, exit recompute) built on the exported updateDiffHunks helpers. - Line-count edit passes run updateRenderCache with dense, stale indexed dirty lines before applyDocumentChange. A new explicit `lineCountChangeInFlight` flag threads from the editor through the host so the session path defers hunk work to the document rebuild, which scans a per-session line snapshot instead of the contaminated live array. Deferred background-tokenize passes keep running region updates from their (current) changed indexes. - A same-line-count gap edit changes the rendered row set, which the debounced refresh cannot express; the host escalates to a deferred full re-render, and VirtualizedFileDiff also invalidates its layout caches. - The editor detach closure now reports whether it is a virtualized recycle or a genuine session end. Session hunks and a dirty marker on the shared FileDiffMetadata survive recycling; CodeView finishes sessions whose instance was already released, and FileDiff.render self-heals session-shaped metadata reused after a teardown.
Prepare the editor for diffs whose collapsed unchanged regions stay collapsed during editing: rendered rows can then be a sparse subset of document lines, which several editor code paths assumed were dense. With expansion still forced during editing, behavior is unchanged. - New visibility oracle: `isAdditionLineRenderable` in virtualDiffLayout derives from hunks plus expansion state (the same inputs as iterateOverDiff), exposed to the editor as the optional `DiffsEditableComponent.isLineRenderable`. FileDiff implements it; plain files leave it unset and every line stays renderable. - `#rerender` filters dirty lines through the oracle, scans the row patch loop from child index 0 (a sparse row can sit at a smaller index than its dense position, and overshooting appended duplicate rows), and only creates new rows beyond the last rendered row. - `#applyChange` only widens the render window toward a caret whose line can actually gain a row; `#scrollToLine` refuses to park its retry state on a collapsed line, which previously became a non-terminating scroll loop against separator geometry. - `__syncRenderView` stamps hunk separator rows contenteditable=false in both diff styles, alongside annotation and deleted rows. - `FileDiff.syncRenderViewToEditor` converts the stored render range (rendered-row units for the windowed AST pipeline) into the document-line units the editor consumes; the two only coincide while everything renders. The RenderRange type documents both readings.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 45a03342f1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…sed regions Give the editor code-fold semantics over a diff's collapsed unchanged regions, ahead of edit mode keeping them collapsed: - Arrow up/down (and shift-extended selection) skip over a collapsed region atomically to the nearest renderable line, staying put when nothing renders in that direction. The skip target comes from a single hunk-metadata walk (getNearestRenderableAdditionLine), not a per-line probe across the gap. - Jumps that must land inside a collapsed region — setSelections, search-match navigation, cmd+d/find-next, cmd+Home/End, undo/redo caret restores — expand the region minimally first via the new `revealLine` host API: one deterministic bump from the nearest gap edge sized to reach the target plus the normal expansion step, routed through expandHunk so CodeView's deferred expansion staging applies. Search counts still include hidden matches; navigating to one reveals it. - VirtualizedFileDiff's line-visibility oracle now accounts for staged pendingExpansions so the caret-scroll retry can park while a reveal waits for the next layout consume. Behavior is unchanged while editing still forces expandUnchanged.
Separator expand buttons keep working while an editor is attached: their pointer wiring was already independent of the edit-mode gutter lockdown and only unreachable because no separators rendered during editing. With collapse live mid-session, a click reveals the gap, the revealed context lines are immediately editable (a gap edit synthesizes a session region), and the expansion re-render rebuilds its AST from diff.additionLines — which the session paths keep current every pass, so live edits survive the render-cache clear. Covered by jsdom tests on the standalone collapse-during-edit vector: a real separator button click mid-edit, then typing and undoing inside the newly revealed context.
Entering edit mode on a diff no longer expands every collapsed unchanged region. Collapsed regions stay collapsed: the caret skips over them like code folds, jumps that must land inside one expand it minimally, separator expand buttons keep working, and the layout no longer jumps when editing starts or ends. Edits re-diff within the current hunk regions and a genuine session end restores the normal recompute (all built up in the preceding commits). - Editor.edit drops expandUnchanged from its option normalization — from the destructuring too, so the setOptions fallback (which replaces options wholesale) can no longer strip a host's own expandUnchanged when another forced option triggers it. - CodeView's edit-forced option facade stops serving expandUnchanged: true for edited diff items; the key rejoins the plain pass-through options. - The React contentEditable option merge stops injecting expandUnchanged: true. - Editors need full file contents, so attaching to a partial diff now triggers loadDiffFiles hydration directly (previously the forced expansion render did it as a side effect). - The genuine session-end recompute now drives layout invalidation explicitly (estimated-height reset plus a virtualizer height reconcile); the expandUnchanged option flip that used to invalidate layout at exit is gone. CodeView applies the same invalidation when it reaps a session whose instance was released. - Docs: the Editor guide describes the fold behavior, and the demo SSR preloads drop the baked-in expandUnchanged (mockData.generated.ts regenerated).
Expanding collapsed context while editing (separator clicks or reveal-on-jump) no longer resets when the session ends: expanded gap edges are snapshotted as old-side line ranges before the exit recompute — old-side coordinates are the ones edits cannot move — and remapped onto the recomputed hunks afterward. A preserved range that still touches a gap's start or end edge restores that edge's expansion; ranges whose gap disappeared just collapse. The exit flow consolidates into FileDiff.completeEditSession (marker-guarded, idempotent), shared by the live editor-detach path and CodeView's reap of sessions whose instance virtualization already released. The virtualized escalation now reports the layout change to its virtualizer so item tops re-derive from the changed heights.
New edit-collapsed fixture: a 60-line diff whose unchanged gap collapses, driven in both split and unified view (the unified content column rebuilds via innerHTML on edits, a named regression risk). Cases: the gap stays collapsed on edit entry, a reverted hunk persists as context until session exit and then collapses away, arrow keys fold-skip the gap, search navigation reveals a hidden match, a separator expand click mid-edit keeps the viewport and accepts typing into the revealed context, a programmatic replace into the gap materializes a rendered hunk without scrolling, and undo/redo round-trip with collapse live.
This is useful for different types of diff editing scenarios
Basically an attempt at doing better line matching on stuff
45a0334 to
7ff2050
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7ff2050d97
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… blank lines Unclear if this will create some knock on effects that make it harder to manage in the future...
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
* feat(diffs): Add edit-session region skeleton for diff hunks While an editor is attached to a FileDiff, hunk updates now preserve the current hunk structure instead of recomputing it per keystroke: each hunk is a frozen region, edits re-diff only the region they land in, a fully reverted region persists as a context-only hunk, edits spanning regions merge them, and edits inside an unchanged gap synthesize a new region. The real recompute runs once on genuine session exit, so post-session state is unchanged. This is groundwork for edit mode preserving collapsed unchanged regions; with expandUnchanged still forced during editing, rendering behavior is unchanged. Details: - New pure module `utils/editSessionHunks.ts` holds the region math (window mapping, region re-diff with zero context, expansion-key remapping, exit recompute) built on the exported updateDiffHunks helpers. - Line-count edit passes run updateRenderCache with dense, stale indexed dirty lines before applyDocumentChange. A new explicit `lineCountChangeInFlight` flag threads from the editor through the host so the session path defers hunk work to the document rebuild, which scans a per-session line snapshot instead of the contaminated live array. Deferred background-tokenize passes keep running region updates from their (current) changed indexes. - A same-line-count gap edit changes the rendered row set, which the debounced refresh cannot express; the host escalates to a deferred full re-render, and VirtualizedFileDiff also invalidates its layout caches. - The editor detach closure now reports whether it is a virtualized recycle or a genuine session end. Session hunks and a dirty marker on the shared FileDiffMetadata survive recycling; CodeView finishes sessions whose instance was already released, and FileDiff.render self-heals session-shaped metadata reused after a teardown. * feat(diffs): Make the editor DOM layer collapse-aware Prepare the editor for diffs whose collapsed unchanged regions stay collapsed during editing: rendered rows can then be a sparse subset of document lines, which several editor code paths assumed were dense. With expansion still forced during editing, behavior is unchanged. - New visibility oracle: `isAdditionLineRenderable` in virtualDiffLayout derives from hunks plus expansion state (the same inputs as iterateOverDiff), exposed to the editor as the optional `DiffsEditableComponent.isLineRenderable`. FileDiff implements it; plain files leave it unset and every line stays renderable. - `#rerender` filters dirty lines through the oracle, scans the row patch loop from child index 0 (a sparse row can sit at a smaller index than its dense position, and overshooting appended duplicate rows), and only creates new rows beyond the last rendered row. - `#applyChange` only widens the render window toward a caret whose line can actually gain a row; `#scrollToLine` refuses to park its retry state on a collapsed line, which previously became a non-terminating scroll loop against separator geometry. - `__syncRenderView` stamps hunk separator rows contenteditable=false in both diff styles, alongside annotation and deleted rows. - `FileDiff.syncRenderViewToEditor` converts the stored render range (rendered-row units for the windowed AST pipeline) into the document-line units the editor consumes; the two only coincide while everything renders. The RenderRange type documents both readings. * feat(diffs): Add fold-skip caret motion and reveal-on-jump for collapsed regions Give the editor code-fold semantics over a diff's collapsed unchanged regions, ahead of edit mode keeping them collapsed: - Arrow up/down (and shift-extended selection) skip over a collapsed region atomically to the nearest renderable line, staying put when nothing renders in that direction. The skip target comes from a single hunk-metadata walk (getNearestRenderableAdditionLine), not a per-line probe across the gap. - Jumps that must land inside a collapsed region — setSelections, search-match navigation, cmd+d/find-next, cmd+Home/End, undo/redo caret restores — expand the region minimally first via the new `revealLine` host API: one deterministic bump from the nearest gap edge sized to reach the target plus the normal expansion step, routed through expandHunk so CodeView's deferred expansion staging applies. Search counts still include hidden matches; navigating to one reveals it. - VirtualizedFileDiff's line-visibility oracle now accounts for staged pendingExpansions so the caret-scroll retry can park while a reveal waits for the next layout consume. Behavior is unchanged while editing still forces expandUnchanged. * feat(diffs): Keep hunk separator expansion interactive during editing Separator expand buttons keep working while an editor is attached: their pointer wiring was already independent of the edit-mode gutter lockdown and only unreachable because no separators rendered during editing. With collapse live mid-session, a click reveals the gap, the revealed context lines are immediately editable (a gap edit synthesizes a session region), and the expansion re-render rebuilds its AST from diff.additionLines — which the session paths keep current every pass, so live edits survive the render-cache clear. Covered by jsdom tests on the standalone collapse-during-edit vector: a real separator button click mid-edit, then typing and undoing inside the newly revealed context. * feat(diffs): Stop forcing expandUnchanged while editing a diff Entering edit mode on a diff no longer expands every collapsed unchanged region. Collapsed regions stay collapsed: the caret skips over them like code folds, jumps that must land inside one expand it minimally, separator expand buttons keep working, and the layout no longer jumps when editing starts or ends. Edits re-diff within the current hunk regions and a genuine session end restores the normal recompute (all built up in the preceding commits). - Editor.edit drops expandUnchanged from its option normalization — from the destructuring too, so the setOptions fallback (which replaces options wholesale) can no longer strip a host's own expandUnchanged when another forced option triggers it. - CodeView's edit-forced option facade stops serving expandUnchanged: true for edited diff items; the key rejoins the plain pass-through options. - The React contentEditable option merge stops injecting expandUnchanged: true. - Editors need full file contents, so attaching to a partial diff now triggers loadDiffFiles hydration directly (previously the forced expansion render did it as a side effect). - The genuine session-end recompute now drives layout invalidation explicitly (estimated-height reset plus a virtualizer height reconcile); the expandUnchanged option flip that used to invalidate layout at exit is gone. CodeView applies the same invalidation when it reaps a session whose instance was released. - Docs: the Editor guide describes the fold behavior, and the demo SSR preloads drop the baked-in expandUnchanged (mockData.generated.ts regenerated). * feat(diffs): Preserve expansion state best-effort across session exit Expanding collapsed context while editing (separator clicks or reveal-on-jump) no longer resets when the session ends: expanded gap edges are snapshotted as old-side line ranges before the exit recompute — old-side coordinates are the ones edits cannot move — and remapped onto the recomputed hunks afterward. A preserved range that still touches a gap's start or end edge restores that edge's expansion; ranges whose gap disappeared just collapse. The exit flow consolidates into FileDiff.completeEditSession (marker-guarded, idempotent), shared by the live editor-detach path and CodeView's reap of sessions whose instance virtualization already released. The virtualized escalation now reports the layout change to its virtualizer so item tops re-derive from the changed heights. * test(diffs): Add browser E2E coverage for editing with collapsed regions New edit-collapsed fixture: a 60-line diff whose unchanged gap collapses, driven in both split and unified view (the unified content column rebuilds via innerHTML on edits, a named regression risk). Cases: the gap stays collapsed on edit entry, a reverted hunk persists as context until session exit and then collapses away, arrow keys fold-skip the gap, search navigation reveals a hidden match, a separator expand click mid-edit keeps the viewport and accepts typing into the revealed context, a programmatic replace into the gap materializes a rendered hunk without scrolling, and undo/redo round-trip with collapse live. * Add a new HTML editable example to /playground This is useful for different types of diff editing scenarios * feat(diffs): semantic diffs, at home Basically an attempt at doing better line matching on stuff * review(fix): Fix the arrow keys issue brought up by codex * fix(bug): Fix a strange edge cases with diffs that include intermixed blank lines Unclear if this will create some knock on effects that make it harder to manage in the future...
This is a mostly AI driven rework to allow editing to work without expanding unchanged regions. This should make the editing process feel more seamless from a normal state.
I think ideally we wouldn't have any required option changes and figure out a way to just make it work behind the scenes, but those are probably follow ups to this core change.