From aaacc6ecb21f9041d0c481582bc399b452de2877 Mon Sep 17 00:00:00 2001 From: Oliver Kieselbach Date: Thu, 20 Aug 2026 13:13:09 +0200 Subject: [PATCH] refactor(backend): order rule evaluation by Sequence, not Timestamp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sequence is a persisted, strictly monotonic per-session counter assigned at emit time (EventSequenceCounter: locked, saved before return, survives restarts). It is immune to every clock problem. Timestamp carries the device clock frame plus, for CMTrace-derived events, the writing process's timezone belief. EvaluateAppInstallDurationCondition and EvaluateEventCorrelationCondition both ordered by Timestamp and expressed "A happened before B" as a timestamp comparison. Both now order by Sequence and compare Sequence for ordering. Timestamp deliberately stays where an actual ELAPSED DURATION is meant: durationSeconds, the evidence fields, and TimeWindowSeconds. Scope, honestly: this is hardening, not a live bug fix. BuiltInAnalyzeRules.cs contains zero event_correlation conditions, so the cross-frame join can only be reached by a self-authored rule. And the app-install duration evaluator pairs app_install_started with app_install_completed — both IME-sourced, one frame — so its arithmetic was already correct. (The measured 68-vs-8-minute inflation comes from EventIngestProcessor.AppInstall, which folds DO-collector download_progress together with IME app_install_completed; that is fixed at the root in the agent.) The TimeWindowSeconds check stays exposed: the window IS a duration, so there is no alternative to a timestamp there. A +1 h skew on B makes any window below 3600 s fail to match. Only the agent-side calibration closes that. Co-Authored-By: Claude Opus 5 (1M context) --- .../RuleEngine.ConditionEvaluators.cs | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/src/Backend/AutopilotMonitor.Functions/Services/RuleEngine.ConditionEvaluators.cs b/src/Backend/AutopilotMonitor.Functions/Services/RuleEngine.ConditionEvaluators.cs index ef763463..bffd8ca7 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/RuleEngine.ConditionEvaluators.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/RuleEngine.ConditionEvaluators.cs @@ -752,7 +752,12 @@ namespace AutopilotMonitor.Functions.Services private (bool matched, object evidence) EvaluateAppInstallDurationCondition(RuleCondition condition, List events) { - var sortedEvents = events.OrderBy(e => e.Timestamp).ThenBy(e => e.Sequence).ToList(); + // Ordering key is Sequence, never Timestamp: Sequence is a persisted, strictly + // monotonic per-session counter assigned at emit time and is immune to every clock + // problem, whereas Timestamp carries the device clock frame plus — for CMTrace-derived + // events — the writer process's timezone belief. Timestamp stays in use below only + // where an actual ELAPSED DURATION is meant, which is what it is for. + var sortedEvents = events.OrderBy(e => e.Sequence).ToList(); var completionEventTypes = string.IsNullOrWhiteSpace(condition.EventType) ? new HashSet(new[] { "app_install_completed", "app_install_failed" }, StringComparer.OrdinalIgnoreCase) @@ -774,7 +779,7 @@ namespace AutopilotMonitor.Functions.Services var startEvent = sortedEvents.LastOrDefault(e => (string.Equals(e.EventType, "app_install_started", StringComparison.OrdinalIgnoreCase) || string.Equals(e.EventType, "app_install_start", StringComparison.OrdinalIgnoreCase)) && - e.Timestamp <= completionEvent.Timestamp && + e.Sequence <= completionEvent.Sequence && string.Equals(GetDataFieldValue(e, "appId") ?? GetDataFieldValue(e, "appName"), appKey, StringComparison.OrdinalIgnoreCase)); if (startEvent == null) @@ -826,7 +831,12 @@ namespace AutopilotMonitor.Functions.Services return (false, "event_correlation requires EventType, CorrelateEventType, and JoinField"); } - var sortedEvents = events.OrderBy(e => e.Timestamp).ThenBy(e => e.Sequence).ToList(); + // Ordering key is Sequence, never Timestamp: Sequence is a persisted, strictly + // monotonic per-session counter assigned at emit time and is immune to every clock + // problem, whereas Timestamp carries the device clock frame plus — for CMTrace-derived + // events — the writer process's timezone belief. Timestamp stays in use below only + // where an actual ELAPSED DURATION is meant, which is what it is for. + var sortedEvents = events.OrderBy(e => e.Sequence).ToList(); // Collect Event A candidates with optional filter var eventAList = sortedEvents @@ -874,16 +884,14 @@ namespace AutopilotMonitor.Functions.Services // Find the latest Event A that occurred before Event B var matchingA = eventAByJoinKey[bKey] - .Where(a => a.Timestamp < eventB.Timestamp || - (a.Timestamp == eventB.Timestamp && a.Sequence < eventB.Sequence)) + .Where(a => a.Sequence < eventB.Sequence) .Where(a => { if (condition.TimeWindowSeconds == null || condition.TimeWindowSeconds <= 0) return true; return (eventB.Timestamp - a.Timestamp).TotalSeconds <= condition.TimeWindowSeconds.Value; }) - .OrderByDescending(a => a.Timestamp) - .ThenByDescending(a => a.Sequence) + .OrderByDescending(a => a.Sequence) .FirstOrDefault(); if (matchingA == null)