fix(diff-view): restore scroll position and fix tab handling on save/deny

[awschmeder]

Related GitHub Issue

Closes: #586

Description

Fixes the diff-view scroll and tab-handling problems described in #586.
The core logic changes are confined to DiffViewProvider.ts;
no tool files needed to change because the default diff path already calls
open()/update()/scrollToFirstDiff()/saveChanges() in the right order.

The "how", broken down by the symptoms in the issue:

1. Diff opens at the wrong scroll position.

   • Removed the unconditional scrollEditorToLine(0) in open(). It fired after the
     diff rendered and overrode whatever position should have been shown, leaving
     off-screen edits out of view.
   • scrollToFirstDiff() is refactored into three private helpers:
     • findFirstDiffLine() walks diff.diffLines() to compute the first-change line and
       clamps it to a valid index so pure end-of-file removals (where the computed index
       lands past document.lineCount - 1) are still brought into view.
     • resolveLiveEditor() re-resolves the modified-side editor from
       vscode.window.visibleTextEditors at scroll time, replacing any stale/detached
       activeDiffEditor reference whose visibleRanges no longer reflect the on-screen
       diff widget. A revealRange on a stale reference is a silent no-op; this was the
       root cause of "scrolls to top" when the file was already open.
     • revealDiffLine() anchors the editor selection on the target line before calling
       revealRange. The selection nudge is necessary when the target is already inside
       the current viewport -- a bare revealRange on a visible line is a no-op.
   • A deferred re-reveal fires 100 ms after the synchronous one. The vscode.diff
     command re-lays-out the diff widget after a short delay; the deferred reveal ensures
     the scroll sticks after that layout pass. It is gated on activeDiffEditor === editor
     so a stale timer from a previous edit never acts on a later edit's diff widget.

2. Viewport jumped to the top after Save/Deny.

   • open() now captures preEditScrollLine (the first visible line) BEFORE the
     tab-closing loop -- after that loop the editor is no longer in visibleTextEditors
     and the capture would always be undefined.
   • Both accept and deny paths route through showEditedFileWithoutDisruptingFocus(),
     which calls revealRange(..., AtTop) to restore the pre-edit scroll position.
   • cancelDeferredScroll() is called at the start of both saveChanges() and
     revertChanges() before any programmatic editor activation. With auto-approve writes,
     saveChanges() runs in the tight window right after scrollToFirstDiff()'s deferred
     timer is set; the cancellation prevents the late reveal from firing after the viewport
     is restored and yanking the user back to the diff target.

3. Tabs left open for files that were not previously open.

   • saveChanges() previously re-showed the file unconditionally; closeAllDiffViews()
     only closes diff-scheme tabs, so the plain file tab leaked. The re-show is now gated.
   • Added closeFileTab() to close the transiently opened plain file tab when the file
     was not open before the edit AND the user did not interact with it.
   • Added a userTouchedDocument signal: while the diff is open for a
     not-previously-open file, an onDidChangeActiveTextEditor listener marks the file as
     "touched" if the user activates its own (non-diff) editor tab. Touched files are kept
     open; untouched ones are closed. The listener is disposed before any programmatic
     showTextDocument so our own re-show never counts as a touch, and is cleared in
     reset().

Additional fixes discovered during implementation:

4. Focus disruption on keep-open. After Save/Deny, showing the edited file previously
   stole the foreground even when the user had navigated to a different file.
   showEditedFileWithoutDisruptingFocus() captures the user's active editor before the
   re-show, uses preserveFocus: true, and re-activates the user's prior editor afterward
   when they were viewing something else.

5. Pin state lost. Closing the target file's tab to open the diff dropped VS Code's pin
   state. open() now records documentWasPinned; after the diff closes the pin is
   restored via workbench.action.pinEditor.

6. Preview-tab eviction. Opening the diff reuses the editor group's single preview slot
   and permanently evicts unrelated preview tabs. open() now snapshots all unrelated
   preview tabs (with their scroll positions) before the diff opens; restorePreviewTabs()
   re-opens any that were evicted after the diff closes. Files that no longer exist on disk
   are skipped gracefully.

7. Diff pane interaction keeps target tab open.
   If the user clicks or edits inside the diff pane itself, the target file's tab is kept
   open after Save/Deny -- even if the file was not previously open. This extends the
   existing userTouchedDocument concept to cover diff-pane interaction via
   onDidChangeTextEditorSelection (filtered to kind = Mouse | Keyboard to exclude
   programmatic selection changes such as those from scrollToFirstDiff()). The listener
   compares by document identity rather than editor object identity to handle stale
   editor references that VS Code creates when the diff widget refreshes. If the user only
   scrolls in the diff pane, the existing close behavior is preserved.

8. Scroll position mirrors the most-recently-viewed pane.
   When the target file is re-revealed after a diff closes, it scrolls to whichever
   position the user last viewed:

   • If the user last scrolled in the diff pane -> the target file mirrors that line.
   • If the user last scrolled in the target file's own editor -> that position is used.
   • Otherwise -> the pre-edit scroll position is restored (existing behavior).
     Both the diff and target file editors are tracked via onDidChangeTextEditorVisibleRanges;
     lastScrolledSource records which fired most recently so the correct position wins.

9. User-configurable auto-close settings (UI tab in Settings).
   The auto-close decisions from item 3 above are now user-adjustable via three new settings
   exposed in Settings > UI:

   • Auto-close Zoo-opened files (autoCloseZooOpenedFiles, default: on) -- close
     transient file tabs Zoo opened once the diff is accepted/denied.
   • Auto-close even after user interaction (autoCloseZooOpenedFilesAfterUserEdited,
     default: off) -- override the "keep if touched" guard; only effective when the base
     toggle is also on.
   • Auto-close newly created files (autoCloseZooOpenedNewFiles, default: off) -- also
     close the tab when Zoo created the file (not just edited an existing one).
     All three are wired from the global settings schema through SettingsView cached state
     to UISettings checkboxes, and read in DiffViewProvider.keepOrCloseEditedFile().
     i18n keys added under the ui namespace in all 18 locale files.

