[web-shared] Mark ignored duplicate events in the observability UI - #3467
[web-shared] Mark ignored duplicate events in the observability UI#3467VaguelySerious wants to merge 6 commits into
Conversation
Concurrent replays of one run share an event log, so a replay working from a stale prefix can commit a write the log already records. The runtime passes over those. The UI showed them as ordinary progress and let them move derived state. Derive the set of passed-over events from the log with `entityEventClass`, restricted to the classes a run records at most once per entity. A retried step legitimately repeats `step_started` and `step_retrying`, one per attempt, so those two are excluded and never marked. Marked events read greyed out in the sidebar event list and the events table, with a tooltip saying the event was ignored. They are also kept out of the derived step status, the queued/ran durations, and trace span geometry, where a second terminal event would otherwise stretch a step to whenever the losing replay happened to commit.
🦋 Changeset detectedLatest commit: 8955cce The changes in this PR will be included in the next version bump. This PR includes changesets to release 16 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Sim WorldSimulated world deterministic testing for races. Traces 🟠 Mint-ordered log — 6 fail of 41 total
Full trace: 🟢 Append-only log — 0 fail of 41 total
Full trace: |
An event the caller cannot identify cannot be marked: callers match on the id, so reporting a missing one tarred every other id-less event with it.
|
(AI) CI state on this branch, for anyone reading the red:
Nothing in this PR touches those paths: the diff is |
| * first so a shorter ID never sorts after a longer one on a fixture or a log | ||
| * that mixes widths. | ||
| */ | ||
| function compareLogPosition(a: Event, b: Event): number { |
There was a problem hiding this comment.
AI Review: eventId is not the authoritative order for every supported history. world-local uses the ID as a sort key only for slot-numbered runs; for legacy ULID runs, eventSortKey() returns null and events.list() deliberately orders by (createdAt, eventId). Since createdAt is captured at create() entry and the ULID is minted later, concurrent writers can produce opposite timestamp and ID orders. The runtime then consumes the backend timestamp-ordered log while this UI fold can reverse the canonical and duplicate events. Please pass an authoritative backend log-position/order key into this classifier (or otherwise preserve a guaranteed authoritative ordering contract) rather than assuming every event ID is a log position, and add a legacy-ULID test where createdAt and eventId disagree.
| closedEntities.add(entity); | ||
| } | ||
|
|
||
| if (!repeatsClass) { |
There was a problem hiding this comment.
AI Review: The fold records a previously unseen class even when it first appears after the entity has already closed. For [step_created, step_completed, step_started, step_started], the runtime stops with replay divergence on the first trailing step_started; it never reaches or classifies the second one. This fold leaves the first unmarked, adds step_started to seenClasses, then falsely marks the second as an ignored duplicate. Once a closed entity encounters an unseen class, classification for that entity should become indeterminate (or the fold should stop), and that class should not be added to seenClasses. Please add this four-event sequence to the shared runtime/UI fixtures.
2 participants
#3381 makes the runtime pass over an event that repeats a class the log already records for the same entity. Nothing about that reaches the UI: events are immutable and carry no "ignored" marker, so the observability UI showed such an event as ordinary progress and let it move derived state.
What this does
Adds
findDuplicateEventIds(events, { isCompleteHistory })to@workflow/web-shared, which derives the passed-over set from the event list usingentityEventClassfrom@workflow/world(added in #3381).The rule follows consumer lifetime rather than event type. The runtime steps over an event only when its class was already recorded for the entity and no registered callback claims it, and a callback stays registered for as long as the entity is open. So a repeat counts here only once a terminal event for the same entity (
step_terminal,wait_completed,hook_disposed) sits earlier in the log, which is the point past which no consumer remains.run_startedis the one class with no entity to close first:workflow.tsdeclines a second one outright.That means a retried step's repeated
step_started/step_retrying, a secondstep_createdon a step still in flight, and repeatedhook_receiveddeliveries are all left alone. Astep_startedafter the step's outcome is marked.The fold walks in log order, sorting on
eventId(fixed-width and monotonic within a run under both id schemes). Timestamps are not authoritative: a writer stampscreatedAton entry but takes its log position at publish time, andoccurredAtis measured on the client.Two surfaces show it:
gray-700) with a tooltip.Three derived views stop counting them:
materializeStepskeeps the outcome the run acted on. Before, astep_completedwritten by a losing replay after astep_failedflipped the materialized status.buildDurationMapmeasures "Ran for" against the terminal event the run acted on, not a later repeat.buildTracefilters them before grouping, so a span bar ends where the step ended rather than where the losing replay committed. The event lists still receive the full log, so a marked event is still listed under its entity.Incomplete histories
Which occurrence of a class came first is a property of the whole log. On a page of a paginated list or the result of a search, the earlier event can be missing, and the fold would report the surviving one instead. So
findDuplicateEventIdsrequires the caller to vouch for completeness and classifies nothing otherwise:EventListViewpasses!hasMoreEvents && !isExactSearchActive,TraceViewerpasses!hasMore. The derived-state helpers take the same flag and default it to false, so an unset caller behaves as it did before this PR. The sidebar only ever sees one entity's slice, so it takes the answer as data from the caller that holds the whole log.Wording
The tooltip says what the log shows rather than what the runtime did with it: "Written by a concurrent replay after an event of the same kind was already recorded and acted on. The run follows the earlier one." Tolerating these repeats is recent, and on a run recorded before #3381 an unclaimed repeat failed the replay rather than being passed over.
Tests
packages/world/src/test-support/duplicate-event-fixtures.tsholds nine logs and the indices no consumer claims. Both sides run them:packages/core/src/duplicate-event-fixtures.test.tsdrives them throughEventsConsumerwith consumers modeling the lifetimes instep.ts/sleep.ts/workflow.ts, andduplicate-events.test.tsruns the same fixtures through the UI classifier. A fixture whose expectation moves fails on both sides.Beyond the fixtures,
duplicate-events.test.ts,event-materialization.test.ts, andtrace-builder.test.tscover log order conflicting withcreatedAtand withoccurredAt, tied timestamps in both caller sort directions, the incomplete-history path, the preserved step outcome, the attempt count on a retried step, and the span end time.No change was needed in
packages/web: it rendersEventListViewfromweb-shared, and its flow-graph mapper already takes the first event per correlation id.