Skip to content

Commit c6f62ba

Browse files
code0xffclaude
andcommitted
fix: reset commit log on merge that interleaves commits past the prior head
The HEAD-refresh prepend path assumed the cached commit list was a contiguous suffix of the fresh first page. A merge can insert side-branch commits between the old head and the cached prefix, so prepending would corrupt revwalk order and break future skip pagination. The refresh now only prepends when the fresh page tail exactly matches the cached prefix; otherwise it resets to the fresh first page. Toggling back into Log mode now also compares the cached head against the latest observed HEAD and refreshes when they diverge, since Status mode does not keep the hidden commit list in sync. Two regression tests cover the no-ff merge case and the stale-Log-cache toggle path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent e8b3a68 commit c6f62ba

3 files changed

Lines changed: 139 additions & 32 deletions

File tree

src/app.rs

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -986,6 +986,45 @@ mod tests {
986986
);
987987
}
988988

989+
#[test]
990+
fn toggling_log_after_status_head_change_reloads_stale_cache() {
991+
let (_dir, path) = make_repo();
992+
run_git(&path, &["commit", "--allow-empty", "-m", "first"]);
993+
994+
let (snapshot, tx) = dummy_snapshot_channel();
995+
let mut app = App {
996+
snapshot,
997+
..app_with_files(vec![])
998+
};
999+
app.repo_path = path.clone();
1000+
app.mode = ViewMode::Status;
1001+
app.log_view
1002+
.set_commits(load_commit_log(&open_repo(&path), 500).unwrap());
1003+
app.last_head_oid = app.log_view.commits.first().map(|c| c.oid);
1004+
assert_eq!(app.log_view.commits[0].summary, "first");
1005+
1006+
run_git(&path, &["commit", "--allow-empty", "-m", "second"]);
1007+
tx.send(SnapshotMsg::Ok(snapshot_with_head(&path), HashMap::new()))
1008+
.unwrap();
1009+
app.poll_snapshot();
1010+
1011+
// Status mode leaves the hidden list untouched, but records the new
1012+
// HEAD. Entering Log mode must notice the mismatch and reconcile page 0
1013+
// rather than reusing the stale cached page as-is.
1014+
assert_eq!(app.log_view.commits.len(), 1);
1015+
assert_eq!(app.log_view.commits[0].summary, "first");
1016+
1017+
app.toggle_mode();
1018+
1019+
assert_eq!(app.mode, ViewMode::Log);
1020+
assert_eq!(app.log_view.commits.len(), 2);
1021+
assert_eq!(app.log_view.commits[0].summary, "second");
1022+
assert_eq!(app.log_view.selected, 1);
1023+
assert_eq!(app.log_view.commits[app.log_view.selected].summary, "first");
1024+
assert!(app.log_view.fully_loaded);
1025+
assert!(app.commit_log_page_rx.is_none());
1026+
}
1027+
9891028
#[test]
9901029
fn head_change_preserves_selected_commit_by_oid() {
9911030
let (_dir, path) = make_repo();
@@ -1956,6 +1995,52 @@ mod tests {
19561995
drop(dir);
19571996
}
19581997

1998+
#[test]
1999+
fn refresh_after_head_change_keeps_merged_side_branch_commits() {
2000+
let (dir, path) = make_repo();
2001+
std::fs::write(Path::new(&path).join("base"), "0").unwrap();
2002+
run_git(&path, &["add", "."]);
2003+
run_git(&path, &["commit", "-m", "c0"]);
2004+
2005+
run_git(&path, &["checkout", "-b", "feature"]);
2006+
std::fs::write(Path::new(&path).join("feature"), "feature").unwrap();
2007+
run_git(&path, &["add", "."]);
2008+
run_git(&path, &["commit", "-m", "feature"]);
2009+
2010+
run_git(&path, &["checkout", "-"]);
2011+
std::fs::write(Path::new(&path).join("main"), "main").unwrap();
2012+
run_git(&path, &["add", "."]);
2013+
run_git(&path, &["commit", "-m", "c1"]);
2014+
2015+
let mut app = app_with_files(vec![]);
2016+
app.repo_path = path.clone();
2017+
app.mode = ViewMode::Log;
2018+
app.log_view
2019+
.set_commits(load_commit_log(&open_repo(&path), 500).unwrap());
2020+
assert_eq!(app.log_view.commits.len(), 2);
2021+
assert_eq!(app.log_view.commits[0].summary, "c1");
2022+
2023+
run_git(
2024+
&path,
2025+
&["merge", "--no-ff", "feature", "-m", "merge feature"],
2026+
);
2027+
2028+
app.refresh_commit_log_after_head_change();
2029+
2030+
let summaries: Vec<_> = app
2031+
.log_view
2032+
.commits
2033+
.iter()
2034+
.map(|c| c.summary.as_str())
2035+
.collect();
2036+
assert!(
2037+
summaries.contains(&"feature"),
2038+
"merged side-branch commit was dropped: {summaries:?}"
2039+
);
2040+
assert_eq!(app.log_view.commits.len(), 4);
2041+
drop(dir);
2042+
}
2043+
19592044
#[test]
19602045
fn refresh_after_head_change_resets_on_divergence() {
19612046
let (dir, path) = make_repo();

src/app/diff_load.rs

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -284,19 +284,32 @@ impl App {
284284
};
285285

286286
// If the previous head still appears in the freshly fetched first
287-
// page, treat the change as a fast-forward / new commit: prepend the
288-
// newer entries onto the existing list so all accumulated pages stay
289-
// valid. Otherwise (rewrite, branch switch, force push, no prior list)
290-
// discard everything and start from the new first page.
287+
// page and the fresh tail lines up with the cached list, treat the
288+
// change as a fast-forward / simple new commit: prepend the newer
289+
// entries onto the existing list so all accumulated pages stay valid.
290+
// A merge can interleave side-branch commits after the old head; in
291+
// that case cached pages are no longer a contiguous prefix of the
292+
// new revwalk, so reset to the freshly loaded first page instead.
291293
let prepend_idx = prior_head_oid.and_then(|oid| page.iter().position(|c| c.oid == oid));
294+
let page_is_short = page.len() < page_size;
295+
let can_prepend = prepend_idx.is_some_and(|idx| {
296+
let fresh_tail = &page[idx..];
297+
!self.log_view.commits.is_empty()
298+
&& fresh_tail.len() <= self.log_view.commits.len()
299+
&& fresh_tail
300+
.iter()
301+
.zip(self.log_view.commits.iter())
302+
.all(|(fresh, cached)| fresh.oid == cached.oid)
303+
});
292304
if let Some(idx) = prepend_idx
293-
&& !self.log_view.commits.is_empty()
305+
&& can_prepend
294306
{
295307
let mut new_head_commits: Vec<_> = page.into_iter().take(idx).collect();
296308
let n_new = new_head_commits.len();
297-
new_head_commits.extend(self.log_view.commits.drain(..));
309+
new_head_commits.append(&mut self.log_view.commits);
298310
self.log_view.commits = new_head_commits;
299311
self.log_view.loaded_count = self.log_view.commits.len();
312+
self.log_view.fully_loaded = page_is_short;
300313
self.log_view.commit_width_cache.set(None);
301314
// Slide the selection so the user keeps looking at the same
302315
// commit even though new entries appeared above it.

src/app/focus.rs

Lines changed: 35 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -10,35 +10,44 @@ impl App {
1010
self.mode = ViewMode::Log;
1111
self.log_view.reset_drill_down();
1212
self.log_view.commit_scroll_x = 0;
13-
// Reuse cached pages on re-entry: refresh_commit_log_after_head_change
14-
// keeps the list in sync during Status mode, so a non-empty
15-
// `commits` already reflects the current HEAD.
16-
if self.log_view.commits.is_empty() {
17-
let page_size = self.cfg_commit_log_page_size;
18-
match self.with_repo(|repo| load_commit_log(repo, page_size)) {
19-
Ok(commits) => {
20-
// Short first page means the entire history fits,
21-
// no further prefetch needed; a full page means
22-
// more may exist and the next move will pull it.
23-
let fully_loaded = commits.len() < page_size;
24-
self.log_view.set_commits(commits);
25-
self.log_view.fully_loaded = fully_loaded;
26-
self.log_view.selected = 0;
27-
// Sync last_head_oid to the freshly loaded HEAD so
28-
// the next snapshot tick doesn't immediately
29-
// re-trigger refresh_commit_log_after_head_change.
30-
self.last_head_oid = self.log_view.commits.first().map(|c| c.oid);
31-
}
32-
Err(e) => {
33-
tracing::warn!(error = %e, "failed to load commit log");
34-
self.log_view.set_commits(Vec::new());
35-
self.log_view.selected = 0;
36-
self.status = Some(format!("git error: {e}"));
13+
// Reuse cached pages on re-entry only while they still match
14+
// the latest HEAD observed by the snapshot worker. Status mode
15+
// intentionally does not refresh the hidden commit list, so a
16+
// HEAD change there must invalidate the cache on the next entry.
17+
let cached_head = self.log_view.commits.first().map(|c| c.oid);
18+
let cache_matches_head =
19+
!self.log_view.commits.is_empty() && cached_head == self.last_head_oid;
20+
if !self.log_view.commits.is_empty() && !cache_matches_head {
21+
self.refresh_commit_log_after_head_change();
22+
} else {
23+
if self.log_view.commits.is_empty() {
24+
self.cancel_commit_log_page_fetch();
25+
let page_size = self.cfg_commit_log_page_size;
26+
match self.with_repo(|repo| load_commit_log(repo, page_size)) {
27+
Ok(commits) => {
28+
// Short first page means the entire history fits,
29+
// no further prefetch needed; a full page means
30+
// more may exist and the next move will pull it.
31+
let fully_loaded = commits.len() < page_size;
32+
self.log_view.set_commits(commits);
33+
self.log_view.fully_loaded = fully_loaded;
34+
self.log_view.selected = 0;
35+
// Sync last_head_oid to the freshly loaded HEAD so
36+
// the next snapshot tick doesn't immediately
37+
// re-trigger refresh_commit_log_after_head_change.
38+
self.last_head_oid = self.log_view.commits.first().map(|c| c.oid);
39+
}
40+
Err(e) => {
41+
tracing::warn!(error = %e, "failed to load commit log");
42+
self.log_view.set_commits(Vec::new());
43+
self.log_view.selected = 0;
44+
self.status = Some(format!("git error: {e}"));
45+
}
3746
}
3847
}
48+
self.load_commit_diff_for_selected();
49+
self.maybe_prefetch_commit_log();
3950
}
40-
self.load_commit_diff_for_selected();
41-
self.maybe_prefetch_commit_log();
4251
}
4352
ViewMode::Log => {
4453
self.mode = ViewMode::Status;

0 commit comments

Comments
 (0)