From 34c2c488261fe541f84cbe3dd1d4f876200123e5 Mon Sep 17 00:00:00 2001 From: Xiaoyang Tan Date: Wed, 3 Jun 2026 12:31:53 -0700 Subject: [PATCH] Fix NEW targets getting distance -1 when max_distance is set computeDistances() was resetting all non-DIRECT targets to -1, overwriting the distance 0 that getDefaultDistance correctly assigned to NEW targets. The BFS only seeded DIRECT targets, so NEW targets stayed at -1 and were dropped by filterChangedTargetsByDistance. Treat CHANGE_TYPE_NEW the same as CHANGE_TYPE_DIRECT in the BFS initialization: seed them at distance 0 so they survive distance filtering and propagate through reverse deps. --- controller/distance_filter.go | 4 +-- controller/getchangedtargets.go | 4 +-- controller/getchangedtargets_test.go | 53 ++++++++++++++++++++++++++++ proto/tango.proto | 6 ++-- 4 files changed, 60 insertions(+), 7 deletions(-) diff --git a/controller/distance_filter.go b/controller/distance_filter.go index 1b3f445..29fb243 100644 --- a/controller/distance_filter.go +++ b/controller/distance_filter.go @@ -41,8 +41,8 @@ func resolveMaxDistance(repoConfig config.RepositoryConfig, outputConfig *pb.Out // filterChangedTargetsByDistance returns targets where 0 <= distance <= maxDist. // Returns the input slice unchanged when maxDist < 0 (filtering disabled). -// Negative-distance targets (e.g. CHANGE_TYPE_NEW or unreachable from a -// DIRECT seed) are always dropped when the filter is active. +// Negative-distance targets (unreachable from a DIRECT/NEW seed) are always +// dropped when the filter is active. func filterChangedTargetsByDistance(targets []*pb.ChangedTarget, maxDist int32) []*pb.ChangedTarget { if maxDist < 0 { return targets diff --git a/controller/getchangedtargets.go b/controller/getchangedtargets.go index 6a057f8..28e8ccc 100644 --- a/controller/getchangedtargets.go +++ b/controller/getchangedtargets.go @@ -799,11 +799,11 @@ func computeDistances(logger *zap.Logger, changedByName map[string]*pb.ChangedTa } } - // initialize all distances to -1, means not set, DIRECT targets at 0. + // initialize all distances to -1, means not set, DIRECT and NEW targets at 0. var queue []string visited := make(map[string]struct{}, len(changedByName)) for name, ct := range changedByName { - if ct.GetChangeType() == pb.CHANGE_TYPE_DIRECT { + if ct.GetChangeType() == pb.CHANGE_TYPE_DIRECT || ct.GetChangeType() == pb.CHANGE_TYPE_NEW { ct.Distance = 0 queue = append(queue, name) visited[name] = struct{}{} diff --git a/controller/getchangedtargets_test.go b/controller/getchangedtargets_test.go index 8dfa125..8342bf3 100644 --- a/controller/getchangedtargets_test.go +++ b/controller/getchangedtargets_test.go @@ -967,6 +967,59 @@ func TestGetChangedTargets_CacheHitWithDistanceFilter(t *testing.T) { assert.NotNil(t, sent[1].GetMetadata()) } +func TestComputeDistances_NewTargetsGetDistanceZero(t *testing.T) { + // Graph: + // A (DIRECT) <- B (INDIRECT) + // N (NEW, no deps) + // + // NEW targets should be treated like DIRECT: distance 0, and seed BFS. + + meta := &pb.Metadata{ + TargetIdMapping: map[int32]string{ + 1: "A", 2: "B", 3: "N", + }, + } + + targetsByName := map[string]*pb.OptimizedTarget{ + "A": {Id: 1}, + "B": {Id: 2, DirectDependencies: []int32{1}}, + "N": {Id: 3}, + } + + changedByName := map[string]*pb.ChangedTarget{ + "A": {ChangeType: pb.CHANGE_TYPE_DIRECT}, + "B": {ChangeType: pb.CHANGE_TYPE_INDIRECT}, + "N": {ChangeType: pb.CHANGE_TYPE_NEW}, + } + + computeDistances(zap.NewNop(), changedByName, targetsByName, meta, -1) + + assert.Equal(t, int32(0), changedByName["A"].GetDistance(), "DIRECT target A should have distance 0") + assert.Equal(t, int32(1), changedByName["B"].GetDistance(), "B depends on DIRECT A, distance should be 1") + assert.Equal(t, int32(0), changedByName["N"].GetDistance(), "NEW target N should have distance 0") +} + +func TestComputeDistances_NewTargetsWithMaxDistance(t *testing.T) { + // When maxDistance is set, NEW targets should still get distance 0 + // and not be filtered out. + + meta := &pb.Metadata{ + TargetIdMapping: map[int32]string{1: "N"}, + } + + targetsByName := map[string]*pb.OptimizedTarget{ + "N": {Id: 1}, + } + + changedByName := map[string]*pb.ChangedTarget{ + "N": {ChangeType: pb.CHANGE_TYPE_NEW}, + } + + computeDistances(zap.NewNop(), changedByName, targetsByName, meta, 1) + + assert.Equal(t, int32(0), changedByName["N"].GetDistance(), "NEW target should have distance 0 even with maxDistance set") +} + func TestComputeDistances_NilMetadata(t *testing.T) { changedByName := map[string]*pb.ChangedTarget{ "A": {ChangeType: pb.CHANGE_TYPE_DIRECT}, diff --git a/proto/tango.proto b/proto/tango.proto index a641d1f..aa20fe2 100644 --- a/proto/tango.proto +++ b/proto/tango.proto @@ -119,9 +119,9 @@ message ChangedTarget { ChangeType change_type = 1; // Type of the change OptimizedTarget old_target = 2; // The old target before the change (first revision) OptimizedTarget new_target = 3; // The new target after the change (second revision) - // Distance from the nearest CHANGE_TYPE_DIRECT target in the reverse dependency graph. - // 0 means the target itself is DIRECT, 1 means it directly depends on a DIRECT target, etc. - // -1 means the distance is not set (e.g., for NEW targets or targets unreachable from DIRECT targets). + // Distance from the nearest CHANGE_TYPE_DIRECT or CHANGE_TYPE_NEW target in the reverse dependency graph. + // 0 means the target itself is DIRECT or NEW, 1 means it directly depends on such a target, etc. + // -1 means the distance is not set (e.g., targets unreachable from DIRECT/NEW targets). int32 distance = 4; }