From 4eac7d042c1a688d47e99d701284a9f07f6d403e Mon Sep 17 00:00:00 2001 From: Oliver Kieselbach Date: Sun, 26 Jul 2026 16:24:45 +0200 Subject: [PATCH] =?UTF-8?q?fix(metrics):=20app=20duration/rate=20stats=20o?= =?UTF-8?q?ver=20real=20installs=20only=20=E2=80=94=20TerminalState=20colu?= =?UTF-8?q?mn=20+=20out-of-order=20reconcile?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Source-data audit (tasks/insights-expansion-spec.md §0.5) verified against production: 17.9% of Succeeded AppInstallSummaries rows carry no duration — 78% are genuine skips (app_install_completed with payload state=Skipped, "Update for X" WinGet policies not applicable) whose state the ingest discarded, the rest are real installs whose start the agent never observed (attach window). Averaging them as 0s understated avg install duration by 18% and distorted the slowest-apps ranking. - AppInstallSummary.TerminalState (Installed/Skipped/Postponed/Error) from the closing event's payload state; sentinel-gated column + mapper + AppsDashboardProjection. Status stays "Succeeded" for compatibility. - MetricsMath.IsSkipTerminalState/HasMeasuredDuration: durations computed over measured rows only; skips leave the failure/success rate (decision 2026-07-26 — a skip is not an install attempt); per-app skipped/unmeasured counts + totals exposed; slowest ranking gates on measured installs. Applied consistently: app metrics payload, apps dashboard (list/analytics/ time series/trend/versions/device models), SLA weekly/snapshot/breach. - StoreAppInstallSummaryAsync: reconcile extracted to internal static ReconcileAppInstallSummaryWithExisting (test seam). Fixes Q4 (terminal batch before started batch left duration permanently unset) and hardens terminal status against late/replayed InProgress batches. - Web: measured-only hints on app detail cards, skip suffix in apps list, fleet-health "measured installs" caption. Backend 3333/3333 (20 new tests), web tsc clean, vitest 627/627. Co-Authored-By: Claude Fable 5 --- ...ppInstallTerminalStateAndReconcileTests.cs | 201 ++++++++++++++++++ .../AppsAnalyticsFailureRateTests.cs | 52 +++++ .../MetricsMathAppPayloadTests.cs | 86 ++++++++ .../Functions/Apps/AppsAnalyticsHelper.cs | 62 ++++-- .../Helpers/MetricsMath.cs | 53 ++++- .../EventIngestProcessor.AppInstall.cs | 19 +- .../Services/SlaBreachEvaluationService.cs | 3 + .../Services/SlaMetricsService.cs | 9 +- .../Services/TableStorageService.Metrics.cs | 78 +++++-- .../Models/Metrics/AppInstallSummary.cs | 13 ++ .../app/apps/[appName]/page.tsx | 10 + .../app/apps/components/InstallsTab.tsx | 16 +- .../app/fleet-health/page.tsx | 6 + 13 files changed, 557 insertions(+), 51 deletions(-) create mode 100644 src/Backend/AutopilotMonitor.Functions.Tests/AppInstallTerminalStateAndReconcileTests.cs 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 ? (