Fix GetDependenciesAtCommit using lexicographic SHA comparison

andrew · andrew · commit e6eb2df769ba · 2026-02-05T19:04:10.000Z
The query used WHERE c.sha &lt;= ? to find the nearest snapshot, but SHA
hex strings don't sort in commit order. Replaced with a position-based
join through branch_commits, matching how GetDependenciesAtRef works.
diff --git a/internal/database/database_test.go b/internal/database/database_test.go
@@ -372,6 +372,93 @@ func TestStoreSnapshotWithDuplicates(t *testing.T) {
 	}
 }
 
+func TestGetDependenciesAtCommit(t *testing.T) {
+	tmpDir := t.TempDir()
+	dbPath := filepath.Join(tmpDir, "pkgs.sqlite3")
+
+	db, err := database.Create(dbPath)
+	if err != nil {
+		t.Fatalf("failed to create database: %v", err)
+	}
+	defer func() { _ = db.Close() }()
+
+	branch, err := db.GetOrCreateBranch("main")
+	if err != nil {
+		t.Fatalf("failed to create branch: %v", err)
+	}
+
+	// Create commits in order. SHA hex values are intentionally chosen so that
+	// lexicographic order differs from commit order:
+	//   commit 1 (position 1): SHA "ff0001" (lexicographically last)
+	//   commit 2 (position 2): SHA "000002" (lexicographically first)
+	//   commit 3 (position 3): SHA "880003" (lexicographically middle)
+	commits := []database.CommitInfo{
+		{SHA: "ff0001", Message: "first commit"},
+		{SHA: "000002", Message: "second commit"},
+		{SHA: "880003", Message: "third commit"},
+	}
+
+	// Store snapshot at commit 1 with lodash
+	err = db.StoreSnapshot(branch.ID, commits[0], []database.SnapshotInfo{
+		{ManifestPath: "package.json", Name: "lodash", Ecosystem: "npm", Requirement: "4.0.0"},
+	})
+	if err != nil {
+		t.Fatalf("failed to store snapshot 1: %v", err)
+	}
+
+	// Store commit 2 (no snapshot, just link it to the branch)
+	err = db.StoreSnapshot(branch.ID, commits[1], []database.SnapshotInfo{
+		{ManifestPath: "package.json", Name: "lodash", Ecosystem: "npm", Requirement: "4.0.0"},
+		{ManifestPath: "package.json", Name: "react", Ecosystem: "npm", Requirement: "18.0.0"},
+	})
+	if err != nil {
+		t.Fatalf("failed to store snapshot 2: %v", err)
+	}
+
+	// Query dependencies at commit 3 (no snapshot at this commit, should get
+	// the snapshot from commit 2 since it's earlier in position order, not
+	// commit 1 which would be wrong if using lexicographic SHA comparison)
+	err = db.StoreSnapshot(branch.ID, commits[2], []database.SnapshotInfo{
+		{ManifestPath: "package.json", Name: "lodash", Ecosystem: "npm", Requirement: "4.0.0"},
+		{ManifestPath: "package.json", Name: "react", Ecosystem: "npm", Requirement: "18.0.0"},
+		{ManifestPath: "package.json", Name: "express", Ecosystem: "npm", Requirement: "4.18.0"},
+	})
+	if err != nil {
+		t.Fatalf("failed to store snapshot 3: %v", err)
+	}
+
+	// Query at commit 2 -- should get the snapshot stored at commit 2 (position 2)
+	deps, err := db.GetDependenciesAtCommit("000002")
+	if err != nil {
+		t.Fatalf("GetDependenciesAtCommit failed: %v", err)
+	}
+
+	if len(deps) != 2 {
+		t.Fatalf("expected 2 dependencies at commit 2, got %d", len(deps))
+	}
+
+	names := map[string]bool{}
+	for _, d := range deps {
+		names[d.Name] = true
+	}
+	if !names["lodash"] || !names["react"] {
+		t.Errorf("expected lodash and react, got %v", names)
+	}
+
+	// Query at commit 1 -- should get only lodash (not react, which was added later)
+	deps, err = db.GetDependenciesAtCommit("ff0001")
+	if err != nil {
+		t.Fatalf("GetDependenciesAtCommit at commit 1 failed: %v", err)
+	}
+
+	if len(deps) != 1 {
+		t.Fatalf("expected 1 dependency at commit 1, got %d", len(deps))
+	}
+	if deps[0].Name != "lodash" {
+		t.Errorf("expected lodash at commit 1, got %s", deps[0].Name)
+	}
+}
+
 func TestSchemaIndexes(t *testing.T) {
 	tmpDir := t.TempDir()
 	dbPath := filepath.Join(tmpDir, "pkgs.sqlite3")
diff --git a/internal/database/queries.go b/internal/database/queries.go
@@ -246,15 +246,17 @@ type Dependency struct {
 }
 
 func (db *DB) GetDependenciesAtCommit(sha string) ([]Dependency, error) {
-	// Find the most recent snapshot at or before this commit
+	// Find the most recent snapshot at or before this commit using position
 	var commitID int64
 	err := db.QueryRow(`
 		SELECT ds.commit_id
 		FROM dependency_snapshots ds
-		JOIN commits c ON c.id = ds.commit_id
-		JOIN branch_commits bc ON bc.commit_id = c.id
-		WHERE c.sha <= ?
-		ORDER BY bc.position DESC
+		JOIN branch_commits snap_bc ON snap_bc.commit_id = ds.commit_id
+		JOIN commits c ON c.sha = ?
+		JOIN branch_commits target_bc ON target_bc.commit_id = c.id
+			AND target_bc.branch_id = snap_bc.branch_id
+		WHERE snap_bc.position <= target_bc.position
+		ORDER BY snap_bc.position DESC
 		LIMIT 1
 	`, sha).Scan(&commitID)
 	if err == sql.ErrNoRows {