Reviewers should pay attention to: the ordering of disposeActiveEditorListener() ->
cancelDeferredScroll() -> closeAllDiffViews() -> keepOrCloseEditedFile() ->
restorePreviewTabs() in both saveChanges() and revertChanges(), the clamp logic in
findFirstDiffLine() and the second clamp in revealDiffLine(), and the stale-editor
guard in resolveLiveEditor().

Incidental change (discovered during this work): the diff tab label constant
DIFF_VIEW_LABEL_CHANGES still read "Original <-> Roo's Changes". Since this is a
user-visible label in the rebranded fork, it was corrected to "Original <-> Zoo's
Changes". It is a one-line string change referenced only via the constant (tests use the
constant, not the literal), so it is behavior-neutral for tab matching. Called out here for
transparency since it is outside the strict scope of the scroll fix.

Test Procedure

Automated:

cd src && npx vitest run integrations/editor/__tests__/DiffViewProvider.spec.ts
cd webview-ui && npx vitest run src/components/settings/__tests__/UISettings.spec.tsx

DiffViewProvider: 67 tests pass, covering:

• scrollToFirstDiff(): first-change line for addition-only, deletion-only, mixed, and
  end-of-file removal (clamped); selection anchor before reveal (in-viewport fix); deferred
  re-reveal fires at 100 ms; live editor resolved at reveal time (stale reference fix);
  stale timer from a previous edit does not act on a new diff editor; no-op when no editor.
• preEditScrollLine: captured from visibleRanges[0].start.line in open(); undefined
  when no visibleRanges; saveChanges() and revertChanges() call revealRange(AtTop)
  when set; no revealRange when undefined; deferred scroll cancelled so it cannot fight
  the scroll restore (auto-approve race).
• userTouchedDocument: saveChanges() and revertChanges() close the tab when untouched;
  keep it open when touched; activation listener fires correctly; listener ignored for diff
  tab activations.
• userTouchedDiffEditor: saveChanges() keeps the tab open when the user clicked/edited
  in the diff pane; closes it when the user only scrolled; listener registered in open();
  programmatic selection changes (kind=Command or undefined) do not set the flag; stale
  editor reference resolved by document identity; unrelated editor events ignored.
• showEditedFileWithoutDisruptingFocus(): re-activates user's prior editor when they
  navigated away; does not re-activate when already on the edited file; re-pins and focuses
  correctly; does not pin when not originally pinned.
• Preview tab snapshot/restore: captures preview tabs with scroll positions, excludes the
  diff target; restores evicted tabs in preview mode with scroll; skips still-open tabs;
  skips files that no longer exist.
• Scroll position precedence: uses diffScrollLine when lastScrolledSource is 'diff';
  uses targetFileScrollLine when lastScrolledSource is 'targetFile'; falls back to
  preEditScrollLine when source is undefined; no revealRange when all sources are
  undefined; target-file scroll overrides diff scroll regardless of values.
• diffScrollListener wiring: records diffScrollLine and sets lastScrolledSource to
  'diff' on diff editor scroll; records targetFileScrollLine and sets source to
  'targetFile' on target file scroll; ignores unrelated editors; lastScrolledSource
  reflects the most recent scroll between the two editors.
• Auto-close settings decision table: already-open file never auto-closed; transient tab
  kept when autoCloseZooOpenedFiles is false; closed when true (default); touched tab kept
  by default; closed when autoCloseZooOpenedFilesAfterUserEdited is true (and base is on);
  new-file tab closed when autoCloseZooOpenedNewFiles is true; all unset settings preserve
  existing behavior.

UISettings: auto-close checkbox tests -- renders all three checkboxes; each reflects its
prop value (true/false); each calls setCachedStateField with the correct key on toggle.

Manual (Extension Development Host, F5): unit tests cannot confirm the composite diff
widget visibly scrolls or that focus/tabs behave correctly across real VS Code sessions.
Full manual matrix is documented in a comment attached to this PR.

Summary: for a 100+ line file, verify the diff opens on the first changed line
and the viewport is restored / the tab is closed appropriately after Save and after Deny,
for both already-open and not-open files, across pure-add, pure-delete, and mixed edits,
including diffs at the top, midpoint, and end of the file. Confirm no focus steal, no
preview-tab loss, and no stuck-scroll across sequential edits. Also verify: clicking or
editing inside the diff pane keeps the target tab open; scrolling only in the diff pane
still closes the tab; after Save the target file scrolls to the last-viewed position in
the diff or target file editor, whichever was scrolled most recently. Verify the three
auto-close checkboxes in Settings > UI toggle the behavior as documented.

Pre-Submission Checklist

• Issue Linked: This PR is linked to an approved GitHub Issue (see "Related GitHub Issue" above).
• Scope: My changes are focused on the linked issue (one major feature/fix per PR).
• Self-Review: I have performed a thorough self-review of my code.
• Testing: New and/or updated tests have been added to cover my changes (if applicable).
• Documentation Impact: I have considered if my changes require documentation updates (see "Documentation Updates" section below).
• Contribution Guidelines: I have read and agree to the Contributor Guidelines.

Screenshots / Videos

Attached in a follow-up comments.

Documentation Updates

• No documentation updates are required.

Additional Notes

