Address PR comments

Author: bouwkast

.github/workflows/auto_bump_test_package_versions.yml

@@ -46,7 +46,7 @@ jobs:
         shell: pwsh
         run: |
           $report = ""
-          $reportPath = "tracer/build/cooldown_report.md"
+          $reportPath = ".nuke/temp/cooldown_report.md"
           if (Test-Path $reportPath) {
             $report = Get-Content $reportPath -Raw
           }

.gitignore

@@ -405,6 +405,3 @@ tools/dumps/
 
 # test optimization temp/run folder
 .dd/
-
-# Generated by GeneratePackageVersions target (consumed by CI workflow, not committed)
-tracer/build/cooldown_report.md

tracer/build/_build/Build.Utilities.cs

@@ -59,8 +59,8 @@ partial class Build
     [Parameter("Only update package versions for packages with the following names")]
     readonly string[] IncludePackages;
 
-    [Parameter("Minimum age in days a NuGet package version must have been published before auto-including (default: 0, no filtering)")]
-    readonly int PackageVersionCooldownDays;
+    [Parameter("Minimum age in days a NuGet package version must have been published before auto-including (default: 2)")]
+    readonly int PackageVersionCooldownDays = 2;
 
     [LazyLocalExecutable(@"C:\Program Files (x86)\Microsoft SDKs\Windows\v10.0A\bin\NETFX 4.8 Tools\gacutil.exe")]
     readonly Lazy<Tool> GacUtil;
@@ -323,7 +323,7 @@ partial class Build
                        resolvedText);
                }
 
-               var reportPath = BuildDirectory / "cooldown_report.md";
+               var reportPath = TemporaryDirectory / "cooldown_report.md";
                await versionGenerator.CooldownReport.SaveToFile(reportPath);
                Logger.Information("Cooldown report saved to {Path}", reportPath);
            }

tracer/build/_build/GeneratePackageVersions/NuGetVersionCache.cs

@@ -25,7 +25,6 @@ public static class NuGetVersionCache
 
     /// <summary>
     /// Load the version cache from disk. Returns an empty dictionary if the file doesn't exist.
-    /// Handles both the old format (plain version strings) and the new format (version + publish date).
     /// </summary>
     public static async Task<Dictionary<string, List<VersionWithDate>>> Load(string path)
     {
@@ -36,30 +35,10 @@ public static async Task<Dictionary<string, List<VersionWithDate>>> Load(string
 
         await using var openStream = File.OpenRead(path);
 
-        // Try new format first: List<KeyValuePair<string, List<VersionWithDate>>>
-        try
-        {
-            var result = await JsonSerializer.DeserializeAsync<List<KeyValuePair<string, List<VersionWithDate>>>>(openStream, JsonOptions);
-            if (result is not null)
-            {
-                return new Dictionary<string, List<VersionWithDate>>(result);
-            }
-        }
-        catch (JsonException)
-        {
-            // Fall through to old format
-        }
-
-        // Reset stream position for retry with old format
-        openStream.Position = 0;
-
-        // Old format: List<KeyValuePair<string, List<string>>>
-        var legacy = await JsonSerializer.DeserializeAsync<List<KeyValuePair<string, List<string>>>>(openStream, JsonOptions)
-                     ?? new List<KeyValuePair<string, List<string>>>();
+        var result = await JsonSerializer.DeserializeAsync<List<KeyValuePair<string, List<VersionWithDate>>>>(openStream, JsonOptions)
+                     ?? new List<KeyValuePair<string, List<VersionWithDate>>>();
 
-        return legacy.ToDictionary(
-            kvp => kvp.Key,
-            kvp => kvp.Value.Select(v => new VersionWithDate(v, null)).ToList());
+        return new Dictionary<string, List<VersionWithDate>>(result);
     }
 
     /// <summary>
@@ -74,8 +53,7 @@ public static async Task Save(string path, Dictionary<string, List<VersionWithDa
                 x.Key,
                 x.Value
                     .Select(v => v with { Version = Version.Parse(v.Version).ToString() })
-                    .OrderBy(v => Version.Parse(v.Version))
-                    .ToList()));
+                    .OrderBy(v => Version.Parse(v.Version))));
         await using var createStream = File.Create(path);
         await JsonSerializer.SerializeAsync(createStream, ordered, JsonOptions);
     }

tracer/build/_build/GeneratePackageVersions/PackageVersionGenerator.cs

@@ -122,18 +122,18 @@ where IsSupported(entry, version.ToString(), framework)
                     select (version, framework))
                    .ToList();
 
+                // Apply cooldown once on the full version list before selection.
+                // This removes versions within the cooldown period (unless already at/below baseline)
+                // so that SelectMax/SelectPackagesFromGlobs never see them.
+                orderedWithFramework = ApplyCooldown(entry, orderedWithFramework, publishDateLookup);
+
                 // Add the last for every minor
                 var latestMajors = SelectMax(orderedWithFramework, v => v.Major).ToList();
                 var latestMinors = SelectMax(orderedWithFramework, v => $"{v.Major}.{v.Minor}").ToList();
                 var latestSpecific = entry.SpecificVersions.Length == 0
                     ? latestMajors
                     : SelectPackagesFromGlobs(orderedWithFramework, entry.SpecificVersions).ToList();
 
-                // Apply cooldown: override versions that are too new with older resolved versions
-                latestMajors = ApplyCooldown(entry, latestMajors, orderedWithFramework, publishDateLookup, v => v.Major).ToList();
-                latestMinors = ApplyCooldown(entry, latestMinors, orderedWithFramework, publishDateLookup, v => $"{v.Major}.{v.Minor}").ToList();
-                latestSpecific = ApplyCooldown(entry, latestSpecific, orderedWithFramework, publishDateLookup, v => v.Major).ToList();
-
                 _latestMinors.Write(entry, latestMinors, requiresDockerDependency);
                 _latestMajors.Write(entry, latestMajors, requiresDockerDependency);
                 _latestSpecific.Write(entry, latestSpecific, requiresDockerDependency);
