Skip redundant restore fetch when all patches already applied

In the default (all-patches) case review-patch was fetching three
times: once for clean upstream, once to apply all patches, and once
more in the finally block to "restore" — even though the working tree
was already fully patched after the second fetch.

Track worktree_fully_patched after the non-interactive update call and
skip the finally fetch when the working tree is already at patch_count=-1.
The --count N and --interactive paths still re-fetch because the final
working-tree state differs from fully patched.

Update tests and the synthetic demo cast accordingly.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017zY8BoH65KBX6cz7Pm8aeF

Author: claude

dfetch/commands/review_patch.py

@@ -160,6 +160,7 @@ def _ignored() -> list[str]:
             assert isinstance(superproject, GitSuperProject)
             superproject.add_path(subproject.local_path)
 
+        worktree_fully_patched = False
         try:
             if interactive:
                 _step_tui(
@@ -177,6 +178,7 @@ def _ignored() -> list[str]:
                     patch_count=chosen_count,
                     eol_preferences_callback=superproject.eol_preferences,
                 )
+                worktree_fully_patched = chosen_count == -1
                 patch_label = (
                     str(total_patches) if chosen_count == -1 else str(chosen_count)
                 )
@@ -188,12 +190,13 @@ def _ignored() -> list[str]:
                 if is_tty():
                     input("Press Enter to restore...")
         finally:
-            subproject.update(
-                force=True,
-                ignored_files_callback=_ignored,
-                patch_count=-1,
-                eol_preferences_callback=superproject.eol_preferences,
-            )
+            if not worktree_fully_patched:
+                subproject.update(
+                    force=True,
+                    ignored_files_callback=_ignored,
+                    patch_count=-1,
+                    eol_preferences_callback=superproject.eol_preferences,
+                )
             if is_git:
                 assert isinstance(superproject, GitSuperProject)
                 superproject.restore_staged(subproject.local_path)

doc/asciicasts/review-patch.cast

@@ -14,45 +14,32 @@
 [7.45, "o", "\u001b[1;34mDfetch (0.14.0)\u001b[0m\r\n"]
 [7.49, "o", "  \u001b[1;92mcpputest:\u001b[0m\r\n"]
 [7.5, "o", "\u001b[?25l"]
-[7.58, "o", "\u001b[32m⠋\u001b[0m \u001b[1;94m> Fetching v3.4\u001b[0m"]
+[7.58, "o", "\u001b[32m⠇\u001b[0m \u001b[1;94m> Fetching v3.4\u001b[0m"]
 [7.66, "o", "\r\u001b[2K\u001b[32m⠙\u001b[0m \u001b[1;94m> Fetching v3.4\u001b[0m"]
 [7.74, "o", "\r\u001b[2K\u001b[32m⠹\u001b[0m \u001b[1;94m> Fetching v3.4\u001b[0m"]
 [7.82, "o", "\r\u001b[2K\u001b[32m⠸\u001b[0m \u001b[1;94m> Fetching v3.4\u001b[0m"]
 [7.9, "o", "\r\u001b[2K\u001b[32m⠼\u001b[0m \u001b[1;94m> Fetching v3.4\u001b[0m"]
 [7.98, "o", "\r\u001b[2K\u001b[32m⠴\u001b[0m \u001b[1;94m> Fetching v3.4\u001b[0m"]
 [8.06, "o", "\r\u001b[2K\u001b[32m⠦\u001b[0m \u001b[1;94m> Fetching v3.4\u001b[0m"]
-[8.14, "o", "\r\u001b[2K\u001b[32m⠇\u001b[0m \u001b[1;94m> Fetching v3.4\u001b[0m"]
+[8.14, "o", "\r\u001b[2K\u001b[32m⠧\u001b[0m \u001b[1;94m> Fetching v3.4\u001b[0m"]
 [8.22, "o", "\r\u001b[2K\u001b[32m⠏\u001b[0m \u001b[1;94m> Fetching v3.4\u001b[0m\r\n\u001b[?25h\r\u001b[1A\u001b[2K"]
 [8.23, "o", "  \u001b[1;34m> Fetched v3.4\u001b[0m\r\n"]
 [8.3, "o", "\u001b[?25l"]
-[8.38, "o", "\u001b[32m⠋\u001b[0m \u001b[1;94m> Fetching v3.4\u001b[0m"]
+[8.38, "o", "\u001b[32m⠇\u001b[0m \u001b[1;94m> Fetching v3.4\u001b[0m"]
 [8.46, "o", "\r\u001b[2K\u001b[32m⠙\u001b[0m \u001b[1;94m> Fetching v3.4\u001b[0m"]
 [8.54, "o", "\r\u001b[2K\u001b[32m⠹\u001b[0m \u001b[1;94m> Fetching v3.4\u001b[0m"]
 [8.62, "o", "\r\u001b[2K\u001b[32m⠸\u001b[0m \u001b[1;94m> Fetching v3.4\u001b[0m"]
 [8.7, "o", "\r\u001b[2K\u001b[32m⠼\u001b[0m \u001b[1;94m> Fetching v3.4\u001b[0m"]
 [8.78, "o", "\r\u001b[2K\u001b[32m⠴\u001b[0m \u001b[1;94m> Fetching v3.4\u001b[0m"]
 [8.86, "o", "\r\u001b[2K\u001b[32m⠦\u001b[0m \u001b[1;94m> Fetching v3.4\u001b[0m"]
