Avoid rehydrating unchanged rebase todos

Narrow rebase refreshes already have complete metadata for existing
todos. Reuse it so repeated todo moves do not spawn git show for the
entire list. New hashes still fall back to hydration.

pkg/commands/git_commands/commit_loader.go

@@ -174,7 +174,7 @@ func (self *CommitLoader) MergeRebasingCommits(hashPool *utils.StringPool, commi
 	}
 
 	if workingTreeState.Rebasing {
-		rebasingCommits, err := self.getHydratedRebasingCommits(hashPool, addConflictedRebasingCommit)
+		rebasingCommits, err := self.getHydratedRebasingCommits(hashPool, commits, addConflictedRebasingCommit)
 		if err != nil {
 			return nil, err
 		}
@@ -251,8 +251,8 @@ func (self *CommitLoader) extractCommitFromLine(hashPool *utils.StringPool, line
 	})
 }
 
-func (self *CommitLoader) getHydratedRebasingCommits(hashPool *utils.StringPool, addConflictingCommit bool) ([]*models.Commit, error) {
+func (self *CommitLoader) getHydratedRebasingCommits(hashPool *utils.StringPool, existingCommits []*models.Commit, addConflictingCommit bool) ([]*models.Commit, error) {
-	return self.getHydratedTodoCommits(hashPool, self.getRebasingCommits(hashPool, addConflictingCommit), false)
+	return self.getHydratedTodoCommits(hashPool, self.getRebasingCommits(hashPool, addConflictingCommit), existingCommits, false)
 }
 
 func (self *CommitLoader) getHydratedSequencerCommits(hashPool *utils.StringPool, workingTreeState models.WorkingTreeState) ([]*models.Commit, error) {
@@ -271,39 +271,56 @@ func (self *CommitLoader) getHydratedSequencerCommits(hashPool *utils.StringPool
 		}
 	}
 
-	return self.getHydratedTodoCommits(hashPool, commits, true)
+	return self.getHydratedTodoCommits(hashPool, commits, nil, true)
 }
 
