diff --git a/docs/design/enrollment-status-reclassification-plan.md b/docs/design/enrollment-status-reclassification-plan.md index 8ff5eca3..9084afbb 100644 --- a/docs/design/enrollment-status-reclassification-plan.md +++ b/docs/design/enrollment-status-reclassification-plan.md @@ -75,16 +75,19 @@ timeline AND let the backend terminalize precisely instead of guessing with grac - [x] Tests `SessionStatusTransitionTests` (20, full matrix). Suite: 2705 pass (one pre-existing timing-flaky maintenance test, green on retry/isolation — unrelated). -## PR4 — Stats / metrics expose 3 states +## PR4 — Stats / metrics expose 3 states ✅ done (Fleet Health deferred to PR5) -- [ ] `SessionStatusBuckets` (`MetricsMath.cs:317`): explicit `AwaitingUser` + `Incomplete` - buckets (out of `Other`). -- [ ] `SessionStats` (`SessionApiModels.cs:442`): add `IncompleteLastNDays`; failure-rate - denominator = `Succeeded + Failed` only. -- [ ] Fleet Health payload (`MetricsMath.BuildFleetHealthPayload`): count Incomplete - separately; keep SuccessRate honest. -- [ ] Aggregation equivalence tests (`SessionStatsAggregationTests`, - `SessionStatsProjectionEquivalenceTests`, `MetricsSummaryProjectionEquivalenceTests`). +- [x] `SessionStatusBuckets` (`MetricsMath.cs`): explicit `AwaitingUser` + `Incomplete` buckets + (out of `Other`); buckets still reconcile to Total by construction. +- [x] Metrics summary (`GetMetricsSummaryAsync`): surface `awaitingUser` + `incomplete`; **failure + rate now `Failed / (Succeeded + Failed)`** — Incomplete + non-terminal excluded from the denominator. +- [x] `SessionStats` (`SessionApiModels.cs`): add `IncompleteLastNDays`; dashboard `SuccessRatePct` + was already terminal-only (`Succeeded/(Succeeded+Failed)`), so Incomplete is excluded there too. +- [x] Daily aggregation (`ComputeUsageMetricsSnapshotAsync`) already uses `Succeeded+Failed` as the + denominator — no change needed (Incomplete already excluded). +- [x] Tests `SessionStatusBucketsTests` (bucketing + terminal-only failure-rate). Suite: 2705 pass. +- [ ] **Deferred to PR5:** Fleet Health payload Incomplete count (its SuccessRate is documented as + "over all sessions" with equivalence tests — fold the Incomplete surfacing in with the web work). ## PR5 — Web surface diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/SessionStatusBucketsTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/SessionStatusBucketsTests.cs new file mode 100644 index 00000000..42bcaeca --- /dev/null +++ b/src/Backend/AutopilotMonitor.Functions.Tests/SessionStatusBucketsTests.cs @@ -0,0 +1,58 @@ +using AutopilotMonitor.Functions.Helpers; + +namespace AutopilotMonitor.Functions.Tests; + +/// +/// Pins the third-state stats bucketing (docs/design/enrollment-status-reclassification.md): +/// AwaitingUser and Incomplete get their own buckets instead of hiding in Other, and the buckets +/// always reconcile to Total by construction — so a session in either state can never be silently +/// miscounted as a failure. +/// +public class SessionStatusBucketsTests +{ + private static SessionStatusBuckets Tally(params string[] statuses) + { + var b = default(SessionStatusBuckets); + foreach (var s in statuses) b = b.Add(s); + return b; + } + + [Fact] + public void Incomplete_and_AwaitingUser_get_their_own_buckets_not_Other() + { + var b = Tally("Incomplete", "Incomplete", "AwaitingUser"); + Assert.Equal(2, b.Incomplete); + Assert.Equal(1, b.AwaitingUser); + Assert.Equal(0, b.Other); + } + + [Fact] + public void Unknown_status_still_lands_in_Other() + { + var b = Tally("Succeeded", "SomethingWeird"); + Assert.Equal(1, b.Succeeded); + Assert.Equal(1, b.Other); + } + + [Fact] + public void Buckets_always_reconcile_to_total() + { + var b = Tally("Succeeded", "Failed", "Incomplete", "AwaitingUser", "InProgress", "Pending", "Stalled", "weird"); + Assert.Equal(8, b.Total); + Assert.Equal(b.Total, + b.Succeeded + b.Failed + b.Incomplete + b.AwaitingUser + b.InProgress + b.Pending + b.Stalled + b.Other); + } + + [Fact] + public void Failure_rate_denominator_is_terminal_only_excluding_incomplete() + { + // 1 succeeded, 1 failed, 8 incomplete (the crcins shape). Honest rate = 1/(1+1) = 50%, + // NOT 1/10 = 10% (Incomplete must not dilute) and NOT counting Incomplete as failures. + var b = Tally("Succeeded", "Failed", "Incomplete", "Incomplete", "Incomplete", "Incomplete", + "Incomplete", "Incomplete", "Incomplete", "Incomplete"); + var terminal = b.Succeeded + b.Failed; + var failureRate = terminal > 0 ? (double)b.Failed / terminal * 100 : 0; + Assert.Equal(50.0, failureRate); + Assert.Equal(8, b.Incomplete); + } +} diff --git a/src/Backend/AutopilotMonitor.Functions/Helpers/MetricsMath.cs b/src/Backend/AutopilotMonitor.Functions/Helpers/MetricsMath.cs index 0d69c997..b63dd278 100644 --- a/src/Backend/AutopilotMonitor.Functions/Helpers/MetricsMath.cs +++ b/src/Backend/AutopilotMonitor.Functions/Helpers/MetricsMath.cs @@ -315,7 +315,8 @@ public static class MetricsMath /// their own buckets, and any unrecognised status (incl. Unknown) lands in . /// public readonly record struct SessionStatusBuckets( - int Total, int Succeeded, int Failed, int InProgress, int Pending, int Stalled, int Other) + int Total, int Succeeded, int Failed, int InProgress, int Pending, int Stalled, + int AwaitingUser, int Incomplete, int Other) { /// Returns a new tally with folded in. public SessionStatusBuckets Add(string? status) @@ -326,7 +327,11 @@ public readonly record struct SessionStatusBuckets( var inProgress = InProgress + (status == "InProgress" ? 1 : 0); var pending = Pending + (status == "Pending" ? 1 : 0); var stalled = Stalled + (status == "Stalled" ? 1 : 0); - var other = total - (succeeded + failed + inProgress + pending + stalled); - return new SessionStatusBuckets(total, succeeded, failed, inProgress, pending, stalled, other); + // AwaitingUser (non-terminal, Device Setup done) and Incomplete (terminal, non-failure) get + // their own buckets so they no longer hide in Other and never inflate the failure count. + var awaitingUser = AwaitingUser + (status == "AwaitingUser" ? 1 : 0); + var incomplete = Incomplete + (status == "Incomplete" ? 1 : 0); + var other = total - (succeeded + failed + inProgress + pending + stalled + awaitingUser + incomplete); + return new SessionStatusBuckets(total, succeeded, failed, inProgress, pending, stalled, awaitingUser, incomplete, other); } } diff --git a/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.AgentApi.cs b/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.AgentApi.cs index 24e08a8c..c771be48 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.AgentApi.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.AgentApi.cs @@ -1315,20 +1315,30 @@ namespace AutopilotMonitor.Functions.Services TallyMetricsSummaryRow(groups, entity); } - return groups.Select(kvp => (object)new + return groups.Select(kvp => { - tenantId = kvp.Key, - totalSessions = kvp.Value.Total, - succeeded = kvp.Value.Succeeded, - failed = kvp.Value.Failed, - inProgress = kvp.Value.InProgress, - pending = kvp.Value.Pending, - stalled = kvp.Value.Stalled, - other = kvp.Value.Other, - failureRate = kvp.Value.Total > 0 - ? Math.Round((double)kvp.Value.Failed / kvp.Value.Total * 100, 1) - : 0.0, - windowDays = days + var b = kvp.Value; + // Honest failure rate: failed over the TERMINAL outcomes only (Succeeded + Failed). + // Incomplete (unknown, non-failure) is deliberately excluded from the denominator, and + // non-terminal states (InProgress/AwaitingUser/Pending/Stalled) never belonged in it. + var terminal = b.Succeeded + b.Failed; + return (object)new + { + tenantId = kvp.Key, + totalSessions = b.Total, + succeeded = b.Succeeded, + failed = b.Failed, + inProgress = b.InProgress, + pending = b.Pending, + stalled = b.Stalled, + awaitingUser = b.AwaitingUser, + incomplete = b.Incomplete, + other = b.Other, + failureRate = terminal > 0 + ? Math.Round((double)b.Failed / terminal * 100, 1) + : 0.0, + windowDays = days + }; }).ToList(); } } diff --git a/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Sessions.cs b/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Sessions.cs index 232842d9..e7e49ed7 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Sessions.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Sessions.cs @@ -1033,6 +1033,7 @@ namespace AutopilotMonitor.Functions.Services int active = 0; int succeeded = 0; int failed = 0; + int incomplete = 0; long succeededDurationSeconds = 0; int succeededWithDurationCount = 0; int totalToday = 0; @@ -1061,6 +1062,10 @@ namespace AutopilotMonitor.Functions.Services case SessionStatus.Failed: failed++; break; + case SessionStatus.Incomplete: + // Terminal, non-failure. Reported separately, kept out of the failure rate. + incomplete++; + break; } if (s.StartedAt >= utcMidnight) @@ -1085,6 +1090,7 @@ namespace AutopilotMonitor.Functions.Services TotalLastNDays = sessions.Count, SucceededLastNDays = succeeded, FailedLastNDays = failed, + IncompleteLastNDays = incomplete, SuccessRatePct = successRatePct, AvgDurationMinutes = avgDurationMinutes, TotalToday = totalToday, diff --git a/src/Shared/AutopilotMonitor.Shared/Models/SessionApiModels.cs b/src/Shared/AutopilotMonitor.Shared/Models/SessionApiModels.cs index 98d94169..0adc5ffa 100644 --- a/src/Shared/AutopilotMonitor.Shared/Models/SessionApiModels.cs +++ b/src/Shared/AutopilotMonitor.Shared/Models/SessionApiModels.cs @@ -463,6 +463,13 @@ namespace AutopilotMonitor.Shared.Models public int SucceededLastNDays { get; set; } public int FailedLastNDays { get; set; } + /// + /// Terminal, non-failure sessions in the window (docs/design/enrollment-status-reclassification.md): + /// the sweep saw no completion or explicit failure. Reported as the third headline bucket and + /// deliberately excluded from (which is over Succeeded + Failed only). + /// + public int IncompleteLastNDays { get; set; } + /// /// Succeeded / (Succeeded + Failed) * 100, rounded. Zero when no terminal /// sessions are in the window (the card renders "0%" rather than NaN).