From ed1c26f98b9b2b374a2d57a62e358a24cd1018ee Mon Sep 17 00:00:00 2001 From: Oliver Kieselbach Date: Fri, 24 Apr 2026 12:00:33 +0200 Subject: [PATCH] v2 codex-03: forward-link events to their originating transition + signal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes Codex review follow-up #3. Two distinct traceability gaps: 1. DecisionTransition.EmittedEventSequences was always empty. The journal record is appended before the effects run, so at construction time the Event.Sequence values haven't been assigned yet. Populating it later would need a journal-reorder or a sidecar mapping file — both more risk than payoff. 2. SessionTraceOrdinal had two independent meanings in the codebase: the reducer-step correlation counter (SessionTraceOrdinalProvider) shared by a signal and its transition, vs. the transport-cursor on TelemetryItem (== TelemetryItemId when session-scoped). The backend parser already prefers the inner (payload) value, but the shared name was misleading. Resolution: - Forward link instead of backward. EnrollmentEvent gets two new nullable columns, CausedByTransitionStepIndex and CausedBySignalOrdinal, populated in EventTimelineEmitter.Emit from currentState.StepIndex and currentState.LastAppliedSignalOrdinal (the reducer already has both, zero new plumbing). Nullable so legacy events and any future non-reducer emits stay valid. Inspector queries "events emitted by step N" = WHERE CausedByTransitionStepIndex = N — trivial index scan. - DecisionTransition.EmittedEventSequences stays on the type for journal backward-compat but the XML doc now states explicitly that it is always empty and points readers at the new forward link. - SessionTraceOrdinal XML docs clarified on DecisionSignal, DecisionTransition (by extension), and TelemetryItem: reducer- correlation counter is authoritative for payload, transport-cursor for envelope, backend parser resolves by preferring inner value. No code change — the two counters were already independent in practice; the name overload was the whole confusion. - Backend TableStorageService persists both new columns in StoreEventAsync + StoreEventsBatchAsync, reads them in MapToEnrollmentEvent. Entity setters guarded by HasValue so legacy agent uploads don't write null-as-zero. Tests (+2 in EventTimelineEmitterTests): - Emit populates CausedByTransitionStepIndex + CausedBySignalOrdinal from a state with StepIndex=7, LastAppliedSignalOrdinal=12. - CausedBySignalOrdinal is null when LastAppliedSignalOrdinal=-1 (fresh initial state sentinel), avoiding a meaningless -1 on the wire. Suites green: DecisionCore 161/161, Agent.V2.Core 645/645 (+2), Functions 734/734. Build 0 warnings. Plan: .claude/plans/codex-03-traceability.md Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Events/EventTimelineEmitterTests.cs | 46 +++++++++++++++++++ .../Telemetry/Events/EventTimelineEmitter.cs | 10 ++++ .../Transport/Telemetry/TelemetryItem.cs | 17 ++++++- .../Services/TableStorageService.Sessions.cs | 19 +++++++- .../Engine/DecisionTransition.cs | 10 +++- .../Signals/DecisionSignal.cs | 16 ++++++- .../Models/Enrollment/EnrollmentEvent.cs | 19 ++++++++ 7 files changed, 133 insertions(+), 4 deletions(-) diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Telemetry/Events/EventTimelineEmitterTests.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Telemetry/Events/EventTimelineEmitterTests.cs index 0801a7af..31fb4483 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Telemetry/Events/EventTimelineEmitterTests.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Telemetry/Events/EventTimelineEmitterTests.cs @@ -494,5 +494,51 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Telemetry.Events var data = (JObject)parsed["Data"]!; Assert.Equal("v-123", (string?)data["verdictId"]); } + + // ============================================================ Codex #3 forward-link + + [Fact] + public void Emit_populates_CausedByTransitionStepIndex_from_currentState_StepIndex() + { + // Codex follow-up #3: every EmitEventTimelineEntry runs as part of a reducer + // step; the event carries the StepIndex forward so Inspector queries can locate + // all events emitted by a specific transition without needing the journal-side + // EmittedEventSequences (which stays empty because event Sequences are assigned + // after the journal record is on disk). + using var r = new Rig(); + var state = DecisionState.CreateInitial("S1", "T1") + .ToBuilder() + .WithStepIndex(7) + .WithLastAppliedSignalOrdinal(12) + .Build(); + + r.Sut.Emit( + new Dictionary { ["eventType"] = "enrollment_complete" }, + state, + At); + + var parsed = JObject.Parse(r.Transport.Enqueued[0].PayloadJson); + Assert.Equal(7L, (long?)parsed["CausedByTransitionStepIndex"]); + Assert.Equal(12L, (long?)parsed["CausedBySignalOrdinal"]); + } + + [Fact] + public void Emit_CausedBySignalOrdinal_is_null_when_state_has_no_applied_signal_yet() + { + // Fresh CreateInitial state has LastAppliedSignalOrdinal=-1 (sentinel for "no + // signal applied yet"). The forward-link is then written as null rather than -1 + // so the nullable column stays absent instead of carrying a meaningless value. + using var r = new Rig(); + var state = DecisionState.CreateInitial("S1", "T1"); // LastAppliedSignalOrdinal == -1 + + r.Sut.Emit( + new Dictionary { ["eventType"] = "agent_started" }, + state, + At); + + var parsed = JObject.Parse(r.Transport.Enqueued[0].PayloadJson); + Assert.Equal(0L, (long?)parsed["CausedByTransitionStepIndex"]); // initial state's StepIndex is 0, still captured + Assert.Null((long?)parsed["CausedBySignalOrdinal"]); + } } } diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Telemetry/Events/EventTimelineEmitter.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Telemetry/Events/EventTimelineEmitter.cs index 3f78525c..2826a74d 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/Telemetry/Events/EventTimelineEmitter.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Telemetry/Events/EventTimelineEmitter.cs @@ -113,6 +113,16 @@ namespace AutopilotMonitor.Agent.V2.Core.Telemetry.Events Timestamp = DateTime.SpecifyKind(occurredAtUtc, DateTimeKind.Utc), ImmediateUpload = ParseImmediateUpload(parameters), Data = ResolveData(parameters, typedPayload), + + // Codex follow-up #3 forward-link: every EmitEventTimelineEntry effect runs + // as part of a reducer step, so currentState.StepIndex == the emitting + // transition's StepIndex and currentState.LastAppliedSignalOrdinal == the + // signal that drove the reduce. Persisting both on the event lets the + // Inspector jump "event → transition → signal" without joining tables. + CausedByTransitionStepIndex = currentState.StepIndex, + CausedBySignalOrdinal = currentState.LastAppliedSignalOrdinal >= 0 + ? currentState.LastAppliedSignalOrdinal + : (long?)null, }; _emitter.Emit(evt); diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Transport/Telemetry/TelemetryItem.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Transport/Telemetry/TelemetryItem.cs index d2c33792..edaaafe0 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/Transport/Telemetry/TelemetryItem.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Transport/Telemetry/TelemetryItem.cs @@ -77,7 +77,22 @@ namespace AutopilotMonitor.Agent.V2.Core.Transport.Telemetry /// Transport-Cursor — monoton pro Session über ALLE Item-Typen. public long TelemetryItemId { get; } - /// Plan §2.2 — equals wenn session-zugehörig, null bei Agent-globalen Items. + /// + /// Transport-envelope view of — present when the + /// item belongs to a session, null for agent-global items. + /// + /// Codex follow-up #3 semantic clarification: this is the transport-cursor + /// counter (spool-item ordering), not the reducer-step correlation counter + /// that lives on DecisionSignal.SessionTraceOrdinal / + /// DecisionTransition.SessionTraceOrdinal. Both counters are monotonic per + /// session but come from independent sources (spool vs. + /// SessionTraceOrdinalProvider). They share a name historically; the + /// backend's TelemetryPayloadParser prefers the inner payload value on + /// ingest for Signals and Transitions, so the reducer-correlation value wins + /// in downstream storage. This envelope value is only used for transport-cursor + /// bookkeeping. + /// + /// public long? SessionTraceOrdinal { get; } /// Bereits fertig serialisiertes JSON der Payload (Event / DecisionSignal / DecisionTransition). diff --git a/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Sessions.cs b/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Sessions.cs index 8a290e07..654e0c51 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Sessions.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Sessions.cs @@ -487,6 +487,14 @@ namespace AutopilotMonitor.Functions.Services if (evt.OriginalTimestamp.HasValue) entity["OriginalTimestamp"] = EnsureUtc(evt.OriginalTimestamp.Value); + // Codex follow-up #3: forward-link columns. Null when absent on the agent + // payload (pre-#3 events, or events emitted outside the reducer pipeline); + // skip the entity setter in that case so the column stays unset. + if (evt.CausedByTransitionStepIndex.HasValue) + entity["CausedByTransitionStepIndex"] = evt.CausedByTransitionStepIndex.Value; + if (evt.CausedBySignalOrdinal.HasValue) + entity["CausedBySignalOrdinal"] = evt.CausedBySignalOrdinal.Value; + await tableClient.UpsertEntityAsync(entity); _logger.LogDebug($"Stored event {evt.EventId}"); @@ -564,6 +572,12 @@ namespace AutopilotMonitor.Functions.Services if (evt.OriginalTimestamp.HasValue) entity["OriginalTimestamp"] = EnsureUtc(evt.OriginalTimestamp.Value); + // Codex follow-up #3: forward-link columns (see StoreEventAsync). + if (evt.CausedByTransitionStepIndex.HasValue) + entity["CausedByTransitionStepIndex"] = evt.CausedByTransitionStepIndex.Value; + if (evt.CausedBySignalOrdinal.HasValue) + entity["CausedBySignalOrdinal"] = evt.CausedBySignalOrdinal.Value; + return new TableTransactionAction(TableTransactionActionType.UpsertReplace, entity); }).ToList(); @@ -1739,7 +1753,10 @@ namespace AutopilotMonitor.Functions.Services RowKey = entity.RowKey, ReceivedAt = entity.GetDateTimeOffset("ReceivedAt")?.UtcDateTime, OriginalTimestamp = entity.GetDateTimeOffset("OriginalTimestamp")?.UtcDateTime, - TimestampClamped = entity.GetBoolean("TimestampClamped") ?? false + TimestampClamped = entity.GetBoolean("TimestampClamped") ?? false, + // Codex follow-up #3 — forward-link columns (nullable for legacy rows). + CausedByTransitionStepIndex = entity.GetInt64("CausedByTransitionStepIndex"), + CausedBySignalOrdinal = entity.GetInt64("CausedBySignalOrdinal"), }; } diff --git a/src/Shared/AutopilotMonitor.DecisionCore/Engine/DecisionTransition.cs b/src/Shared/AutopilotMonitor.DecisionCore/Engine/DecisionTransition.cs index d4ec8d5f..80980624 100644 --- a/src/Shared/AutopilotMonitor.DecisionCore/Engine/DecisionTransition.cs +++ b/src/Shared/AutopilotMonitor.DecisionCore/Engine/DecisionTransition.cs @@ -99,7 +99,15 @@ namespace AutopilotMonitor.DecisionCore.Engine public IReadOnlyList Guards { get; } - /// Plan §2.2 cross-reference — Event.Sequence values this transition emitted as effects. + /// + /// Plan §2.2 cross-reference — Event.Sequence values this transition emitted as + /// effects. Currently always empty (Codex follow-up #3): the journal record + /// is appended before effects run, so the Event.Sequence values are not yet known + /// at construction time. The forward link is instead carried by + /// EnrollmentEvent.CausedByTransitionStepIndex — query the Events table by + /// StepIndex for the "events this step emitted" lookup. The property stays on the + /// type for journal backwards-compatibility and for a potential future sidecar fill. + /// public IReadOnlyList EmittedEventSequences { get; } public ClassifierVerdict? ClassifierVerdict { get; } diff --git a/src/Shared/AutopilotMonitor.DecisionCore/Signals/DecisionSignal.cs b/src/Shared/AutopilotMonitor.DecisionCore/Signals/DecisionSignal.cs index a2b2ca8d..169aa365 100644 --- a/src/Shared/AutopilotMonitor.DecisionCore/Signals/DecisionSignal.cs +++ b/src/Shared/AutopilotMonitor.DecisionCore/Signals/DecisionSignal.cs @@ -52,7 +52,21 @@ namespace AutopilotMonitor.DecisionCore.Signals /// Monotonic per SignalLog; RowKey for Azure Signals table. public long SessionSignalOrdinal { get; } - /// Session-wide monotonic across all telemetry types (Event + Signal + Transition). Inspector correlation only. + /// + /// Session-wide monotonic reducer-step correlation counter — a signal and the + /// transition it produces share the same value. Assigned by + /// SessionTraceOrdinalProvider.Next() in SignalIngress.ProcessItem + /// and copied by the reducer into DecisionTransition.SessionTraceOrdinal. + /// + /// Codex follow-up #3: this is not the same counter as + /// Transport.Telemetry.TelemetryItem.SessionTraceOrdinal, which is the + /// transport-cursor (== TelemetryItemId). They are both monotonic per + /// session and can coincide numerically on fresh sessions, but their sources are + /// independent. The backend's TelemetryPayloadParser resolves the + /// ambiguity by preferring the inner (payload) value on ingest, so the + /// reducer-correlation semantics are authoritative end-to-end. + /// + /// public long SessionTraceOrdinal { get; } public DecisionSignalKind Kind { get; } diff --git a/src/Shared/AutopilotMonitor.Shared/Models/Enrollment/EnrollmentEvent.cs b/src/Shared/AutopilotMonitor.Shared/Models/Enrollment/EnrollmentEvent.cs index a39230d6..da0f345f 100644 --- a/src/Shared/AutopilotMonitor.Shared/Models/Enrollment/EnrollmentEvent.cs +++ b/src/Shared/AutopilotMonitor.Shared/Models/Enrollment/EnrollmentEvent.cs @@ -144,6 +144,25 @@ namespace AutopilotMonitor.Shared.Models [JsonPropertyName("rowKey")] public string RowKey { get; set; } = default!; + /// + /// Reducer StepIndex of the DecisionTransition whose + /// EmitEventTimelineEntry effect produced this event. Nullable for + /// forward-compat (events that pre-date Codex follow-up #3 or that are emitted + /// outside the reducer pipeline). Together with + /// this replaces the always-empty DecisionTransition.EmittedEventSequences + /// — the forward link lives on the event side because the event's Sequence + /// is assigned after the journal record is already on disk. + /// + public long? CausedByTransitionStepIndex { get; set; } + + /// + /// SessionSignalOrdinal of the signal that drove the transition which emitted + /// this event. Nullable for the same reason as + /// . Lets the Inspector jump + /// "event → causing signal" without joining through the transitions table. + /// + public long? CausedBySignalOrdinal { get; set; } + public EnrollmentEvent() { EventId = Guid.NewGuid().ToString();