-func (self *CommitLoader) getHydratedTodoCommits(hashPool *utils.StringPool, todoCommits []*models.Commit, todoFileHasShortHashes bool) ([]*models.Commit, error) {
+func (self *CommitLoader) getHydratedTodoCommits(
+	hashPool *utils.StringPool,
+	todoCommits []*models.Commit,
+	existingCommits []*models.Commit,
+	todoFileHasShortHashes bool,
+) ([]*models.Commit, error) {
 	if len(todoCommits) == 0 {
 		return nil, nil
 	}
 
-	commitHashes := lo.FilterMap(todoCommits, func(commit *models.Commit, _ int) (string, bool) {
+	// A refresh of only the rebasing todos should reuse the already loaded todos to avoid
-		return commit.Hash(), commit.Hash() != ""
+	// unnecessary git show calls.
-	})
-
-	// note that we're not filtering these as we do non-rebasing commits just because
-	// I suspect that will cause some damage
-	cmdObj := self.cmd.New(
-		NewGitCmd("show").
-			Config("log.showSignature=false").
-			Arg("--no-patch", "--oneline", "--abbrev=20", prettyFormat).
-			Arg(commitHashes...).
-			ToArgv(),
-	).DontLog()
-
 	fullCommits := map[string]*models.Commit{}
-	err := cmdObj.RunAndProcessLines(func(line string) (bool, error) {
+	for _, commit := range existingCommits {
-		if line == "" || line[0] != '+' {
+		if commit.IsTODO() && commit.Hash() != "" {
-			return false, nil
+			// Make a copy of the commit; that's necessary to avoid mutating the original commit
+			// when we later reuse it in the loop at the end of this function.
+			fullCommits[commit.Hash()] = lo.ToPtr(*commit)
 		}
-		commit := self.extractCommitFromLine(hashPool, line[1:], false)
+	}
-		fullCommits[commit.Hash()] = commit
+
-		return false, nil
+	commitHashesToFetch := lo.FilterMap(todoCommits, func(commit *models.Commit, _ int) (string, bool) {
+		return commit.Hash(), commit.Hash() != "" && fullCommits[commit.Hash()] == nil
 	})
-	if err != nil {
+
-		return nil, err
+	if len(commitHashesToFetch) > 0 {
+		// note that we're not filtering these as we do non-rebasing commits just because
+		// I suspect that will cause some damage
+		cmdObj := self.cmd.New(
+			NewGitCmd("show").
+				Config("log.showSignature=false").
+				Arg("--no-patch", "--oneline", "--abbrev=20", prettyFormat).
+				Arg(commitHashesToFetch...).
+				ToArgv(),
+		).DontLog()
+
+		err := cmdObj.RunAndProcessLines(func(line string) (bool, error) {
+			if line == "" || line[0] != '+' {
+				return false, nil
+			}
+			commit := self.extractCommitFromLine(hashPool, line[1:], false)
+			fullCommits[commit.Hash()] = commit
+			return false, nil
+		})
+		if err != nil {
+			return nil, err
+		}
 	}
 
 	findFullCommit := lo.Ternary(todoFileHasShortHashes,

pkg/commands/git_commands/commit_loader_test.go

@@ -538,6 +538,110 @@ func TestCommitLoader_getConflictedCommitImpl(t *testing.T) {
 	}
 }
 
+func TestCommitLoaderGetHydratedTodoCommitsReusesExistingCommit(t *testing.T) {
+	hashPool := &utils.StringPool{}
+	runner := oscommands.NewFakeRunner(t)
+	loader := &CommitLoader{
+		cmd: oscommands.NewDummyCmdObjBuilder(runner),
+	}
+	existingCommit := models.NewCommit(hashPool, models.NewCommitOpts{
+		Hash:          "0123456789012345678901234567890123456789",
+		Name:          "hydrated subject",
+		AuthorName:    "Jane Doe",
+		AuthorEmail:   "jane@example.com",
+		UnixTimestamp: 1234,
+		Parents:       []string{"1123456789012345678901234567890123456789"},
+		Status:        models.StatusRebasing,
+		Action:        todo.Pick,
+	})
+	refreshedTodo := models.NewCommit(hashPool, models.NewCommitOpts{
+		Hash:       existingCommit.Hash(),
+		Name:       "subject from the todo file",
+		Status:     models.StatusConflicted,
+		Action:     todo.Fixup,
+		ActionFlag: "-C",
+	})
+
+	commits, err := loader.getHydratedTodoCommits(
+		hashPool,
+		[]*models.Commit{refreshedTodo},
+		[]*models.Commit{existingCommit},
+		false,
+	)
+
+	assert.NoError(t, err)
+	assert.Equal(t, []*models.Commit{
+		models.NewCommit(hashPool, models.NewCommitOpts{
+			Hash:          existingCommit.Hash(),
+			Name:          "hydrated subject",
+			AuthorName:    "Jane Doe",
+			AuthorEmail:   "jane@example.com",
+			UnixTimestamp: 1234,
+			Parents:       []string{"1123456789012345678901234567890123456789"},
+			Status:        models.StatusConflicted,
+			Action:        todo.Fixup,
+			ActionFlag:    "-C",
+		}),
+	}, commits)
+	assert.Equal(t, todo.Pick, existingCommit.Action)
+	assert.Equal(t, models.StatusRebasing, existingCommit.Status)
+	runner.CheckForMissingCalls()
+}
+
+func TestCommitLoaderGetHydratedTodoCommitsLoadsMissingCommit(t *testing.T) {
+	hashPool := &utils.StringPool{}
+	existingHash := "0123456789012345678901234567890123456789"
+	missingHash := "2123456789012345678901234567890123456789"
+	missingCommitOutput := strings.ReplaceAll(
+		`+2123456789012345678901234567890123456789|1235|John Doe|john@example.com||>|tag: new|new subject`,
+		"|",
+		"\x00",
+	)
+	runner := oscommands.NewFakeRunner(t).ExpectGitArgs(
+		[]string{
+			"-c", "log.showSignature=false", "show", "--no-patch", "--oneline", "--abbrev=20",
+			prettyFormat, missingHash,
+		},
+		missingCommitOutput,
+		nil,
+	)
+	loader := &CommitLoader{
+		cmd: oscommands.NewDummyCmdObjBuilder(runner),
+	}
+	existingCommit := models.NewCommit(hashPool, models.NewCommitOpts{
+		Hash:   existingHash,
+		Name:   "existing subject",
+		Status: models.StatusRebasing,
+		Action: todo.Pick,
+	})
+	refreshedTodos := []*models.Commit{
+		models.NewCommit(hashPool, models.NewCommitOpts{
+			Hash:   existingHash,
+			Status: models.StatusRebasing,
+			Action: todo.Pick,
+		}),
+		models.NewCommit(hashPool, models.NewCommitOpts{
+			Hash:   missingHash,
+			Status: models.StatusRebasing,
+			Action: todo.Edit,
+		}),
+	}
+
+	commits, err := loader.getHydratedTodoCommits(
+		hashPool,
+		refreshedTodos,
+		[]*models.Commit{existingCommit},
+		false,
+	)
+
+	assert.NoError(t, err)
+	assert.Len(t, commits, 2)
+	assert.Equal(t, "existing subject", commits[0].Name)
+	assert.Equal(t, "new subject", commits[1].Name)
+	assert.Equal(t, todo.Edit, commits[1].Action)
+	runner.CheckForMissingCalls()
+}
+
 func TestCommitLoader_setCommitStatuses(t *testing.T) {
 	type scenario struct {
 		testName           string