fix(replay): attribute deferred violations to the causing step, not detection

This commit is contained in:
pj committed 2026-07-13 16:00:42 +05:30
1 parent cb480d5b28
commit 626158fece
2 files changed
+115 -4

No files matched your search

@@ -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",
+54 -4
View File
@@ -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<number, Step>();
for (const s of steps) if (s) byIndex.set(s.step, s);
const clones = new Map<number, Step>();
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<string>();
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,
};
}