refactor(deploy): evaluator groups instances by site once; doc corrections

Tallying filtered the whole deployed-instance set inside the site loop, making the
aggregate O(sites x instances) — the one per-entity loop in an otherwise batched
pass. Group once into a lookup and index per site. Also: ResolveArtifactState's
missing-hash branch is a defensive default, not the race the comment described
(the same site list feeds the hash computation), and the staleness batch omits an
instance undeployed mid-batch as well as one that never had a snapshot.

Claude-Session: https://claude.ai/code/session_01Y5ef34iUUMjmwnMpL9xgmB
This commit is contained in:
Joseph Doherty
2026-09-14 13:55:26 -04:00
parent ada91bb82c
commit 3d22f272d3
@@ -78,10 +78,15 @@ public sealed class PendingUpdatesEvaluator : IPendingUpdatesEvaluator
var staleness = await _deploymentService.GetInstanceStalenessAsync(
deployedInstances.Select(i => i.Id).ToList(), cancellationToken);
// Grouped ONCE, not re-scanned per site: tallying by filtering the whole
// deployed set inside the site loop would make the aggregate O(sites x
// instances) — the one genuinely per-entity loop in an otherwise batched pass.
var instancesBySite = deployedInstances.ToLookup(i => i.SiteId);
var rows = new List<SitePendingUpdates>(sites.Count);
foreach (var site in sites.OrderBy(s => s.Name, StringComparer.OrdinalIgnoreCase))
{
var (deployed, stale, unknown) = TallyInstances(site.Id, deployedInstances, staleness);
var (deployed, stale, unknown) = TallyInstances(instancesBySite[site.Id], staleness);
rows.Add(new SitePendingUpdates(
site.Id,
site.SiteIdentifier,
@@ -114,9 +119,16 @@ public sealed class PendingUpdatesEvaluator : IPendingUpdatesEvaluator
/// Decides a site's artifact state by comparing the hash stamped by its last
/// successful artifact deploy against the hash the current definitions would
/// produce. A site with no stamp has never had artifacts deployed, which is
/// <see cref="ArtifactDeploymentState.NeverDeployed"/>, NOT out of date. A site
/// missing from the computed map (it appeared between the two reads) is reported
/// out of date, the side that prompts an operator to look rather than hiding it.
/// <see cref="ArtifactDeploymentState.NeverDeployed"/>, NOT out of date.
///
/// <para>
/// A stamped site missing from the computed map is reported out of date. That is a
/// defensive default, not a reachable race: this call path hands the SAME site list
/// to <see cref="ArtifactDeploymentService.ComputeCurrentArtifactHashesAsync"/>,
/// which sets an entry for every input site or throws. It is kept so that a future
/// producer of a partial map fails toward VISIBLE (out of date, an operator looks)
/// rather than hidden (current, a stale site silently reads clean).
/// </para>
/// </summary>
/// <param name="site">The site to classify.</param>
/// <param name="currentArtifactHashes">Site id to the hash a deploy would send right now.</param>
@@ -137,26 +149,25 @@ public sealed class PendingUpdatesEvaluator : IPendingUpdatesEvaluator
/// <summary>
/// Counts one site's deployed, stale and un-evaluatable instances. Only instances
/// present in the staleness result are counted — the batch OMITS an instance with
/// no deployed snapshot, which is exactly the "nothing to be stale against"
/// exclusion the counts need.
/// no deployed snapshot at evaluation time (including one that was undeployed
/// mid-batch), which is exactly the "nothing to be stale against" exclusion the
/// counts need.
/// </summary>
/// <param name="siteId">Site whose instances are being tallied.</param>
/// <param name="deployedInstances">All instances in a state other than NotDeployed.</param>
/// <param name="siteInstances">
/// This site's instances in a state other than NotDeployed, pre-grouped by the caller.
/// </param>
/// <param name="staleness">Instance id to stale (true) / current (false) / unknown (null).</param>
/// <returns>The deployed, stale and unknown counts for the site.</returns>
private static (int Deployed, int Stale, int Unknown) TallyInstances(
int siteId,
IReadOnlyList<Instance> deployedInstances,
IEnumerable<Instance> siteInstances,
IReadOnlyDictionary<int, bool?> staleness)
{
var deployed = 0;
var stale = 0;
var unknown = 0;
foreach (var instance in deployedInstances)
foreach (var instance in siteInstances)
{
if (instance.SiteId != siteId)
continue;
if (!staleness.TryGetValue(instance.Id, out var isStale))
continue;