@@ -164,90 +164,59 @@ where IsSupported(entry, version.ToString(), framework)
         }
 
         /// <summary>
-        /// Applies cooldown filtering to selected versions. For each version that was published
+        /// Applies cooldown filtering to the full ordered version list. For each version published
         /// within the cooldown period, checks if it was already accepted (at or below baseline).
-        /// If already accepted, keeps it. Otherwise, finds the best resolved version that is
-        /// either outside the cooldown or at the baseline. Overridden versions are added to CooldownReport.
+        /// If already accepted, keeps it. Otherwise, removes it so that downstream selection
+        /// (SelectMax/SelectPackagesFromGlobs) naturally picks the next-best version.
+        /// Removed versions are recorded in CooldownReport.
         /// </summary>
-        private List<(TargetFramework framework, IEnumerable<Version> versions)> ApplyCooldown<T>(
+        private List<(Version version, TargetFramework framework)> ApplyCooldown(
             PackageVersionEntry entry,
-            List<(TargetFramework framework, IEnumerable<Version> versions)> selectedVersions,
-            List<(Version version, TargetFramework framework)> allOrderedVersions,
-            Dictionary<string, DateTimeOffset?> publishDateLookup,
-            Func<Version, T> groupBy)
+            List<(Version version, TargetFramework framework)> orderedVersions,
+            Dictionary<string, DateTimeOffset?> publishDateLookup)
         {
             _baseline.TryGetValue(entry.NugetPackageSearchName, out var baselineVersion);
 
-            var result = new List<(TargetFramework framework, IEnumerable<Version> versions)>();
+            var result = new List<(Version version, TargetFramework framework)>();
 
-            foreach (var (framework, versions) in selectedVersions)
+            foreach (var (version, framework) in orderedVersions)
             {
-                // All versions available for this framework, grouped the same way as selection
-                var allForFramework = allOrderedVersions
-                    .Where(x => x.framework == framework)
-                    .Select(x => x.version)
-                    .ToList();
+                var versionKey = version.ToString();
+                publishDateLookup.TryGetValue(versionKey, out var publishedDate);
 
-                var filteredVersions = new List<Version>();
-                foreach (var version in versions)
+                if (!IsWithinCooldown(publishedDate))
                 {
-                    var versionKey = version.ToString();
-                    publishDateLookup.TryGetValue(versionKey, out var publishedDate);
-
-                    if (!IsWithinCooldown(publishedDate))
-                    {
-                        // Outside cooldown -- always accept
-                        filteredVersions.Add(version);
-                        continue;
-                    }
+                    // Outside cooldown -- always accept
+                    result.Add((version, framework));
+                    continue;
+                }
 
-                    if (baselineVersion is not null && version <= baselineVersion)
-                    {
-                        // Within cooldown but already accepted in a previous run -- keep it
-                        filteredVersions.Add(version);
-                        continue;
-                    }
-
-                    // Within cooldown and above baseline -- override to the best available:
-                    // the baseline version if it exists in this group, otherwise the latest
-                    // version outside cooldown
-                    var groupKey = groupBy(version);
-                    var versionsInGroup = allForFramework
-                        .Where(v => EqualityComparer<T>.Default.Equals(groupBy(v), groupKey))
-                        .Where(v => v < version)
-                        .OrderByDescending(v => v);
-
-                    // Prefer baseline version if it's in this group
-                    Version resolved = null;
-                    if (baselineVersion is not null)
-                    {
-                        resolved = versionsInGroup.FirstOrDefault(v => v <= baselineVersion);
-                    }
+                if (baselineVersion is not null && version <= baselineVersion)
+                {
+                    // Within cooldown but already accepted in a previous run -- keep it
+                    result.Add((version, framework));
+                    continue;
+                }
 
-                    // Fall back to latest outside cooldown
-                    resolved ??= versionsInGroup.FirstOrDefault(v =>
+                // Within cooldown and above baseline -- remove from the list.
+                // The best fallback is whatever SelectMax ends up picking from the remaining versions.
+                // Find what that would be for reporting purposes.
+                var fallback = orderedVersions
+                    .Where(v => v.framework == framework && v.version < version)
+                    .Select(v => v.version)
+                    .OrderByDescending(v => v)
+                    .FirstOrDefault(v =>
                     {
-                        publishDateLookup.TryGetValue(v.ToString(), out var rDate);
-                        return !IsWithinCooldown(rDate);
+                        publishDateLookup.TryGetValue(v.ToString(), out var d);
+                        return !IsWithinCooldown(d) || (baselineVersion is not null && v <= baselineVersion);
                     });
 
-                    CooldownReport.Add(new CooldownReport.CooldownEntry(
-                        entry.NugetPackageSearchName,
-                        entry.IntegrationName,
-                        versionKey,
-                        publishedDate,
-                        resolved?.ToString()));
-
-                    if (resolved is not null)
-                    {
-                        filteredVersions.Add(resolved);
-                    }
-                }
-
-                if (filteredVersions.Count > 0)
-                {
-                    result.Add((framework, filteredVersions.Distinct().OrderBy(v => v).ToList()));
-                }
+                CooldownReport.Add(new CooldownReport.CooldownEntry(
+                    entry.NugetPackageSearchName,
+                    entry.IntegrationName,
+                    versionKey,
+                    publishedDate,
+                    fallback?.ToString()));
             }
 
             return result;