diff --git a/scanner/git.go b/scanner/git.go index 455b870..4ab2c75 100644 --- a/scanner/git.go +++ b/scanner/git.go @@ -272,21 +272,27 @@ func AnalyzeImpactFromAnalyses(changedFiles []FileInfo, analyses []FileAnalysis) } } - // Build impact info - var impacts []ImpactInfo + // Usage count descending, then full path ascending. usageCounts is a map, + // and a sort on count alone left equal counts in map order, so the diff + // impact footer and blast-radius JSON reordered between identical runs + // and an output budget then showed a different subset each time. The + // tie-break is the full path, not the rendered basename, so two changed + // files with the same name in different directories still order the same. + paths := make([]string, 0, len(usageCounts)) for file, count := range usageCounts { if count > 0 { - impacts = append(impacts, ImpactInfo{ - File: filepath.Base(file), - UsedBy: count, - }) + paths = append(paths, file) } } - - // Sort by usage count descending - sort.Slice(impacts, func(i, j int) bool { - return impacts[i].UsedBy > impacts[j].UsedBy + sort.Slice(paths, func(i, j int) bool { + if usageCounts[paths[i]] != usageCounts[paths[j]] { + return usageCounts[paths[i]] > usageCounts[paths[j]] + } + return paths[i] < paths[j] }) - + impacts := make([]ImpactInfo, 0, len(paths)) + for _, file := range paths { + impacts = append(impacts, ImpactInfo{File: filepath.Base(file), UsedBy: usageCounts[file]}) + } return impacts } diff --git a/scanner/impact_order_test.go b/scanner/impact_order_test.go new file mode 100644 index 0000000..f3db512 --- /dev/null +++ b/scanner/impact_order_test.go @@ -0,0 +1,44 @@ +package scanner + +import ( + "fmt" + "reflect" + "testing" +) + +// usageCounts is a map, and the sort was by count alone, so changed files with +// the same importer count came out in map order: the diff impact footer and +// blast-radius JSON reordered between identical runs, and an output budget +// then kept a different subset each time. +func TestAnalyzeImpactOrderIsDeterministicAcrossTies(t *testing.T) { + var changed []FileInfo + var analyses []FileAnalysis + for i := 0; i < 12; i++ { + name := fmt.Sprintf("src/c%02d.ts", i) + changed = append(changed, FileInfo{Path: name}) + // every changed file is imported by exactly two others: all tied + for j := 0; j < 2; j++ { + analyses = append(analyses, FileAnalysis{ + Path: fmt.Sprintf("src/user%02d_%d.ts", i, j), + Imports: []string{"./" + fmt.Sprintf("c%02d", i)}, + }) + } + } + // one file with more importers must lead + analyses = append(analyses, FileAnalysis{Path: "src/extra.ts", Imports: []string{"./c07"}}) + + first := AnalyzeImpactFromAnalyses(changed, analyses) + if len(first) != 12 || first[0].File != "c07.ts" || first[0].UsedBy != 3 { + t.Fatalf("impacts = %+v, want c07.ts (3) first", first) + } + for i := 2; i < len(first); i++ { + if first[i-1].UsedBy == first[i].UsedBy && first[i-1].File > first[i].File { + t.Fatalf("tie not broken by path: %s before %s", first[i-1].File, first[i].File) + } + } + for run := 1; run < 8; run++ { + if got := AnalyzeImpactFromAnalyses(changed, analyses); !reflect.DeepEqual(got, first) { + t.Fatalf("run %d = %+v, want %+v", run, got, first) + } + } +}