The fix targets the DEFAULT (diff-visible) path only. When the
EXPERIMENT_IDS.PREVENT_FOCUS_DISRUPTION experiment is enabled, tools call
saveDirectly() instead, which intentionally avoids any diff view, scroll-to-change, or
viewport restoration -- none of the code in this PR runs on that path.

Commentary / Opinion

Automatically closing an un-touched tab after applying an edit or creating a new file could be somewhat confusing to novice coders.

On the other hand, not closing un-touched files after edits creates significant tab pollution in long sessions, leading to dozens of tabs left open.

The unclosed-tab pollution problem also burdens the context window in an uncontrolled manner and drives up inference cost, as Zoo's default includes 20 open tabs (probably set too high, by the way... 1 is a better default).

On balance, I think the best solution is to automatically close files that the user did not either explicitly open or have some affirmative interaction with by viewing / focusing on or editing. The three new settings in Settings > UI give users full control to opt out of any part of this behavior.

Summary by CodeRabbit

• New Features
  • Added three new settings to control whether editor tabs opened by Zoo are automatically closed after accept/reject, optionally after user interaction, and for newly created files.
• Bug Fixes
  • Improved diff view behavior to open on the first changed line (with end-of-file clamping) and better preserve/restore tab focus and scroll positions when accepting/rejecting.
  • Updated the diff view label to “Original ↔ Zoo’s Changes”.
• Documentation
  • Added release-note metadata and UI localization for the new auto-close settings across supported languages.
• Tests
  • Expanded DiffViewProvider test coverage for scroll, focus, and tab restore/auto-close scenarios.

[awschmeder]

Manual Test Plan: Diff View Scroll Position + Tab/Focus Handling

Covers the fixes in DiffViewProvider.ts
for issue #586 and the follow-on bugs found during review. Unit tests verify which line is
targeted and which branch runs, but they cannot confirm that the composite VS Code diff
widget visibly scrolls, or that tab/focus behavior is correct in a live session.

Run this matrix manually in the Extension Development Host.

Bugs this plan must catch

These are the concrete defects found and fixed. Each has dedicated coverage below.

1. In-viewport no-scroll -- file already open and scrolled to the middle; a diff
   targeting a line already inside the viewport did not scroll (the captured editor
   reported a stale viewport, so revealRange was a silent no-op).
2. Scroll-to-top on open -- file already open and scrolled to the middle; the diff
   jumped to the top and never showed the change (stale/detached editor reference).
