Skip to content

Fix: Use BFS to collect all commits in walk_commits_to_base for --rebase-merges support#331

Closed
txf0096 wants to merge 1 commit intogit-ai-project:mainfrom
txf0096:fix/bfs-walk-commits-clean
Closed

Fix: Use BFS to collect all commits in walk_commits_to_base for --rebase-merges support#331
txf0096 wants to merge 1 commit intogit-ai-project:mainfrom
txf0096:fix/bfs-walk-commits-clean

Conversation

@txf0096
Copy link
Copy Markdown
Contributor

@txf0096 txf0096 commented Jan 7, 2026

Fix: Use BFS to collect all commits in walk_commits_to_base for --rebase-merges support

Problem

The walk_commits_to_base function only follows parent(0), causing commits from merged branches to be lost during git rebase --rebase-merges. This results in AI authorship data being lost for side branch commits.

Fixes the failing test_rebase_preserve_merges integration test.

Solution

Replace linear traversal with BFS (Breadth-First Search) to traverse all parent commits, ensuring merge commits' second parents are not missed.

Changes

File: src/authorship/rebase_authorship.rs

Before (Lines 823-841):

pub fn walk_commits_to_base(...) {
    let mut commits = Vec::new();
    let mut current = repository.find_commit(head.to_string())?;
    let base_str = base.to_string();

    while current.id().to_string() != base_str {
        commits.push(current.id().to_string());
        current = current.parent(0)?;  // ❌ Only follows first parent
    }

    Ok(commits)
}

After:

pub fn walk_commits_to_base(...) {
    use std::collections::{HashSet, VecDeque};

    let mut commits = Vec::new();
    let mut visited = HashSet::new();
    let mut queue = VecDeque::new();

    let base_str = base.to_string();
    let head_commit = repository.find_commit(head.to_string())?;

    queue.push_back(head_commit);

    while let Some(current) = queue.pop_front() {
        let commit_id = current.id().to_string();

        if commit_id == base_str {
            continue;
        }

        if visited.contains(&commit_id) {
            continue;
        }

        visited.insert(commit_id.clone());
        commits.push(commit_id);

        // ✅ Traverse all parents (handles merge commits)
        let parent_count = current.parent_count().unwrap_or(0);
        for i in 0..parent_count {
            if let Ok(parent) = current.parent(i) {
                queue.push_back(parent);
            }
        }
    }

    Ok(commits)
}

Test Results

Before

$ cargo test test_rebase_preserve_merges
test test_rebase_preserve_merges ... FAILED

Line 1: Expected AI author but got 'Test User'
Expected: ExpectedLine { contents: "// AI side", author_type: Ai }
Actual content: "// AI side"
Full blame output:
f68de6d (Test User 2026-01-07 21:57:30 +0800 1) // AI side

After

$ cargo test test_rebase_preserve_merges
test test_rebase_preserve_merges ... ok

test result: ok. 1 passed; 0 failed

Impact

  • ✅ Fixes --rebase-merges authorship tracking
  • ✅ All existing tests pass
  • ✅ Zero impact on regular rebase (no merge commits)
  • ✅ Fully backward compatible

Checklist

  • Tests pass locally
  • No breaking changes
  • Fixes failing integration test
  • Code follows existing patterns

Additional Notes

This is a critical fix for users relying on git rebase --rebase-merges to preserve merge commit structure. Without this fix, AI authorship data is silently lost for all commits in merged branches.

…ase-merges support

The walk_commits_to_base function was only following parent(0), causing it to
miss commits from merged branches when using git rebase --rebase-merges. This
resulted in AI authorship being lost for side branch commits.

Changed from linear traversal (parent(0) only) to BFS (Breadth-First Search)
that traverses all parents, ensuring merge commits' second parents are not
missed.

Fixes #328
@CLAassistant
Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


shawn.tian seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@txf0096 txf0096 closed this Jan 7, 2026
@txf0096 txf0096 deleted the fix/bfs-walk-commits-clean branch January 7, 2026 18:21
@txf0096 txf0096 restored the fix/bfs-walk-commits-clean branch January 7, 2026 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants