Artifact [c7eb7decf1]

Artifact c7eb7decf1b4b3f40f619e353c4025fd1a97b96338d616f533b28125b89686a9:

• File internal/index/branchcommits_test.go — part of check-in [21fde3240d] at 2026-08-26 12:48:22 on branch landings-batch — A landing's commits in ancestry order, not date order

  The page listed the branch through the timeline, which orders by (date DESC, hash DESC). Two check-ins in the same second tie on date and then order by hash, which has nothing to do with which is the parent. Commits land back-to-back here routinely.

  zit hit this in landing show and its fix is the one to inherit: the verb owns no ancestry ordering of its own. A recursive walk from the branch tip, ordered by longest path back, stopping where the branch diverges from its target.

  The test commits three times in one second with hashes whose order is the reverse of ancestry — the old sort gets it exactly backwards and looks plausible doing it. (user: dmestas, size: 4596) [annotate] [blame] [check-ins using] [more...]

package index

import (
	"testing"
)

// ci records a check-in with an explicit parent chain.
func ci(t *testing.T, ix *Index, hash, branch, date, comment string, parents ...string) {
	t.Helper()
	if _, err := ix.db.Exec(`INSERT OR REPLACE INTO checkin (hash, date, user, comment, branch)
		VALUES (?, ?, 'dan', ?, ?)`, hash, date, comment, branch); err != nil {
		t.Fatal(err)
	}
	for i, p := range parents {
		if _, err := ix.db.Exec(`INSERT OR REPLACE INTO checkin_parent (child, parent, ord)
			VALUES (?, ?, ?)`, hash, p, i); err != nil {
			t.Fatal(err)
		}
	}
}

func order(cs []BranchCommit) []string {
	out := make([]string, len(cs))
	for i, c := range cs {
		out[i] = c.Hash
	}
	return out
}

// The test that proves it, and the reason zit caught this while review
// did not: two check-ins in the SAME SECOND tie on date and then order
// by hash, which has nothing to do with which is the parent.
//
// The hashes here are chosen so that hash order is the REVERSE of
// ancestry order — a date sort with a hash tiebreak gets this exactly
// backwards, and looks entirely plausible doing it.
func TestBranchCommitsOrderByAncestryNotDate(t *testing.T) {
	ix := openIndex(t)
	ci(t, ix, "zzz1", "trunk", "2026-01-01T10:00:00", "base")
	ci(t, ix, "ccc1", "feat", "2026-01-02T10:00:00", "first on the branch", "zzz1")
	ci(t, ix, "bbb2", "feat", "2026-01-02T10:00:00", "second, same second", "ccc1")
	ci(t, ix, "aaa3", "feat", "2026-01-02T10:00:00", "third, same second", "bbb2")

	got, err := ix.BranchCommits("feat", 50)
	if err != nil {
		t.Fatal(err)
	}
	want := []string{"ccc1", "bbb2", "aaa3"}
	have := order(got)
	if len(have) != len(want) {
		t.Fatalf("got %v, want %v", have, want)
	}
	for i := range want {
		if have[i] != want[i] {
			t.Fatalf("ancestry order is %v, want %v — sorted by something\n"+
				"other than the DAG", have, want)
		}
	}
}

// A parent must precede its child. That is the whole invariant, and it
// has to hold whatever the dates say — including when a clock went
// backwards between two commits.
func TestBranchCommitsKeepParentsFirstAcrossNonMonotoneDates(t *testing.T) {
	ix := openIndex(t)
	ci(t, ix, "zzz1", "trunk", "2026-01-01T10:00:00", "base")
	ci(t, ix, "p", "feat", "2026-01-05T10:00:00", "parent, later clock", "zzz1")
	ci(t, ix, "c", "feat", "2026-01-02T10:00:00", "child, earlier clock", "p")

	got, _ := ix.BranchCommits("feat", 50)
	have := order(got)
	if len(have) != 2 || have[0] != "p" || have[1] != "c" {
		t.Fatalf("got %v, want [p c] — a child sorted ahead of its parent", have)
	}
}

// The walk stops where the branch does: shared history is the target's,
// not the proposal's.
func TestBranchCommitsExcludeSharedHistory(t *testing.T) {
	ix := openIndex(t)
	ci(t, ix, "t1", "trunk", "2026-01-01T10:00:00", "trunk one")
	ci(t, ix, "t2", "trunk", "2026-01-01T11:00:00", "trunk two", "t1")
	ci(t, ix, "f1", "feat", "2026-01-02T10:00:00", "branch one", "t2")

	got, _ := ix.BranchCommits("feat", 50)
	if have := order(got); len(have) != 1 || have[0] != "f1" {
		t.Fatalf("got %v, want only the branch's own commit", have)
	}
}

// A branch that forked and rejoined: the merge's depth must be the
// LONGEST path back, or a commit on the short side outranks a child it
// is an ancestor of.
func TestBranchCommitsHandleAForkAndRejoin(t *testing.T) {
	ix := openIndex(t)
	ci(t, ix, "t1", "trunk", "2026-01-01T10:00:00", "base")
	ci(t, ix, "f1", "feat", "2026-01-02T10:00:00", "root", "t1")
	ci(t, ix, "f2", "feat", "2026-01-02T10:00:00", "left", "f1")
	ci(t, ix, "f3", "feat", "2026-01-02T10:00:00", "left again", "f2")
	ci(t, ix, "f4", "feat", "2026-01-02T10:00:00", "right", "f1")
	ci(t, ix, "f5", "feat", "2026-01-02T10:00:00", "rejoin", "f3", "f4")

	got, _ := ix.BranchCommits("feat", 50)
	pos := map[string]int{}
	for i, c := range got {
		pos[c.Hash] = i
	}
	if len(pos) != 5 {
		t.Fatalf("got %d commits, want 5: %v", len(pos), order(got))
	}
	for _, pair := range [][2]string{{"f1", "f2"}, {"f2", "f3"}, {"f1", "f4"},
		{"f3", "f5"}, {"f4", "f5"}} {
		if pos[pair[0]] >= pos[pair[1]] {
			t.Errorf("%s does not precede %s: %v", pair[0], pair[1], order(got))
		}
	}
}

// An empty or unknown branch is not an error; a landing can name a
// branch whose check-ins have not been folded yet.
func TestBranchCommitsToleratesAnUnknownBranch(t *testing.T) {
	ix := openIndex(t)
	got, err := ix.BranchCommits("nothing-here", 50)
	if err != nil || len(got) != 0 {
		t.Fatalf("got %v err=%v", got, err)
	}
	if got, err := ix.BranchCommits("", 50); err != nil || got != nil {
		t.Fatalf("empty branch: %v err=%v", got, err)
	}
}