diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/AppInstallTerminalStateAndReconcileTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/AppInstallTerminalStateAndReconcileTests.cs new file mode 100644 index 00000000..8ff902fa --- /dev/null +++ b/src/Backend/AutopilotMonitor.Functions.Tests/AppInstallTerminalStateAndReconcileTests.cs @@ -0,0 +1,201 @@ +using Azure.Data.Tables; +using AutopilotMonitor.Functions.Services; +using AutopilotMonitor.Shared.Models; +using Xunit; + +namespace AutopilotMonitor.Functions.Tests; + +/// +/// PR0 (2026-07-26, source-data audit §0.5 of tasks/insights-expansion-spec.md). +/// +/// Ingest side: the agent emits app_install_completed for EVERY terminal transition — +/// including Skipped/Postponed no-ops (V1 wire parity; production: 78 % of zero-duration rows +/// were skips). The payload's state field must be persisted as +/// so metrics can exclude skips from durations +/// and rates. +/// +/// Store side: pins the +/// cross-batch contract — earliest StartedAt wins, terminal status is sticky, and the Q4 +/// out-of-order case (terminal batch first, started batch later) recomputes the duration. +/// +public class AppInstallTerminalStateAndReconcileTests +{ + private static readonly DateTime T0 = new(2026, 7, 26, 10, 0, 0, DateTimeKind.Utc); + + // ── ingest: TerminalState fold ────────────────────────────────────────── + + private static Dictionary Aggregate(params EnrollmentEvent[] events) + { + var summaries = new Dictionary(StringComparer.OrdinalIgnoreCase); + foreach (var evt in events) + EventIngestProcessor.AggregateAppInstallEvent(evt, "tenant", "session", summaries); + return summaries; + } + + private static EnrollmentEvent AppEvent(string eventType, DateTime ts, string? state = null, string appName = "App") + { + var evt = new EnrollmentEvent + { + EventType = eventType, + Timestamp = ts, + Data = new Dictionary { ["appName"] = appName }, + }; + if (state != null) evt.Data["state"] = state; + return evt; + } + + [Theory] + [InlineData("Installed")] + [InlineData("Skipped")] + [InlineData("Postponed")] + public void Completed_PersistsPayloadState(string state) + { + var s = Aggregate(AppEvent("app_install_completed", T0, state))["App"].Summary; + + Assert.Equal("Succeeded", s.Status); // backward-compatible status + Assert.Equal(state, s.TerminalState); // new precision column + } + + [Fact] + public void Completed_WithoutState_LeavesSentinelEmpty_NeverGuessed() + { + var s = Aggregate(AppEvent("app_install_completed", T0))["App"].Summary; + Assert.Equal(string.Empty, s.TerminalState); + } + + [Fact] + public void Completed_WithNonTerminalOrGarbageState_LeavesSentinelEmpty() + { + // A payload state outside the terminal set (bug or drift) must not be persisted. + var s = Aggregate(AppEvent("app_install_completed", T0, "Downloading"))["App"].Summary; + Assert.Equal(string.Empty, s.TerminalState); + } + + [Fact] + public void Failed_SetsError() + { + var s = Aggregate(AppEvent("app_install_failed", T0))["App"].Summary; + Assert.Equal("Failed", s.Status); + Assert.Equal("Error", s.TerminalState); + } + + [Fact] + public void FailedThenCompleted_RetrySucceeds_LastTerminalWins() + { + // IME retry path (audit Q1): Error → retry → Installed. The later completed event is + // the authoritative outcome for Status AND TerminalState. + var s = Aggregate( + AppEvent("app_install_failed", T0), + AppEvent("app_install_completed", T0.AddMinutes(10), "Installed"))["App"].Summary; + + Assert.Equal("Succeeded", s.Status); + Assert.Equal("Installed", s.TerminalState); + } + + [Fact] + public void SkippedEvent_SetsSkipped_ButNeverOverridesStrongerTerminal() + { + var fresh = Aggregate(AppEvent("app_install_skipped", T0))["App"].Summary; + Assert.Equal("Succeeded", fresh.Status); + Assert.Equal("Skipped", fresh.TerminalState); + + var afterFailure = Aggregate( + AppEvent("app_install_failed", T0), + AppEvent("app_install_skipped", T0.AddSeconds(5)))["App"].Summary; + Assert.Equal("Error", afterFailure.TerminalState); + } + + // ── store: cross-batch reconcile ──────────────────────────────────────── + + [Fact] + public void Reconcile_Q4_OutOfOrder_RecomputesDurationFromStoredCompletedAt() + { + // Terminal batch landed first: row has CompletedAt but no usable duration. + var existing = new TableEntity("tenant", "session_App") + { + ["Status"] = "Succeeded", + ["StartedAt"] = new DateTimeOffset(T0.AddMinutes(5)), // late (completed-event) stamp + ["CompletedAt"] = new DateTimeOffset(T0.AddMinutes(5)), + }; + // Started batch arrives now with the true (earlier) start. + var summary = new AppInstallSummary + { + AppName = "App", SessionId = "session", TenantId = "tenant", + Status = "InProgress", StartedAt = T0, + }; + + TableStorageService.ReconcileAppInstallSummaryWithExisting(summary, existing); + + Assert.Equal(T0, summary.StartedAt); // earlier start kept + Assert.Equal(T0.AddMinutes(5), summary.CompletedAt); // endpoint adopted from the row + Assert.Equal(300, summary.DurationSeconds); // recomputed — was permanently unset before PR0 + Assert.Equal("Succeeded", summary.Status); // sticky terminal (below) + } + + [Fact] + public void Reconcile_TerminalStatusIsSticky_AgainstLateInProgressBatch() + { + var existing = new TableEntity("tenant", "session_App") { ["Status"] = "Failed" }; + var summary = new AppInstallSummary { AppName = "App", Status = "InProgress", StartedAt = T0 }; + + TableStorageService.ReconcileAppInstallSummaryWithExisting(summary, existing); + + Assert.Equal("Failed", summary.Status); + } + + [Fact] + public void Reconcile_TerminalToTerminal_StaysWithIncomingEvidence() + { + // Failed row + succeeded batch (successful retry) → the new terminal wins. + var existing = new TableEntity("tenant", "session_App") { ["Status"] = "Failed" }; + var summary = new AppInstallSummary { AppName = "App", Status = "Succeeded", StartedAt = T0 }; + + TableStorageService.ReconcileAppInstallSummaryWithExisting(summary, existing); + + Assert.Equal("Succeeded", summary.Status); + } + + [Fact] + public void Reconcile_Q4Guard_DoesNotFireWithoutStoredCompletedAt() + { + var existing = new TableEntity("tenant", "session_App") { ["Status"] = "InProgress" }; + var summary = new AppInstallSummary { AppName = "App", Status = "InProgress", StartedAt = T0 }; + + TableStorageService.ReconcileAppInstallSummaryWithExisting(summary, existing); + + Assert.Null(summary.CompletedAt); + Assert.Equal(0, summary.DurationSeconds); + } + + [Fact] + public void Reconcile_Q4Guard_IgnoresStoredCompletedAtBeforeIncomingStart() + { + // Negative interval (clock mess) must not produce a negative duration. + var existing = new TableEntity("tenant", "session_App") + { + ["Status"] = "Succeeded", + ["CompletedAt"] = new DateTimeOffset(T0.AddMinutes(-10)), + }; + var summary = new AppInstallSummary { AppName = "App", Status = "InProgress", StartedAt = T0 }; + + TableStorageService.ReconcileAppInstallSummaryWithExisting(summary, existing); + + Assert.Null(summary.CompletedAt); + Assert.Equal(0, summary.DurationSeconds); + } + + // ── entity builder: sentinel gating for the new column ───────────────── + + [Fact] + public void EntityBuilder_TerminalState_IsSentinelGated() + { + var withState = new AppInstallSummary { AppName = "App", TerminalState = "Skipped" }; + var entity = TableStorageService.BuildAppInstallSummaryEntity(withState, "rk"); + Assert.Equal("Skipped", entity.GetString("TerminalState")); + + var withoutState = new AppInstallSummary { AppName = "App" }; + var empty = TableStorageService.BuildAppInstallSummaryEntity(withoutState, "rk"); + // Column absent → Merge-mode preserves a prior batch's terminal state. + Assert.False(empty.ContainsKey("TerminalState")); + } +} diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/AppsAnalyticsFailureRateTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/AppsAnalyticsFailureRateTests.cs index 3daa5fc8..06c3e0a0 100644 --- a/src/Backend/AutopilotMonitor.Functions.Tests/AppsAnalyticsFailureRateTests.cs +++ b/src/Backend/AutopilotMonitor.Functions.Tests/AppsAnalyticsFailureRateTests.cs @@ -162,6 +162,58 @@ public class AppsAnalyticsFailureRateTests Assert.Equal(50d, todayBucket.GetProperty("failureRate").GetDouble()); } + // ── PR0 (2026-07-26): skip / unmeasured classification ────────────────── + + [Fact] + public void List_Skips_LeaveRateAndDurations_AndAreReportedSeparately() + { + var skip = A("Succeeded"); + skip.TerminalState = "Skipped"; + skip.DurationSeconds = 0; + var unmeasured = A("Succeeded"); + unmeasured.TerminalState = "Installed"; + unmeasured.DurationSeconds = 0; + + var summaries = new List { A("Failed"), A("Succeeded"), skip, unmeasured }; + + var root = JsonSerializer.SerializeToElement( + AppsAnalyticsHelper.BuildAppsListResponse(summaries, days: 30)); + var app = root.GetProperty("apps")[0]; + + Assert.Equal(4, app.GetProperty("totalInstalls").GetInt32()); + Assert.Equal(2, app.GetProperty("succeeded").GetInt32()); // real install + unmeasured + Assert.Equal(1, app.GetProperty("skipped").GetInt32()); + Assert.Equal(1, app.GetProperty("unmeasured").GetInt32()); + // 1 / (1 + 2) — the skip is not an attempt. + Assert.Equal(33.3d, app.GetProperty("failureRate").GetDouble()); + // avg over the single measured row (30s) — the zero must not drag it to 15. + Assert.Equal(30d, app.GetProperty("avgDurationSeconds").GetDouble()); + } + + [Fact] + public async Task Analytics_Summary_CarriesSkippedAndUnmeasured_AndP95OverMeasuredOnly() + { + var skip = A("Succeeded"); + skip.TerminalState = "Skipped"; + skip.DurationSeconds = 0; + var unmeasured = A("Succeeded"); + unmeasured.TerminalState = "Installed"; + unmeasured.DurationSeconds = 0; + var measured = A("Succeeded"); + measured.TerminalState = "Installed"; + measured.DurationSeconds = 100; + + var root = await BuildAnalyticsAsync(new List { skip, unmeasured, measured }); + var summary = root.GetProperty("summary"); + + Assert.Equal(2, summary.GetProperty("succeeded").GetInt32()); + Assert.Equal(1, summary.GetProperty("skipped").GetInt32()); + Assert.Equal(1, summary.GetProperty("unmeasured").GetInt32()); + Assert.Equal(100d, summary.GetProperty("avgDurationSeconds").GetDouble()); + Assert.Equal(100, summary.GetProperty("p95DurationSeconds").GetInt32()); + Assert.Equal(0d, summary.GetProperty("failureRate").GetDouble()); + } + [Fact] public async Task Analytics_DeviceModelBreakdown_RatesAndSampleFloor_UseFinishedInstalls() { diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/MetricsMathAppPayloadTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/MetricsMathAppPayloadTests.cs index 1430e15f..89692335 100644 --- a/src/Backend/AutopilotMonitor.Functions.Tests/MetricsMathAppPayloadTests.cs +++ b/src/Backend/AutopilotMonitor.Functions.Tests/MetricsMathAppPayloadTests.cs @@ -137,6 +137,92 @@ public class MetricsMathAppPayloadTests Assert.Equal(50d, app.GetProperty("failureRate").GetDouble()); } + // ── PR0 (2026-07-26): skip/unmeasured classification ──────────────────── + + [Fact] + public void Skips_LeaveTheFailureRate_AndAreReportedSeparately() + { + // 1 Failed + 1 real install + 3 skips ("Update for X" not applicable). + // Rate must be 1/(1+1) = 50%, NOT 1/(1+4) = 20% — skips are not attempts. + var summaries = new List + { + new() { AppName = "App A", Status = "Failed", TerminalState = "Error" }, + new() { AppName = "App A", Status = "Succeeded", TerminalState = "Installed", DurationSeconds = 30 }, + new() { AppName = "App A", Status = "Succeeded", TerminalState = "Skipped" }, + new() { AppName = "App A", Status = "Succeeded", TerminalState = "Skipped" }, + new() { AppName = "App A", Status = "Succeeded", TerminalState = "Postponed" }, + }; + + var root = Build(summaries); + var app = root.GetProperty("topFailingApps")[0]; + + Assert.Equal(50d, app.GetProperty("failureRate").GetDouble()); + Assert.Equal(1, app.GetProperty("succeeded").GetInt32()); + Assert.Equal(3, app.GetProperty("skipped").GetInt32()); + Assert.Equal(0, app.GetProperty("unmeasured").GetInt32()); + Assert.Equal(3, root.GetProperty("totalSkipped").GetInt32()); + } + + [Fact] + public void ZeroDurations_AreExcludedFromAverages_AndCountedAsUnmeasured() + { + // Two measured installs (100s, 200s) + one whose start was never observed (audit §0.5: + // agent attach window — duration UNKNOWN). avg must be 150, not (100+200+0)/3 = 100. + var summaries = new List + { + new() { AppName = "App A", Status = "Succeeded", TerminalState = "Installed", DurationSeconds = 100 }, + new() { AppName = "App A", Status = "Succeeded", TerminalState = "Installed", DurationSeconds = 200 }, + new() { AppName = "App A", Status = "Succeeded", TerminalState = "Installed", DurationSeconds = 0 }, + }; + + var root = Build(summaries); + Assert.Equal(1, root.GetProperty("totalUnmeasured").GetInt32()); + + // Rebuild with a third measured row to clear the sample floor and read the average. + summaries.Add(new AppInstallSummary { AppName = "App A", Status = "Succeeded", TerminalState = "Installed", DurationSeconds = 150 }); + var slowest = Build(summaries).GetProperty("slowestApps"); + Assert.Equal(1, slowest.GetArrayLength()); + Assert.Equal(150d, slowest[0].GetProperty("avgDurationSeconds").GetDouble()); + Assert.Equal(1, slowest[0].GetProperty("unmeasured").GetInt32()); + } + + [Fact] + public void LegacyZeroDurationRows_StayInRate_ButNeverInDurations() + { + // Rows written before TerminalState existed: cannot be classified as skip vs unmeasured. + // They keep the legacy rate behavior (counted as succeeded) and are excluded from + // durations (counted as unmeasured — duration genuinely not measured). + var summaries = new List + { + new() { AppName = "App A", Status = "Failed" }, + new() { AppName = "App A", Status = "Succeeded", DurationSeconds = 0 }, // legacy, no TerminalState + new() { AppName = "App A", Status = "Succeeded", DurationSeconds = 60, TerminalState = "Installed" }, + }; + + var app = Build(summaries).GetProperty("topFailingApps")[0]; + + Assert.Equal(2, app.GetProperty("succeeded").GetInt32()); // legacy row stays in the rate + Assert.Equal(33.3d, app.GetProperty("failureRate").GetDouble()); + Assert.Equal(1, app.GetProperty("unmeasured").GetInt32()); + Assert.Equal(0, app.GetProperty("skipped").GetInt32()); + } + + [Fact] + public void SlowestApps_GateOnMeasuredInstalls_NotOnSucceededCount() + { + // 3 succeeded but only 2 measured → below the floor; an app must not rank "fast" + // (or at all) on a handful of unobserved zeros. + var summaries = new List + { + new() { AppName = "Mostly Unmeasured", Status = "Succeeded", TerminalState = "Installed", DurationSeconds = 5 }, + new() { AppName = "Mostly Unmeasured", Status = "Succeeded", TerminalState = "Installed", DurationSeconds = 5 }, + new() { AppName = "Mostly Unmeasured", Status = "Succeeded", TerminalState = "Installed", DurationSeconds = 0 }, + }; + + var slowest = Build(summaries).GetProperty("slowestApps"); + Assert.Equal(0, slowest.GetArrayLength()); + } + [Fact] public void SlowestApps_DropsAppsBelowSampleFloor() { diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Apps/AppsAnalyticsHelper.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Apps/AppsAnalyticsHelper.cs index 05b97976..53f26342 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Apps/AppsAnalyticsHelper.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Apps/AppsAnalyticsHelper.cs @@ -143,11 +143,15 @@ namespace AutopilotMonitor.Functions.Functions.Apps var apps = summaries.GroupBy(s => s.AppName).Select(g => { + // PR0 classification (see MetricsMath.IsSkipTerminalState / HasMeasuredDuration): + // skips leave the rate + durations; duration stats read measured rows only. var total = g.Count(); - var succeeded = g.Count(s => s.Status == "Succeeded"); + var succeededAll = g.Where(s => s.Status == "Succeeded").ToList(); + var skipped = succeededAll.Count(MetricsMath.IsSkipTerminalState); + var installed = succeededAll.Where(s => !MetricsMath.IsSkipTerminalState(s)).ToList(); + var measured = installed.Where(MetricsMath.HasMeasuredDuration).ToList(); var failed = g.Count(s => s.Status == "Failed"); - var completed = g.Where(s => s.Status == "Succeeded").ToList(); - var failureRate = MetricsMath.TerminalFailureRatePct(failed, succeeded); + var failureRate = MetricsMath.TerminalFailureRatePct(failed, installed.Count); var firstHalf = g.Where(s => s.StartedAt < midpoint).ToList(); var secondHalf = g.Where(s => s.StartedAt >= midpoint).ToList(); @@ -158,12 +162,14 @@ namespace AutopilotMonitor.Functions.Functions.Apps appName = g.Key, appType = g.Select(s => s.AppType).FirstOrDefault(t => !string.IsNullOrEmpty(t)) ?? string.Empty, totalInstalls = total, - succeeded, + succeeded = installed.Count, + skipped, + unmeasured = installed.Count - measured.Count, failed, failureRate, - avgDurationSeconds = completed.Count > 0 ? Math.Round(completed.Average(s => s.DurationSeconds), 0) : 0, - maxDurationSeconds = completed.Count > 0 ? completed.Max(s => s.DurationSeconds) : 0, - avgDownloadBytes = completed.Count > 0 ? (long)completed.Average(s => s.DownloadBytes) : 0, + avgDurationSeconds = measured.Count > 0 ? Math.Round(measured.Average(s => s.DurationSeconds), 0) : 0, + maxDurationSeconds = measured.Count > 0 ? measured.Max(s => s.DurationSeconds) : 0, + avgDownloadBytes = measured.Count > 0 ? (long)measured.Average(s => s.DownloadBytes) : 0, trend, trendDelta, lastSeenAt = g.Max(s => s.CompletedAt ?? s.StartedAt) @@ -241,6 +247,8 @@ namespace AutopilotMonitor.Functions.Functions.Apps { totalInstalls = 0, succeeded = 0, + skipped = 0, + unmeasured = 0, failed = 0, failureRate = 0, avgDurationSeconds = 0, @@ -260,13 +268,17 @@ namespace AutopilotMonitor.Functions.Functions.Apps } var total = summaries.Count; - var succeeded = summaries.Count(s => s.Status == "Succeeded"); + // PR0 classification — skips leave rate + durations; durations read measured rows only. + var succeededAll = summaries.Where(s => s.Status == "Succeeded").ToList(); + var skipped = succeededAll.Count(MetricsMath.IsSkipTerminalState); + var installed = succeededAll.Where(s => !MetricsMath.IsSkipTerminalState(s)).ToList(); + var measured = installed.Where(MetricsMath.HasMeasuredDuration).ToList(); + var succeeded = installed.Count; var failed = summaries.Count(s => s.Status == "Failed"); - var completed = summaries.Where(s => s.Status == "Succeeded").ToList(); var failureRate = MetricsMath.TerminalFailureRatePct(failed, succeeded); - var avgDurationSeconds = completed.Count > 0 ? Math.Round(completed.Average(s => s.DurationSeconds), 0) : 0; - var p95DurationSeconds = Percentile(completed.Select(s => s.DurationSeconds).ToList(), 0.95); - var avgDownloadBytes = completed.Count > 0 ? (long)completed.Average(s => s.DownloadBytes) : 0; + var avgDurationSeconds = measured.Count > 0 ? Math.Round(measured.Average(s => s.DurationSeconds), 0) : 0; + var p95DurationSeconds = Percentile(measured.Select(s => s.DurationSeconds).ToList(), 0.95); + var avgDownloadBytes = measured.Count > 0 ? (long)measured.Average(s => s.DownloadBytes) : 0; // Trend (same rule as list endpoint). var firstHalf = summaries.Where(s => s.StartedAt < midpoint).ToList(); @@ -287,7 +299,8 @@ namespace AutopilotMonitor.Functions.Functions.Apps { var vTotal = g.Count(); var vFailed = g.Count(s => s.Status == "Failed"); - var vSucceeded = g.Count(s => s.Status == "Succeeded"); + // Same PR0 convention as the top-level rate: skips are not attempts. + var vSucceeded = g.Count(s => s.Status == "Succeeded" && !MetricsMath.IsSkipTerminalState(s)); return new { appVersion = g.Key, @@ -347,7 +360,8 @@ namespace AutopilotMonitor.Functions.Functions.Apps { var modelTotal = g.Count(); var modelFailed = g.Count(x => x.Summary.Status == "Failed"); - var modelSucceeded = g.Count(x => x.Summary.Status == "Succeeded"); + // PR0 convention: skips are not attempts (same population as the headline rate). + var modelSucceeded = g.Count(x => x.Summary.Status == "Succeeded" && !MetricsMath.IsSkipTerminalState(x.Summary)); var modelFailureRate = MetricsMath.TerminalFailureRatePct(modelFailed, modelSucceeded); var lift = failureRate > 0 ? Math.Round(modelFailureRate / failureRate, 2) @@ -378,6 +392,8 @@ namespace AutopilotMonitor.Functions.Functions.Apps { totalInstalls = total, succeeded, + skipped, + unmeasured = installed.Count - measured.Count, failed, failureRate, avgDurationSeconds, @@ -490,10 +506,13 @@ namespace AutopilotMonitor.Functions.Functions.Apps private static (string Trend, double? TrendDelta) ComputeFailureTrend( List firstHalf, List secondHalf) { + // PR0 convention: skips are not attempts — keep the trend on the same population + // as the headline failure rate, or a wave of not-applicable update policies would + // fake an "improving" trend. var fhFailed = firstHalf.Count(s => s.Status == "Failed"); - var fhSucceeded = firstHalf.Count(s => s.Status == "Succeeded"); + var fhSucceeded = firstHalf.Count(s => s.Status == "Succeeded" && !MetricsMath.IsSkipTerminalState(s)); var shFailed = secondHalf.Count(s => s.Status == "Failed"); - var shSucceeded = secondHalf.Count(s => s.Status == "Succeeded"); + var shSucceeded = secondHalf.Count(s => s.Status == "Succeeded" && !MetricsMath.IsSkipTerminalState(s)); if (fhFailed + fhSucceeded < 5 || shFailed + shSucceeded < 5) return ("stable", null); @@ -531,16 +550,17 @@ namespace AutopilotMonitor.Functions.Functions.Apps var items = kv.Value; var bTotal = items.Count; var bFailed = items.Count(s => s.Status == "Failed"); - var bSucceeded = items.Count(s => s.Status == "Succeeded"); - var bCompleted = items.Where(s => s.Status == "Succeeded").ToList(); + // PR0 convention: skips are not attempts; durations read measured rows only. + var bInstalled = items.Where(s => s.Status == "Succeeded" && !MetricsMath.IsSkipTerminalState(s)).ToList(); + var bMeasured = bInstalled.Where(MetricsMath.HasMeasuredDuration).ToList(); return (object)new { bucketStart = DateTime.SpecifyKind(kv.Key, DateTimeKind.Utc), installs = bTotal, - succeeded = bSucceeded, + succeeded = bInstalled.Count, failed = bFailed, - failureRate = MetricsMath.TerminalFailureRatePct(bFailed, bSucceeded), - avgDurationSeconds = bCompleted.Count > 0 ? Math.Round(bCompleted.Average(s => s.DurationSeconds), 0) : 0 + failureRate = MetricsMath.TerminalFailureRatePct(bFailed, bInstalled.Count), + avgDurationSeconds = bMeasured.Count > 0 ? Math.Round(bMeasured.Average(s => s.DurationSeconds), 0) : 0 }; }) .ToList(); diff --git a/src/Backend/AutopilotMonitor.Functions/Helpers/MetricsMath.cs b/src/Backend/AutopilotMonitor.Functions/Helpers/MetricsMath.cs index 94a55419..24cdfbd4 100644 --- a/src/Backend/AutopilotMonitor.Functions/Helpers/MetricsMath.cs +++ b/src/Backend/AutopilotMonitor.Functions/Helpers/MetricsMath.cs @@ -20,6 +20,28 @@ public static class MetricsMath return finished > 0 ? Math.Round((double)failed / finished * 100, 1) : 0; } + /// + /// True when the row's terminal state says no real install attempt happened — + /// Skipped (not applicable, e.g. WinGet "Update for X" with the target app absent) + /// or Postponed. PR0 decision (2026-07-26): such rows are excluded from duration + /// statistics AND from the failure/success rate; they are reported as their own + /// category instead. Rows written before the TerminalState column existed return + /// false here (empty state) — they keep the legacy rate behavior because they + /// cannot be reclassified honestly. + /// + public static bool IsSkipTerminalState(AppInstallSummary s) + => s.TerminalState == "Skipped" || s.TerminalState == "Postponed"; + + /// + /// True for a succeeded row whose install duration was actually observed. Zero / + /// absent duration on a non-skip succeeded row means the START was never observed + /// (agent attach window — audit finding, §0.5 of the insights spec): the duration is + /// UNKNOWN, not zero — averaging it in would understate real install times (measured + /// −18 % in production). Duration statistics must only read rows where this is true. + /// + public static bool HasMeasuredDuration(AppInstallSummary s) + => s.DurationSeconds > 0; + /// /// Builds the complete app-metrics response object from a (pre-time-filtered) set of app /// install summaries. Single source of truth for both the tenant (metrics/app) and @@ -38,7 +60,16 @@ public static class MetricsMath var appGroups = summaryList.GroupBy(s => s.AppName).Select(g => { - var completed = g.Where(s => s.Status == "Succeeded").ToList(); + // PR0 (2026-07-26) classification — see IsSkipTerminalState / HasMeasuredDuration: + // skipped = no real install attempt (TerminalState Skipped/Postponed) + // installed = succeeded rows that are not known skips (includes legacy rows + // without TerminalState — they cannot be reclassified) + // measured = installed rows with an observed duration — the ONLY duration input + // unmeasured = installed rows whose start was never observed (duration unknown) + var succeededAll = g.Where(s => s.Status == "Succeeded").ToList(); + var skipped = succeededAll.Where(IsSkipTerminalState).ToList(); + var installed = succeededAll.Where(s => !IsSkipTerminalState(s)).ToList(); + var measured = installed.Where(HasMeasuredDuration).ToList(); var failed = g.Where(s => s.Status == "Failed").ToList(); var total = g.Count(); @@ -51,12 +82,16 @@ public static class MetricsMath { appName = g.Key, totalInstalls = total, - succeeded = completed.Count, + succeeded = installed.Count, + skipped = skipped.Count, + unmeasured = installed.Count - measured.Count, failed = failed.Count, - failureRate = TerminalFailureRatePct(failed.Count, completed.Count), - avgDurationSeconds = completed.Count > 0 ? Math.Round(completed.Average(s => s.DurationSeconds), 0) : 0, - maxDurationSeconds = completed.Count > 0 ? completed.Max(s => s.DurationSeconds) : 0, - avgDownloadBytes = completed.Count > 0 ? (long)completed.Average(s => s.DownloadBytes) : 0, + // Skips leave the rate (they are not attempts); legacy rows stay in it. + failureRate = TerminalFailureRatePct(failed.Count, installed.Count), + avgDurationSeconds = measured.Count > 0 ? Math.Round(measured.Average(s => s.DurationSeconds), 0) : 0, + maxDurationSeconds = measured.Count > 0 ? measured.Max(s => s.DurationSeconds) : 0, + measuredInstalls = measured.Count, + avgDownloadBytes = measured.Count > 0 ? (long)measured.Average(s => s.DownloadBytes) : 0, doTotalBytesDownloaded = doG.TotalBytesDownloaded, doBytesFromPeers = doG.BytesFromPeers, doBytesFromCacheServer = doG.BytesFromCacheServer, @@ -71,8 +106,10 @@ public static class MetricsMath }; }).ToList(); + // Slowest ranking gates on MEASURED installs — an app whose durations were mostly + // unobserved must not rank as "fast" on a handful of zeros (audit §0.5). var slowestApps = SelectSlowestApps( - appGroups, a => a.succeeded, a => (double)a.avgDurationSeconds, minSamples: 3, take: 10); + appGroups, a => a.measuredInstalls, a => (double)a.avgDurationSeconds, minSamples: 3, take: 10); var topFailingApps = appGroups .Where(a => a.failed > 0) @@ -88,6 +125,8 @@ public static class MetricsMath success = true, totalApps = appGroups.Count, totalInstalls = summaryList.Count, + totalSkipped = appGroups.Sum(a => a.skipped), + totalUnmeasured = appGroups.Sum(a => a.unmeasured), slowestApps, topFailingApps, deliveryOptimization = new diff --git a/src/Backend/AutopilotMonitor.Functions/Services/EventIngestProcessor.AppInstall.cs b/src/Backend/AutopilotMonitor.Functions/Services/EventIngestProcessor.AppInstall.cs index 7d42c352..da86b6da 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/EventIngestProcessor.AppInstall.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/EventIngestProcessor.AppInstall.cs @@ -9,7 +9,9 @@ namespace AutopilotMonitor.Functions.Services /// public sealed partial class EventIngestProcessor { - private void AggregateAppInstallEvent(EnrollmentEvent evt, string tenantId, string sessionId, Dictionary summaries) + // internal static (was private instance): pure function over its parameters — exposed as a + // test seam so the TerminalState / status-fold contract is pinned by unit tests (PR0). + internal static void AggregateAppInstallEvent(EnrollmentEvent evt, string tenantId, string sessionId, Dictionary summaries) { bool isRelevant = evt.EventType == "app_install_started" || evt.EventType == "app_install_start" || @@ -110,10 +112,21 @@ namespace AutopilotMonitor.Functions.Services summary.CompletedAt = evt.Timestamp; if (summary.StartedAt != DateTime.MinValue) summary.DurationSeconds = Math.Max(1, EventTimestampValidator.SafeDurationSeconds(summary.StartedAt, evt.Timestamp)); + // PR0 (2026-07-26): the agent emits app_install_completed for EVERY terminal + // transition — including Skipped/Postponed (V1 wire parity). The payload's + // `state` field is the only way to tell a real install apart from a no-op + // (e.g. WinGet "Update for X" policies that were not applicable). Persist it + // so metrics can exclude skips from durations and rates. Unknown/absent state + // leaves the sentinel empty — never guessed. + var terminalStateRaw = evt.Data?.ContainsKey("state") == true + ? evt.Data["state"]?.ToString() : null; + if (terminalStateRaw == "Installed" || terminalStateRaw == "Skipped" || terminalStateRaw == "Postponed") + summary.TerminalState = terminalStateRaw; break; case "app_install_failed": summary.Status = "Failed"; + summary.TerminalState = "Error"; summary.CompletedAt = evt.Timestamp; if (summary.StartedAt != DateTime.MinValue) summary.DurationSeconds = Math.Max(1, EventTimestampValidator.SafeDurationSeconds(summary.StartedAt, evt.Timestamp)); @@ -143,6 +156,10 @@ namespace AutopilotMonitor.Functions.Services // Empty (sentinel: no observation yet) and "InProgress" both flip to Succeeded. if (summary.Status == "InProgress" || summary.Status == string.Empty) summary.Status = "Succeeded"; + // Same guard for the terminal state: a dedicated skipped event never overrides + // a stronger terminal (Installed/Error) already seen in this batch. + if (string.IsNullOrEmpty(summary.TerminalState)) + summary.TerminalState = "Skipped"; break; case "download_progress": diff --git a/src/Backend/AutopilotMonitor.Functions/Services/SlaBreachEvaluationService.cs b/src/Backend/AutopilotMonitor.Functions/Services/SlaBreachEvaluationService.cs index 10bbf297..8f680bf1 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/SlaBreachEvaluationService.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/SlaBreachEvaluationService.cs @@ -193,6 +193,9 @@ namespace AutopilotMonitor.Functions.Services var raw = await _metricsRepo.GetAppInstallSummariesByTenantAsync(config.TenantId, now.AddDays(-8)); appInstalls = raw .Where(a => (a.Status == "Succeeded" || a.Status == "Failed") + // PR0 (2026-07-26): skips are not install attempts — keep the breach + // evaluation on the same population as the SLA dashboard rate. + && !Helpers.MetricsMath.IsSkipTerminalState(a) && SlaMetricsService.GetIsoWeekKey(a.StartedAt) == currentWeekKey) .ToList(); } diff --git a/src/Backend/AutopilotMonitor.Functions/Services/SlaMetricsService.cs b/src/Backend/AutopilotMonitor.Functions/Services/SlaMetricsService.cs index 003171d8..729432b5 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/SlaMetricsService.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/SlaMetricsService.cs @@ -146,7 +146,10 @@ namespace AutopilotMonitor.Functions.Services { var weekApps = appInstalls .Where(a => GetIsoWeekKey(a.StartedAt) == group.Key && - (a.Status == "Succeeded" || a.Status == "Failed")) + (a.Status == "Succeeded" || a.Status == "Failed") && + // PR0 (2026-07-26): skips are not install attempts — they must + // not pad the SLA app-install success rate. + !Helpers.MetricsMath.IsSkipTerminalState(a)) .ToList(); if (weekApps.Count >= 5) { @@ -175,7 +178,9 @@ namespace AutopilotMonitor.Functions.Services { var currentWeekApps = appInstalls .Where(a => GetIsoWeekKey(a.StartedAt) == currentWeekKey && - (a.Status == "Succeeded" || a.Status == "Failed")) + (a.Status == "Succeeded" || a.Status == "Failed") && + // PR0 (2026-07-26): same convention as the weekly trend above. + !Helpers.MetricsMath.IsSkipTerminalState(a)) .ToList(); if (currentWeekApps.Count >= 5) diff --git a/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Metrics.cs b/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Metrics.cs index 218e5053..4f4cee0a 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Metrics.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Metrics.cs @@ -216,7 +216,46 @@ namespace AutopilotMonitor.Functions.Services var existingResult = await tableClient.GetEntityIfExistsAsync(summary.TenantId, rowKey); if (existingResult.HasValue) { - var existing = existingResult.Value!; + ReconcileAppInstallSummaryWithExisting(summary, existingResult.Value!); + } + + var entity = BuildAppInstallSummaryEntity(summary, rowKey); + + // Merge-mode (default) preserves columns absent from the entity. Combined with the + // dynamic property-add inside BuildAppInstallSummaryEntity this means a batch that + // observed only progress / telemetry events for an app cannot clobber a prior + // terminal Status / CompletedAt / DurationSeconds / FailureCode / FailureMessage. + await tableClient.UpsertEntityAsync(entity, TableUpdateMode.Merge); + return true; + } + catch (Exception ex) + { + _logger.LogError(ex, $"Failed to store app install summary for {summary.AppName}"); + return false; + } + } + + /// + /// Reconciles an incoming per-batch against the stored row + /// before the Merge-mode upsert. Extracted as an internal static seam so the cross-batch + /// contract (earliest StartedAt wins, terminal status is sticky, out-of-order duration + /// recompute, prior-batch metadata preservation) is pinned by unit tests. + /// + internal static void ReconcileAppInstallSummaryWithExisting(AppInstallSummary summary, TableEntity existing) + { + // Terminal status is sticky (PR0 hardening, same out-of-order family as Q4): + // a late or replayed batch that only observed pre-terminal events carries + // Status "InProgress" — a real (non-sentinel) value that Merge-mode would + // happily write over a row the terminal batch already closed. Only another + // terminal observation may change a terminal status (Failed→Succeeded on a + // successful retry stays legal). + var existingStatus = existing.GetString("Status"); + if ((existingStatus == "Succeeded" || existingStatus == "Failed") && + summary.Status != "Succeeded" && summary.Status != "Failed") + { + summary.Status = existingStatus; + } + var existingStartedAt = existing.GetDateTimeOffset("StartedAt")?.UtcDateTime; if (existingStartedAt.HasValue && existingStartedAt.Value != DateTime.MinValue) { @@ -231,6 +270,21 @@ namespace AutopilotMonitor.Functions.Services } } + // Q4 (source-data audit 2026-07-26): out-of-order arrival — the terminal batch + // landed FIRST (row already carries CompletedAt), the started batch arrives now + // with the earlier StartedAt. Neither batch alone could compute the duration; + // recompute it here against the stored CompletedAt so the row doesn't keep a + // permanently unset duration despite both endpoints being known. + if (!summary.CompletedAt.HasValue && summary.StartedAt != DateTime.MinValue) + { + var existingCompletedAt = existing.GetDateTimeOffset("CompletedAt")?.UtcDateTime; + if (existingCompletedAt.HasValue && existingCompletedAt.Value >= summary.StartedAt) + { + summary.CompletedAt = existingCompletedAt.Value; + summary.DurationSeconds = (int)(existingCompletedAt.Value - summary.StartedAt).TotalSeconds; + } + } + // Preserve DownloadDurationSeconds and DownloadBytes from prior batch if current batch has no value if (summary.DownloadDurationSeconds == 0) { @@ -310,23 +364,6 @@ namespace AutopilotMonitor.Functions.Services if (!string.IsNullOrEmpty(existingDetection)) summary.DetectionResult = existingDetection; } - - } - - var entity = BuildAppInstallSummaryEntity(summary, rowKey); - - // Merge-mode (default) preserves columns absent from the entity. Combined with the - // dynamic property-add inside BuildAppInstallSummaryEntity this means a batch that - // observed only progress / telemetry events for an app cannot clobber a prior - // terminal Status / CompletedAt / DurationSeconds / FailureCode / FailureMessage. - await tableClient.UpsertEntityAsync(entity, TableUpdateMode.Merge); - return true; - } - catch (Exception ex) - { - _logger.LogError(ex, $"Failed to store app install summary for {summary.AppName}"); - return false; - } } /// @@ -421,7 +458,7 @@ namespace AutopilotMonitor.Functions.Services internal static readonly string[] AppsDashboardProjection = { "PartitionKey", "RowKey", "TenantId", "SessionId", "AppName", "AppType", "AppVersion", - "Status", "StartedAt", "CompletedAt", "DurationSeconds", "DownloadBytes", + "Status", "TerminalState", "StartedAt", "CompletedAt", "DurationSeconds", "DownloadBytes", "AttemptNumber", "InstallerPhase", "FailureCode", "FailureMessage", "ExitCode", "DetectionResult" }; @@ -514,6 +551,7 @@ namespace AutopilotMonitor.Functions.Services SessionId = entity.GetString("SessionId") ?? string.Empty, TenantId = entity.GetString("TenantId") ?? entity.PartitionKey, Status = entity.GetString("Status") ?? "InProgress", + TerminalState = entity.GetString("TerminalState") ?? string.Empty, DurationSeconds = entity.GetInt32("DurationSeconds") ?? 0, DownloadBytes = entity.GetInt64("DownloadBytes") ?? 0, DownloadDurationSeconds = entity.GetInt32("DownloadDurationSeconds") ?? 0, @@ -1381,6 +1419,8 @@ namespace AutopilotMonitor.Functions.Services // Sentinel-gated lifecycle columns: only write when the current batch observed them. if (!string.IsNullOrEmpty(summary.Status)) entity["Status"] = summary.Status; + if (!string.IsNullOrEmpty(summary.TerminalState)) + entity["TerminalState"] = summary.TerminalState; if (summary.DurationSeconds > 0) entity["DurationSeconds"] = summary.DurationSeconds; if (summary.CompletedAt.HasValue) diff --git a/src/Shared/AutopilotMonitor.Shared/Models/Metrics/AppInstallSummary.cs b/src/Shared/AutopilotMonitor.Shared/Models/Metrics/AppInstallSummary.cs index 64eb2336..438988fe 100644 --- a/src/Shared/AutopilotMonitor.Shared/Models/Metrics/AppInstallSummary.cs +++ b/src/Shared/AutopilotMonitor.Shared/Models/Metrics/AppInstallSummary.cs @@ -23,6 +23,19 @@ namespace AutopilotMonitor.Shared.Models /// public string Status { get; set; } = string.Empty; + /// + /// Terminal lifecycle state carried by the agent payload on the closing event: + /// "Installed", "Skipped", "Postponed" (from app_install_completed's + /// state field / app_install_skipped) or "Error" (from + /// app_install_failed). Empty = sentinel: no terminal event observed in the + /// current batch, or a row written before this column existed (2026-07 PR0) — + /// storage omits the column on empty so Merge-mode preserves a prior value. + /// Skipped/Postponed rows are no real install attempt: they are excluded from + /// duration statistics and from the failure/success rate (PR0 decision 2026-07-26); + /// Status stays "Succeeded" for backward compatibility. + /// + public string TerminalState { get; set; } = string.Empty; + /// Total installation duration in seconds (from start to complete/failed) public int DurationSeconds { get; set; } diff --git a/src/Web/autopilot-monitor-web/app/apps/[appName]/page.tsx b/src/Web/autopilot-monitor-web/app/apps/[appName]/page.tsx index 476e673b..8723f686 100644 --- a/src/Web/autopilot-monitor-web/app/apps/[appName]/page.tsx +++ b/src/Web/autopilot-monitor-web/app/apps/[appName]/page.tsx @@ -82,6 +82,10 @@ interface AnalyticsResponse { summary: { totalInstalls: number; succeeded: number; + /** Not-applicable no-ops (TerminalState Skipped/Postponed) — not attempts, excluded from rate + durations. */ + skipped: number; + /** Real installs whose start was never observed — duration unknown, excluded from duration stats. */ + unmeasured: number; failed: number; failureRate: number; avgDurationSeconds: number; @@ -440,6 +444,7 @@ export default function AppDetailPage() { label="Succeeded" value={analytics.summary.succeeded.toString()} color="text-emerald-700" + hint={analytics.summary.skipped > 0 ? `+${analytics.summary.skipped} skipped (not applicable)` : undefined} /> 0 + ? `measured installs only — ${analytics.summary.unmeasured} without observed start` + : "measured installs only" + } /> {row.totalInstalls} - {row.succeeded} + + {row.succeeded} + {row.skipped > 0 && ( + + +{row.skipped}s + + )} + {row.failed} = 20 ? "text-red-700 font-semibold" : row.failureRate >= 5 ? "text-amber-700" : "text-gray-700"}> diff --git a/src/Web/autopilot-monitor-web/app/fleet-health/page.tsx b/src/Web/autopilot-monitor-web/app/fleet-health/page.tsx index 1bb293f5..eb59823e 100644 --- a/src/Web/autopilot-monitor-web/app/fleet-health/page.tsx +++ b/src/Web/autopilot-monitor-web/app/fleet-health/page.tsx @@ -515,6 +515,12 @@ export default function FleetHealthPage() {

Slowest Apps + + measured installs +

{!appMetrics || appMetrics.slowestApps.length === 0 ? (