-[8.94, "o", "\r\u001b[2K\u001b[32m⠇\u001b[0m \u001b[1;94m> Fetching v3.4\u001b[0m"]
+[8.94, "o", "\r\u001b[2K\u001b[32m⠧\u001b[0m \u001b[1;94m> Fetching v3.4\u001b[0m"]
 [9.02, "o", "\r\u001b[2K\u001b[32m⠏\u001b[0m \u001b[1;94m> Fetching v3.4\u001b[0m\r\n\u001b[?25h\r\u001b[1A\u001b[2K"]
 [9.03, "o", "  \u001b[1;34m> Fetched v3.4\u001b[0m\r\n"]
 [9.04, "o", "  \u001b[1;34m> Applying patch \"patches/cpputest.patch\"\u001b[0m\r\n"]
 [9.09, "o", "    \u001b[34msuccessfully patched 1/1:   \u001b[0m\u001b[34m \u001b[0m         \r\n\u001b[34mb'README.md'\u001b[0m                              \r\n"]
 [9.15, "o", "  \u001b[1;34m> stage = upstream, working tree = 1 patch(es) applied — open your editor and run `git diff` to inspect\u001b[0m\r\n"]
 [9.2, "o", "Press Enter to restore..."]
 [11.2, "o", "\r\n"]
-[11.3, "o", "\u001b[?25l"]
-[11.38, "o", "\u001b[32m⠋\u001b[0m \u001b[1;94m> Fetching v3.4\u001b[0m"]
-[11.46, "o", "\r\u001b[2K\u001b[32m⠙\u001b[0m \u001b[1;94m> Fetching v3.4\u001b[0m"]
-[11.54, "o", "\r\u001b[2K\u001b[32m⠹\u001b[0m \u001b[1;94m> Fetching v3.4\u001b[0m"]
-[11.62, "o", "\r\u001b[2K\u001b[32m⠸\u001b[0m \u001b[1;94m> Fetching v3.4\u001b[0m"]
-[11.7, "o", "\r\u001b[2K\u001b[32m⠼\u001b[0m \u001b[1;94m> Fetching v3.4\u001b[0m"]
-[11.78, "o", "\r\u001b[2K\u001b[32m⠴\u001b[0m \u001b[1;94m> Fetching v3.4\u001b[0m"]
-[11.86, "o", "\r\u001b[2K\u001b[32m⠦\u001b[0m \u001b[1;94m> Fetching v3.4\u001b[0m"]
-[11.94, "o", "\r\u001b[2K\u001b[32m⠇\u001b[0m \u001b[1;94m> Fetching v3.4\u001b[0m"]
-[12.02, "o", "\r\u001b[2K\u001b[32m⠏\u001b[0m \u001b[1;94m> Fetching v3.4\u001b[0m\r\n\u001b[?25h\r\u001b[1A\u001b[2K"]
-[12.03, "o", "  \u001b[1;34m> Fetched v3.4\u001b[0m\r\n"]
-[12.04, "o", "  \u001b[1;34m> Applying patch \"patches/cpputest.patch\"\u001b[0m\r\n"]
-[12.09, "o", "    \u001b[34msuccessfully patched 1/1:   \u001b[0m\u001b[34m \u001b[0m         \r\n\u001b[34mb'README.md'\u001b[0m                              \r\n"]
-[12.15, "o", "  \u001b[1;34m> restored\u001b[0m\r\n"]
-[12.2, "o", "$ "]
-[15.2, "o", ""]
+[11.25, "o", "  \u001b[1;34m> restored\u001b[0m\r\n"]
+[11.3, "o", "$ "]
+[14.3, "o", ""]

tests/test_review_patch.py

@@ -63,16 +63,14 @@ def test_review_all_patches_calls_update_add_path_update():
                     cmd(_make_args())
 
     update_calls = fake_sub.update.call_args_list
+    assert len(update_calls) == 2, "all-patches case must only fetch twice (no redundant restore fetch)"
     assert update_calls[0] == call(
         force=True, ignored_files_callback=ANY, patch_count=0, eol_preferences_callback=ANY
     ), "first call must fetch clean upstream"
     fake_super.add_path.assert_called_once_with("my_project")
     assert update_calls[1] == call(
         force=True, ignored_files_callback=ANY, patch_count=-1, eol_preferences_callback=ANY
-    ), "second call (inside try) applies all patches"
-    assert update_calls[2] == call(
-        force=True, ignored_files_callback=ANY, patch_count=-1, eol_preferences_callback=ANY
-    ), "third call (finally) restores all patches"
+    ), "second call applies all patches; working tree is already restored so no third fetch"
     fake_super.restore_staged.assert_called_once_with("my_project")
 
 
@@ -116,7 +114,7 @@ def test_svn_superproject_warns_and_skips_staging():
     mock_log.warning.assert_called_once()
     fake_super.add_path.assert_not_called()
     fake_super.restore_staged.assert_not_called()
-    assert fake_sub.update.call_count == 3
+    assert fake_sub.update.call_count == 2
 
 
 # ---------------------------------------------------------------------------