From 626158fece4a67f3d8b594fe2cb807383e3b25bf Mon Sep 17 00:00:00 2001 From: PJ Date: Mon, 13 Jul 2026 16:00:42 +0530 Subject: [PATCH] fix(replay): attribute deferred violations to the causing step, not detection --- replay-ui/src/__tests__/run-history.test.ts | 61 +++++++++++++++++++++ replay-ui/src/lib/run-history.ts | 58 ++++++++++++++++++-- 2 files changed, 115 insertions(+), 4 deletions(-) diff --git a/replay-ui/src/__tests__/run-history.test.ts b/replay-ui/src/__tests__/run-history.test.ts index e9bc1d3..1a2dc85 100644 --- a/replay-ui/src/__tests__/run-history.test.ts +++ b/replay-ui/src/__tests__/run-history.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect } from "bun:test"; import { buildRunHistory, collectPropertyNames, + relocateViolationsToCause, sortLanes, statusForProperty, } from "../lib/run-history"; @@ -52,7 +53,67 @@ describe("sortLanes", () => { }); }); +// Bug class: a next/eventually violation records on the DETECTION step but is +// caused earlier; leaving it on the detection step lights up an unrelated +// action in the Violations tab while the timeline dot sits on the cause step. +describe("relocateViolationsToCause", () => { + it("moves a deferred violation to the step its witness blames", () => { + const steps = [ + step({ step: 290, residuals: { p: { op: "predicate", name: "p3" } } }), + step({ + step: 291, + violations: ["p"], + witnesses: { p: { step: 290, reason: "predicate false" } }, + residuals: { p: { op: "false" } }, + }), + step({ step: 292 }), + ]; + const moved = relocateViolationsToCause(steps); + + expect(moved[0]?.violations).toEqual(["p"]); + expect(moved[0]?.witnesses?.p?.reason).toBe("predicate false"); + expect(moved[1]?.violations).toEqual([]); + expect(moved[1]?.witnesses?.p).toBeUndefined(); + // originals are cloned, never mutated + expect(steps[1]?.violations).toEqual(["p"]); + expect(steps[1]?.witnesses?.p?.step).toBe(290); + }); + + it("leaves a violation without a witness on its detection step", () => { + const steps = [step({ step: 5, violations: ["p"], residuals: { p: { op: "false" } } })]; + expect(relocateViolationsToCause(steps)).toBe(steps); + }); + + it("keeps the violation put when the blamed step is absent from the trace", () => { + const steps = [step({ step: 9, violations: ["p"], witnesses: { p: { step: 3 } } })]; + expect(relocateViolationsToCause(steps)[0]?.violations).toEqual(["p"]); + }); +}); + describe("buildRunHistory", () => { + it("anchors a deferred violation's lane cell to the cause step, not detection", () => { + const run = { + id: "run-2", + steps: [summary({ index: 290, has_violations: true }), summary({ index: 291 })], + } as unknown as Run; + const responses = [ + step({ step: 290, residuals: { p: { op: "predicate" } } }), + step({ + step: 291, + violations: ["p"], + witnesses: { p: { step: 290 } }, + residuals: { p: { op: "false" } }, + }), + ]; + + const history = buildRunHistory(run, responses); + + expect(history.lanes[0].statuses).toEqual(["violated", "pending"]); + expect(history.steps[0]?.violations).toEqual(["p"]); + expect(history.steps[1]?.violations).toEqual([]); + expect(history.firstViolationStep).toBe(290); + }); + it("aligns lane statuses, metrics samples, and first-violation index by position", () => { const run = { id: "run-1", diff --git a/replay-ui/src/lib/run-history.ts b/replay-ui/src/lib/run-history.ts index 5103100..09e925f 100644 --- a/replay-ui/src/lib/run-history.ts +++ b/replay-ui/src/lib/run-history.ts @@ -14,6 +14,55 @@ export interface RunHistory { steps: (Step | null)[]; } +// A next/eventually obligation is evaluated one or more steps AFTER the action +// that armed it, so the checker records the violation on the DETECTION step +// while its witness names the CAUSE step. The timeline dot (the backend's +// markViolations) already sits on the cause step; mirror that here so the +// Violations tab and property lanes light up on the same step — the guilty +// action — not the unrelated action that happened to be running when the +// obligation resolved. Steps are cloned, never mutated in place. +export function relocateViolationsToCause( + steps: (Step | null)[], +): (Step | null)[] { + const byIndex = new Map(); + for (const s of steps) if (s) byIndex.set(s.step, s); + + const clones = new Map(); + const clone = (s: Step): Step => { + let c = clones.get(s.step); + if (!c) { + c = { + ...s, + violations: [...(s.violations ?? [])], + witnesses: { ...(s.witnesses ?? {}) }, + }; + clones.set(s.step, c); + } + return c; + }; + + for (const s of steps) { + for (const name of s?.violations ?? []) { + const cause = s?.witnesses?.[name]?.step; + if (cause === undefined || cause === s?.step) continue; + const target = byIndex.get(cause); + if (!target) continue; + const from = clone(s as Step); + const to = clone(target); + from.violations = (from.violations ?? []).filter((n) => n !== name); + const witness = s?.witnesses?.[name]; + if (witness) { + delete from.witnesses?.[name]; + (to.witnesses ??= {})[name] = witness; + } + if (!(to.violations ?? []).includes(name)) (to.violations ??= []).push(name); + } + } + + if (clones.size === 0) return steps; + return steps.map((s) => (s ? clones.get(s.step) ?? s : s)); +} + export function collectPropertyNames(steps: (Step | null)[]): string[] { const names = new Set(); for (const step of steps) { @@ -47,10 +96,11 @@ export function buildRunHistory( run: Run, responses: (Step | null)[], ): RunHistory { - const propertyNames = collectPropertyNames(responses); + const steps = relocateViolationsToCause(responses); + const propertyNames = collectPropertyNames(steps); const lanes: PropertyLane[] = propertyNames.map((name) => ({ name, - statuses: responses.map((step) => statusForProperty(name, step)), + statuses: steps.map((step) => statusForProperty(name, step)), })); const firstViolationStep = run.steps.find((entry) => entry.has_violations)?.index; const firstExceptionStep = run.steps.find((entry) => entry.has_exceptions)?.index; @@ -63,7 +113,7 @@ export function buildRunHistory( const metricsSamples: MetricsSample[] = run.steps.map((entry, position) => ({ stepIndex: entry.index, timestamp: entry.timestamp, - metrics: responses[position]?.metrics, + metrics: steps[position]?.metrics, })); return { names: propertyNames, @@ -73,6 +123,6 @@ export function buildRunHistory( exceptionStepIndices, violationStepIndices, metricsSamples, - steps: responses, + steps, }; }