3. End-of-file no-scroll -- a diff targeting the last line failed to scroll.
4. Stuck at line 0 -- once the scroll broke, every subsequent edit also scrolled to
   the top until the file was closed and reopened (leaking deferred-scroll timers acting
   on a later edit's diff editor).
5. Auto-approve late-timer race -- with auto-approve write enabled, a deferred scroll
   could fire after the post-save viewport restore and yank the file back to the change.
6. Preview-tab eviction -- an unrelated file open as a preview tab (italic,
   not-yet-edited) was permanently lost when the diff reused the editor group's single
   preview slot.
7. Focus disruption on keep-open -- after Save/Deny, the edited file was force-shown
   and stole the foreground even when the user had navigated to a different file.

Prerequisites

1. Launch the Extension Development Host: open the repo in VS Code and press F5
   (Run Extension). A second VS Code window opens with the dev build of Zoo Code.
2. In the dev window, open a workspace containing a long file -- several hundred lines,
   well beyond one screen height. A file of ~400-600 lines is ideal so that "top",
   "midpoint", and "end" are all clearly off-screen from one another. Have at least two
   such files available (referred to below as file-1 and file-2).
3. Confirm the preventFocusDisruption experiment is OFF (Settings ->
   Experimental). This fix only applies to the default diff-visible path; with the
   experiment on, saveDirectly() runs and none of this behavior applies.
4. When prompting Zoo, instruct it to use the diff/apply_diff tool so it does not apply
   edits via shell commands. Example: "Using the apply_diff tool, remove the last line of
   long.ts." This keeps the edit on the path under test.
5. Know how to toggle auto-approve writes -- Matrix 4 requires it ON, all other
   matrices assume it OFF (so you can observe the diff and click Save/Deny).

What "correct" looks like

• On diff open: the diff editor is scrolled so the first changed line is visible
  (centered), regardless of whether the change is a pure addition, pure deletion, or mixed,
  regardless of where in the file the change is, and regardless of where the file was
  already scrolled (including when the change is already inside the current viewport).
• After Save:
  • File was already open before the edit -> the file is shown and the viewport is
    restored to the pre-edit scroll position (the saved line at the top of the viewport).
  • File was NOT open before, and the user did not interact with it -> the transient tab
    is closed (no tab pollution).
  • File was NOT open before, but the user clicked into its tab during the diff -> the tab
    stays open.
• After Deny: same tab keep/close and scroll-restore behavior as Save. A
  newly-created (non-existent) file is removed and its tab closed.
• Focus: focus is never stolen to the edited file. If you navigate to a different file
  while the diff is up, you remain on that file after Save/Deny -- you are neither pulled
  back to the edited file nor left on a stale diff tab.
• Preview tabs: an unrelated file open as a preview tab is not permanently lost; after
  the diff session ends it is restored as a preview tab at its prior scroll position.
• Sequential edits: scrolling stays correct edit after edit; a broken scroll never
  "sticks" across edits.

---

Matrix 1 -- Diff opens on the first change (scroll-to-first-diff)

For each combination below, trigger the edit and observe ONLY the diff-open scroll
position (do not Save/Deny yet -- that is Matrix 2/3).

Edit positions to test: line 0 / top, midpoint of the file, end of the file.
Edit shapes to test: pure addition, pure deletion, mixed (remove + add).

For the "Already open" rows, first scroll the file to the middle before prompting, so
the in-viewport and stale-viewport cases are exercised (bugs 1-3).

#	File state	Position	Shape	Expected on open
1.1	Already open (scrolled to top)	Top	Add	Diff shows the added line near the top
1.2	Already open (scrolled to MIDDLE)	Midpoint	Add	Diff scrolls to the midpoint change (NOT the top)
1.3	Already open (scrolled to MIDDLE)	End	Add	Diff scrolls to the end change
1.4	Already open (scrolled to MIDDLE)	Midpoint	Delete	Diff scrolls to the deletion
1.5	Already open (scrolled to MIDDLE)	End	Delete	Diff scrolls to the deletion (clamped to last line, widget moves)
1.6	Already open (scrolled to MIDDLE)	Midpoint	Mixed	Diff scrolls to the first changed line
1.7	Already open (scrolled to MIDDLE)	End	Mixed	Diff scrolls to the first changed line
1.8	NOT open	End	Delete	File opens; diff scrolls to the deletion
1.9	NOT open	End	Add	File opens; diff scrolls to the added region
1.10	NOT open	Midpoint	Mixed	File opens; diff scrolls to the first changed line
1.11	Already open (scrolled to MIDDLE)	Change INSIDE the current viewport	Mixed	Diff scrolls/recenters on the change; viewport does NOT jump to the top (bug 1)
1.12	Already open (scrolled to MIDDLE)	End (last line)	Mixed	Diff scrolls to the last line; the widget actually moves (bug 3)

Pay special attention to 1.2-1.7, 1.11, 1.12 -- these are the regressions where an
already-open, mid-scrolled file either jumped to the top or refused to scroll.

1.S -- "Stuck scroll" sequential check (bug 4)

1. Open file-1, scroll to the middle.
2. Ask Zoo to make an end-of-file edit to file-1 via apply_diff. Observe the
   diff-open scroll (it should reach the end, not the top).
3. WITHOUT closing the file, ask Zoo to make a midpoint edit to file-1.
4. Expected: the second edit scrolls to the midpoint change. It must NOT be "stuck"
   scrolling to line 0. Repeat for several edits in a row.

---

Matrix 2 -- Save (accept) behavior

Set up a known pre-edit scroll position before each run by manually scrolling the file, so
restoration is observable.

#	File state before edit	User action during diff	After clicking Save
2.1	Open, scrolled to midpoint	none	File shown; viewport restored to the midpoint scroll position
2.2	Open, scrolled to end	none	File shown; viewport restored to the end scroll position
2.3	NOT open	none	Transient file tab is CLOSED; no leftover tab
2.4	NOT open	click into the file's editor tab during the diff	File tab STAYS open after Save
2.5	Open; then navigate to a different file during the diff	viewing another file	Focus stays on the other file; you are not pulled back
2.6	New file (did not exist)	none	File is created and saved; no stray diff tab; viewport sane

---

Matrix 3 -- Deny (reject) behavior

#	File state before edit	User action during diff	After clicking Deny
3.1	Open, scrolled to midpoint	none	Edit reverted; viewport restored to the midpoint scroll position
3.2	NOT open	none	Edit reverted; transient file tab is CLOSED
3.3	NOT open	click into the file's editor tab during the diff	Edit reverted; file tab STAYS open
3.4	New file (did not exist)	none	File is deleted; its tab is closed; created dirs cleaned up
3.5	Open; navigate elsewhere during the diff	viewing another file	Focus stays on the other file

---

Matrix 4 -- Auto-approve late-timer race (bug 5)

Enable auto-approve writes for these rows so save runs immediately after the diff
opens (the tight window where a deferred scroll could fire after the viewport is restored).

#	File state before edit	Position	Expected after the auto-approved save settles
4.1	Open, scrolled to midpoint	Midpoint	File ends restored to the pre-edit midpoint scroll; it does NOT settle on the change line a beat later
4.2	Open, scrolled to midpoint	End	File ends restored to the pre-edit scroll; no late jump to the end
4.3	Open, scrolled to top	Midpoint	File ends at the pre-edit top; no delayed scroll-to-change
4.4	Sequence: auto-approve several edits in a row	mixed positions	No accumulating drift; each settles at the restored position; no tab accumulation

Watch for ~100ms-late movement: the buggy behavior was a brief correct restore followed
by the viewport snapping to the diff target.

---

Matrix 5 -- Preview-tab eviction (bug 6)

A preview tab is the italicized, single reusable tab VS Code shows when you open a file
but have not edited it.

#	Setup	Action	Expected
5.1	Open file-1 as a preview tab (single click in Explorer, do not edit). file-2 is closed.	Ask Zoo to apply_diff file-2, then Save.	file-1 is NOT permanently lost: after the diff session it is restored as a preview tab at its prior scroll position. file-2 handled per Matrix 2.
5.2	Same as 5.1	Click Deny instead of Save	Same restore of file-1's preview tab.
5.3	Open file-1 as a preview tab, scrolled to the middle.	apply_diff file-2 + Save	file-1 restored AND scrolled back to its prior (middle) position.
5.4	Open file-1 as a permanent (non-preview, e.g. double-click or edited) tab.	apply_diff file-2 + Save	file-1 is untouched throughout (permanent tabs are never evicted); no duplicate file-1 tab appears.
5.5	Open file-1 as a preview tab.	During the diff, the preview file is deleted/renamed on disk, then Save	Restore is skipped gracefully -- no error dialog, no broken tab.

---

Matrix 6 -- Focus preservation on keep-open (bug 7)

#	Setup	Action sequence	Expected after Save/Deny
6.1	file-1 open (non-preview). file-2 closed.	apply_diff file-2; click file-2's plain tab; then click file-1; then Save	You remain on file-1 (where you were actually looking). Focus is NOT yanked to file-2. file-2 stays open in the background.
6.2	Same as 6.1	Same but click Deny	Same: you remain on file-1; edit reverted; file-2 stays open in the background.
6.3	file-2 already open and active.	apply_diff file-2; stay on file-2; Save	You remain on file-2 at its restored scroll position (no redundant focus round-trip).
6.4	Two editor groups (split). file-1 active in group 1.	apply_diff file-2 (opens in a group); navigate around; Save	You end on whatever editor you were last viewing; no surprise foreground change.

---

Regression smoke checks

• Pinned tab: pin the file's tab, run an edit + Save. Confirm the tab is not unpinned
  or duplicated.
• Dirty unrelated tab: have another file with unsaved changes open; run an edit + Save.
  Confirm the unrelated dirty tab is untouched (closeFileTab filters on !isDirty and
  matches only the target path).
• Multiple sequential edits: ask Zoo to make several edits in a row to the same long
  file; confirm the viewport does not drift to the top between edits, the scroll never gets
  "stuck", and tabs do not accumulate.
• Diff tab label: confirm the diff tab reads "Original <-> Zoo's Changes".
• Interrupted session: start an edit, then reload the dev window (Cmd+Shift+P ->
  "Developer: Reload Window") before accepting. Confirm no preview file is left
  permanently promoted and no stray diff tab remains.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

DiffViewProvider extended to track pin/scroll/touch state, snapshot unrelated preview tabs, install activation/scroll/interact listeners on open, reliably reveal the first changed line with EOF clamping and guarded deferred reveal, apply user-configurable auto-close settings on save/revert, and restore or close tabs accordingly. Settings UI and schema updated to expose three auto-close options across all locales.

Changes

Diff View Scroll Position, Tab State, and Auto-Close Settings

Layer / File(s)	Summary
Changeset documentation .changeset/fix-diff-scroll-position.md, .changeset/add-editor-auto-close-settings.md	Two changelog entries document the diff-view scroll-to-first-change behavior and the new auto-close settings for controlling transient tab closure.
Settings schema and extension state packages/types/src/global-settings.ts, packages/types/src/vscode-extension-host.ts	Global settings schema extended with three new optional boolean fields (autoCloseZooOpenedFiles, autoCloseZooOpenedFilesAfterUserEdited, autoCloseZooOpenedNewFiles); ExtensionState type updated to include those settings keys.
Settings UI state management webview-ui/src/components/settings/SettingsView.tsx	SettingsView destructures the three new auto-close settings from cached state, includes them in handleSubmit payload with default fallbacks, and forwards them as props to UISettings for rendering.
Settings UI component rendering webview-ui/src/components/settings/UISettings.tsx	UISettings component accepts three new optional boolean props and renders corresponding SearchableSetting checkboxes with translation keys, default values, and state updates via setCachedStateField.
Settings UI and state tests webview-ui/src/components/settings/__tests__/SettingsView.change-detection.spec.tsx, webview-ui/src/components/settings/__tests__/UISettings.spec.tsx	Test mocks extended with new default settings; SettingsView tests verify cached state initialization; UISettings tests verify checkbox rendering, state reflection, and callback wiring for all three options.
Multi-language localization webview-ui/src/i18n/locales/**/settings.json (18 locales)	Translation keys for the three auto-close settings added across all language locales; each includes label and description strings describing when Zoo auto-closes tabs.
DiffViewProvider state tracking and label src/integrations/editor/DiffViewProvider.ts	Private state expanded with pin/scroll/touch flags, deferred scroll timer, and preview-tab snapshots; diff label constant updated to "Original ↔ Zoo's Changes".
State capture and initialization on diff open src/integrations/editor/DiffViewProvider.ts	On open(), capture visible scroll position, record pin/open flags, snapshot preview tabs, avoid forced top-scroll, and install listeners for tab activation, diff-editor interaction, and scroll tracking in both editors.
Save and revert completion flows src/integrations/editor/DiffViewProvider.ts	saveChanges() and revertChanges() dispose listeners, cancel timers, apply auto-close settings logic to decide whether to keep or close the edited file tab, restore evicted preview tabs, and close transient tabs for new-file revert.
Helpers for tab, focus, and scroll restoration src/integrations/editor/DiffViewProvider.ts	New helper methods centralize listener/timer disposal, auto-close decision logic, re-showing edited file without disrupting focus (restore prior active editor and optional re-pin), selective preview-tab restoration, and transient tab closure.
Scroll to first diff with clamping and guarded reveal src/integrations/editor/DiffViewProvider.ts	scrollToFirstDiff() resolves the live modified-side editor at reveal time, computes first changed line with EOF clamping, anchors selection, and uses guarded 100ms deferred reveal to prevent stale-timer interference.
Session state cleanup src/integrations/editor/DiffViewProvider.ts	reset() clears all tracked state, cancels deferred scroll, disposes listeners, and clears preview-tab snapshots.
Test infrastructure and mocks src/integrations/editor/__tests__/DiffViewProvider.spec.ts	Extended Vitest mocks: fs/promises.access, refined vscode.window.tabGroups, and richer editor primitives (Range, Position, Selection, TextEditorRevealType.AtTop, TabInputText).
Test coverage: open(), scroll reveal, and preview restoration src/integrations/editor/__tests__/DiffViewProvider.spec.ts	Tests verify open() pin/open tracking; scrollToFirstDiff() covers add-only, delete-only, and mixed diffs with EOF clamping, deferred re-reveal, and live vs stale editor targeting; preview-tab snapshot/restore validates scroll+column capture and selective restoration.
Test coverage: scroll capture and restoration src/integrations/editor/__tests__/DiffViewProvider.spec.ts	Tests validate preEditScrollLine capture during open(), restoration with AtTop on save/revert, skipping when undefined, and deferred-scroll cancellation preventing override after saveChanges().
Test coverage: user touch and tab semantics src/integrations/editor/__tests__/DiffViewProvider.spec.ts	Tests validate userTouchedDocument behavior: tab close/keep logic based on open and touch state; tests verify flag is set when plain file editor is activated during diff.
Test coverage: diff editor interaction and selection tracking src/integrations/editor/__tests__/DiffViewProvider.spec.ts	Tests validate userTouchedDiffEditor behavior: close/keep tab semantics based on interaction; onDidChangeTextEditorSelection wiring filters for mouse/editor-originating events only, excluding programmatic selection and unrelated editors.
Test coverage: scroll position precedence and listener wiring src/integrations/editor/__tests__/DiffViewProvider.spec.ts	Tests validate correct scroll-reveal source precedence (diffScrollLine vs targetFileScrollLine vs preEditScrollLine); visible-range listener wiring tracks recency and ignores unrelated editor scrolls.
Test coverage: auto-close settings decision table src/integrations/editor/__tests__/DiffViewProvider.spec.ts	Comprehensive tests validate how DiffViewProvider state and auto-close settings combinations control tab keep/close behavior across already-open files, transient vs touched tabs, user-interaction overrides, and new-file handling.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Poem

🐰 I hopped through diffs and saved the view,
Pinning tabs and scrolls where they once knew,
Three new settings guide when tabs should stay,
Previews return, no sudden jumps away,
Zoo's Changes settle soft in every language, true.

Suggested reviewers

• taltas
• JamesRobert20
• hannesrudolph
• edelauna

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The PR title accurately describes the main focus: fixing diff-view scroll position restoration and tab handling on save/deny operations.
Description check	✅ Passed	The PR description is comprehensive and well-structured, covering all required template sections with detailed implementation explanation, test procedures, and pre-submission checklist.
Linked Issues check	✅ Passed	The PR fully addresses all requirements from issue #586: diff scroll positioning at first changed line, post-Save/Deny viewport restoration, tab pollution prevention via touch-tracking, focus preservation, editor state persistence, race-condition avoidance, and user-configurable auto-close settings with UI controls.
Out of Scope Changes check	✅ Passed	All changes align with issue #586 requirements. The incidental diff-tab label correction (Roo→Zoo) is transparent and behavior-neutral; all code changes serve scroll/tab/focus objectives; i18n additions support the new auto-close settings.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[awschmeder]

Videos from Manual Testing:

Target file open - scrolled to top - diff scrolls to edit at top, middle, bottom:
https://github.com/user-attachments/assets/b290570c-373b-4cb7-b640-966a038ce0aa

Target file open - scrolled to middle - diff scrolls to edit and restores viewport after accept:
https://github.com/user-attachments/assets/f2e8a29f-0faf-4d9c-b12a-115b2c1af94d

Target file not open - diff scrolls to edit - tab is cleaned up after accept or reject:
https://github.com/user-attachments/assets/db8eee31-64f5-4c91-a352-59b3d9d81a16

Stress test multiple sequential writes & scrolling with apply_diff - no race conditions or errors:
https://github.com/user-attachments/assets/02d95019-9826-4f7e-8fa3-178a95ce02dc

Preview tab open with file 1, diff targets unopened file 2, after accept, preview tab is restored:
https://github.com/user-attachments/assets/a89d501b-b456-4694-8446-c1321abb71a9

Permanent tab open with file 1, diff targets unopened file 2, user changes tab then accepts - no jump back:
https://github.com/user-attachments/assets/197ddd37-2f38-4811-b97f-44058d6a72b6

Pinned file targeted with diff - stays pinned correctly:
https://github.com/user-attachments/assets/a016eb27-6f54-4759-b3f5-d2dec3041bb6

[codecov]

Codecov Report

❌ Patch coverage is 87.70950% with 22 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/integrations/editor/DiffViewProvider.ts	87.05%	12 Missing and 10 partials ⚠️

📢 Thoughts on this report? Let us know!

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/integrations/editor/DiffViewProvider.ts`:
- Around line 615-633: captureUnrelatedPreviewTabs currently returns only { uri,
scrollLine } so restorePreviewTabs reopens previews in the active group; include
the originating group/viewColumn in the snapshot (e.g., add tab.group.viewColumn
or tab.group.id to the captured object inside captureUnrelatedPreviewTabs) and
update restorePreviewTabs to call vscode.window.showTextDocument(snapshot.uri, {
preview: true, preserveFocus: true, viewColumn: snapshot.viewColumn }) (or open
in the original group via its id) and honor the saved scrollLine after opening;
update types/usages of the snapshot array and any related helper functions to
handle the new viewColumn field.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 82610a67-0e7a-4e94-8022-c666f20722a9

📥 Commits

Reviewing files that changed from the base of the PR and between 47f9470 and c51054c.

📒 Files selected for processing (3)

• .changeset/fix-diff-scroll-position.md
• src/integrations/editor/DiffViewProvider.ts
• src/integrations/editor/__tests__/DiffViewProvider.spec.ts

[awschmeder]

Follow-up commit summary

A second commit (6ea564c) was added after the initial PR push to address a gap in the preview-tab restore logic.

Fix: restore preview tabs to their original editor group

The initial implementation of captureUnrelatedPreviewTabs() / restorePreviewTabs() captured each evicted preview tab's URI and scroll position but did not record which editor group (column) the tab belonged to. As a result, showTextDocument during restore omitted viewColumn, causing VS Code to reopen the tab in the active group rather than its original group.

Changes:

• snapshotPreviewTabs type extended to include viewColumn: vscode.ViewColumn
• captureUnrelatedPreviewTabs() now records group.viewColumn alongside the URI and scroll line
• restorePreviewTabs() passes viewColumn: snapshot.viewColumn to showTextDocument
• Tests updated: setTabs() helper accepts a viewColumn argument; snapshot/restore assertions verify the correct column is captured and passed through

[awschmeder]

Update -- commit 003a31e

Two additional behaviors have been implemented on top of the original scroll/tab fix:

Keep target tab open when user interacts with the diff pane
Previously, the target file tab was only kept open if the user activated the file's own editor tab. This commit extends that: if the user clicks or edits anywhere inside the diff pane itself (onDidChangeTextEditorSelection with kind = Mouse | Keyboard), the tab is also kept open after Save/Deny. Programmatic selection changes (e.g. from scrollToFirstDiff()) are filtered out via the kind check. A stale-editor guard compares by document identity rather than editor object identity, so the listener works correctly when VS Code creates a fresh wrapper around the same document.

Target file scrolls to most-recently-viewed position
When the target file is re-revealed, it now scrolls to whichever position the user last viewed:

• If the user last scrolled in the diff pane -- target file mirrors that line.
• If the user last scrolled in the target file's own editor -- that position is used.
• Otherwise -- pre-edit scroll position is restored (existing behavior).

Tests: 59 tests pass (was 44). New coverage: userTouchedDiffEditor keep/close logic, programmatic-kind filter, stale-editor regression, scroll precedence (diff vs. target file vs. fallback), and diffScrollListener wiring.

[coderabbitai]

🧹 Nitpick comments (2)

.changeset/fix-diff-scroll-position.md (1)

5-5: 💤 Low value

Minor wording improvement on preview-tab preservation claim.

Line 5 reads "A first file that was open in a preview tab is restored" — "first file" is grammatically awkward. Consider: "A file that was open in a preview tab is restored" or "A preview tab that was open for a file is restored".

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.changeset/fix-diff-scroll-position.md at line 5, Replace the awkward phrase
"A first file that was open in a preview tab is restored" with clearer wording
such as "A file that was open in a preview tab is restored" (or alternatively "A
preview tab that was open for a file is restored") so the sentence reads
naturally and unambiguously.

src/integrations/editor/__tests__/DiffViewProvider.spec.ts (1)

1536-1536: 💤 Low value

Consider adding a guard for clearer test failures.

The non-null assertion on scrollCallback! will produce a confusing runtime error if the mock setup changes. Adding a defensive check would make failures more diagnosable.

Suggested improvement

-			return { scrollCallback: scrollCallback!, activeDiffEditor: (diffViewProvider as any).activeDiffEditor, fsPath }
+			if (!scrollCallback) {
+				throw new Error("onDidChangeTextEditorVisibleRanges mock was not invoked during open()")
+			}
+			return { scrollCallback, activeDiffEditor: (diffViewProvider as any).activeDiffEditor, fsPath }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/integrations/editor/__tests__/DiffViewProvider.spec.ts` at line 1536,
Replace the non-null assertion on scrollCallback in the test return with a
defensive check: before returning the object from the test helper, assert that
scrollCallback is defined (e.g. if (!scrollCallback) throw new
Error('scrollCallback not initialized - ensure mock setup sets it') or use
expect(scrollCallback).toBeDefined()), then return { scrollCallback,
activeDiffEditor: (diffViewProvider as any).activeDiffEditor, fsPath }; this
makes failures clearer and avoids a cryptic runtime error from the `!` operator.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In @.changeset/fix-diff-scroll-position.md:
- Line 5: Replace the awkward phrase "A first file that was open in a preview
tab is restored" with clearer wording such as "A file that was open in a preview
tab is restored" (or alternatively "A preview tab that was open for a file is
restored") so the sentence reads naturally and unambiguously.

In `@src/integrations/editor/__tests__/DiffViewProvider.spec.ts`:
- Line 1536: Replace the non-null assertion on scrollCallback in the test return
with a defensive check: before returning the object from the test helper, assert
that scrollCallback is defined (e.g. if (!scrollCallback) throw new
Error('scrollCallback not initialized - ensure mock setup sets it') or use
expect(scrollCallback).toBeDefined()), then return { scrollCallback,
activeDiffEditor: (diffViewProvider as any).activeDiffEditor, fsPath }; this
makes failures clearer and avoids a cryptic runtime error from the `!` operator.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 59610f4e-2eb5-4a1a-9dd5-8c49b849a4e4

📥 Commits

Reviewing files that changed from the base of the PR and between 6ea564c and 003a31e.

📒 Files selected for processing (3)

• .changeset/fix-diff-scroll-position.md
• src/integrations/editor/DiffViewProvider.ts
• src/integrations/editor/__tests__/DiffViewProvider.spec.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• src/integrations/editor/DiffViewProvider.ts

[edelauna]

thanks for this, had some comments related to the implementation.

[coderabbitai]

Actionable comments posted: 5

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@webview-ui/src/i18n/locales/de/settings.json`:
- Around line 988-998: Replace the ASCII transliterated German text with proper
German characters containing umlauts and special characters in the three
settings: autoCloseZooOpenedFiles, autoCloseZooOpenedFilesAfterUserEdited, and
autoCloseZooOpenedNewFiles. Specifically, replace `geoffnete` with `geöffnete`,
`schliessen` with `schließen`, `Anderung` with `Änderung`, `fuer` with `für`,
`voriibergehend` with `vorübergehend`, and `wahrend` with `während` throughout
all labels and descriptions to restore proper German readability and consistency
with the rest of the locale file.

In `@webview-ui/src/i18n/locales/fr/settings.json`:
- Around line 988-998: The French translations in the three newly added settings
entries (autoCloseZooOpenedFiles, autoCloseZooOpenedFilesAfterUserEdited, and
autoCloseZooOpenedNewFiles) are missing accents on multiple French words, making
them inconsistent with proper French spelling conventions. Restore the missing
diacritical marks: "activee" should be "activée", "apres" should be "après",
"deja" should be "déjà", "meme" should be "même", "crees" should be "créés",
"plutot" should be "plutôt", "modifies" should be "modifiés", "egalement" should
be "également", and "fermes" should be "fermés" in both the label and
description fields of these three settings entries.

In `@webview-ui/src/i18n/locales/it/settings.json`:
- Around line 992-994: In the description field of the
autoCloseZooOpenedFilesAfterUserEdited key in the Italian settings locale file,
correct two Italian grammatical errors: replace `e disabilitata` with `è
disabilitata` (adding the accent to the verb) and replace `gia` with `già`
(adding the accent to the adverb). These corrections ensure proper Italian
spelling and grammar in the user-facing description text.

In `@webview-ui/src/i18n/locales/ru/settings.json`:
- Around line 989-998: The Russian translation strings in the settings.json file
for keys including autoCloseZooOpenedFilesAfterUserEdited and
autoCloseZooOpenedNewFiles are written in Latin transliteration instead of
proper Cyrillic script, which creates poor user experience for Russian speakers.
Replace all transliterated label and description values with their proper
Cyrillic equivalents throughout this section to ensure the UI displays correct
Russian text.

In `@webview-ui/src/i18n/locales/vi/settings.json`:
- Around line 989-998: The Vietnamese translation strings for the new settings
entries (autoCloseZooOpenedFiles, autoCloseZooOpenedFilesAfterUserEdited, and
autoCloseZooOpenedNewFiles) are using unaccented transliteration instead of
proper Vietnamese orthography with diacritical marks. Replace all occurrences of
unaccented text like "Tu dong", "cac", "bat", "da", "mo", "se", "dong", "chap
nhan", "tu choi", "thay doi", "khong", "bao gio", "bi", "ngay ca", "sau khi",
"nguoi dung", "tuong tac", "nhan", "chuot", "go phim", "vao", "trong", "phien",
"chinh sua", "tat", "moi", "tao", "cua", and similar words with their properly
accented Vietnamese equivalents to ensure consistency with the rest of the
Vietnamese locale file.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 4a722397-2897-4b24-b877-27c9bd5e12f1

📥 Commits

Reviewing files that changed from the base of the PR and between 003a31e and 57fe83d.

📒 Files selected for processing (27)

• .changeset/add-editor-auto-close-settings.md
• packages/types/src/global-settings.ts
• packages/types/src/vscode-extension-host.ts
• src/integrations/editor/DiffViewProvider.ts
• src/integrations/editor/__tests__/DiffViewProvider.spec.ts
• webview-ui/src/components/settings/SettingsView.tsx
• webview-ui/src/components/settings/UISettings.tsx
• webview-ui/src/components/settings/__tests__/SettingsView.change-detection.spec.tsx
• webview-ui/src/components/settings/__tests__/UISettings.spec.tsx
• webview-ui/src/i18n/locales/ca/settings.json
• webview-ui/src/i18n/locales/de/settings.json
• webview-ui/src/i18n/locales/en/settings.json
• webview-ui/src/i18n/locales/es/settings.json
• webview-ui/src/i18n/locales/fr/settings.json
• webview-ui/src/i18n/locales/hi/settings.json
• webview-ui/src/i18n/locales/id/settings.json
• webview-ui/src/i18n/locales/it/settings.json
• webview-ui/src/i18n/locales/ja/settings.json
• webview-ui/src/i18n/locales/ko/settings.json
• webview-ui/src/i18n/locales/nl/settings.json
• webview-ui/src/i18n/locales/pl/settings.json
• webview-ui/src/i18n/locales/pt-BR/settings.json
• webview-ui/src/i18n/locales/ru/settings.json
• webview-ui/src/i18n/locales/tr/settings.json
• webview-ui/src/i18n/locales/vi/settings.json
• webview-ui/src/i18n/locales/zh-CN/settings.json
• webview-ui/src/i18n/locales/zh-TW/settings.json

✅ Files skipped from review due to trivial changes (10)

• webview-ui/src/i18n/locales/hi/settings.json
• webview-ui/src/i18n/locales/pl/settings.json
• webview-ui/src/i18n/locales/zh-CN/settings.json
• webview-ui/src/i18n/locales/en/settings.json
• .changeset/add-editor-auto-close-settings.md
• webview-ui/src/i18n/locales/pt-BR/settings.json
• webview-ui/src/i18n/locales/es/settings.json
• webview-ui/src/i18n/locales/id/settings.json
• webview-ui/src/i18n/locales/tr/settings.json
• webview-ui/src/i18n/locales/ca/settings.json

🚧 Files skipped from review as they are similar to previous changes (1)

• src/integrations/editor/DiffViewProvider.ts

[navedmerchant]

we need to investigate why so many agents create these changeset files lol, I saw it in other PRs too. I think it might be some instruction in the zoo repository

[navedmerchant]

figured it out, looks like they look at changesets in the repo and assume we need to generate one. We need to add instructions about it in agents.md, or just allow changesets in general

[navedmerchant]

Taking a look, @awschmeder can you see why CI is failing?

[navedmerchant]

Resolved @edelaunas comments in my commit, merging this, really improves the UX, but files almost flash before your eyes lol. We should promote the experimental feature of not opening files further