diff --git a/internal/replay/runs_cache.go b/internal/replay/runs_cache.go index 3cc87a5..0adf27a 100644 --- a/internal/replay/runs_cache.go +++ b/internal/replay/runs_cache.go @@ -91,7 +91,7 @@ func scanSteps(tracePath string) ([]StepSummary, []int64, int, time.Time, error) reader := bufio.NewReaderSize(file, 64*1024) steps := []StepSummary{} offsets := []int64{} - violationCount := 0 + attributions := []violationAttribution{} var offset int64 for { lineStart := offset @@ -102,19 +102,20 @@ func scanSteps(tracePath string) ([]StepSummary, []int64, int, time.Time, error) trimmed = trimmed[:len(trimmed)-1] } if len(trimmed) > 0 { - summary, partial, decodeErr := decodeStepSummary(trimmed) + summary, lineAttributions, decodeErr := decodeStepSummary(trimmed) if decodeErr != nil { return nil, nil, 0, time.Time{}, decodeErr } steps = append(steps, summary) offsets = append(offsets, lineStart) - violationCount += partial + attributions = append(attributions, lineAttributions...) } if err != nil { break } } - return steps, offsets, violationCount, info.ModTime(), nil + markViolations(steps, attributions) + return steps, offsets, len(attributions), info.ModTime(), nil } // Step decodes the full Step record at index n (1-based, matching trace.Step.Index). diff --git a/internal/replay/runs_decode.go b/internal/replay/runs_decode.go index 553cffb..5f5185f 100644 --- a/internal/replay/runs_decode.go +++ b/internal/replay/runs_decode.go @@ -56,7 +56,16 @@ func tallyTrace(tracePath string) (steps, violations int, err error) { return steps, violations, nil } -func decodeStepSummary(line []byte) (StepSummary, int, error) { +// violationAttribution maps one recorded violation to the step the marker +// belongs on: the causing step its witness names, falling back to the step +// whose trace line carries it (the detection step) for witnesses written +// before the step field existed. +type violationAttribution struct { + attributedStep int + detectedStep int +} + +func decodeStepSummary(line []byte) (StepSummary, []violationAttribution, error) { var partial struct { Index int `json:"step"` Timestamp time.Time `json:"timestamp"` @@ -76,15 +85,28 @@ func decodeStepSummary(line []byte) (StepSummary, int, error) { } `json:"next_action,omitempty"` Exceptions []json.RawMessage `json:"exceptions,omitempty"` Violations []string `json:"violations,omitempty"` + Witnesses map[string]struct { + Step int `json:"step"` + } `json:"witnesses,omitempty"` } if err := json.Unmarshal(line, &partial); err != nil { - return StepSummary{}, 0, fmt.Errorf("decode step: %w", err) + return StepSummary{}, nil, fmt.Errorf("decode step: %w", err) + } + var attributions []violationAttribution + for _, name := range partial.Violations { + attribution := violationAttribution{ + attributedStep: partial.Index, + detectedStep: partial.Index, + } + if witness, ok := partial.Witnesses[name]; ok && witness.Step > 0 { + attribution.attributedStep = witness.Step + } + attributions = append(attributions, attribution) } summary := StepSummary{ Index: partial.Index, Timestamp: partial.Timestamp, Screen: partial.Screen, - HasViolations: len(partial.Violations) > 0, HasExceptions: len(partial.Exceptions) > 0, } if partial.NextAction != nil { @@ -113,7 +135,33 @@ func decodeStepSummary(line []byte) (StepSummary, int, error) { } } } - return summary, len(partial.Violations), nil + return summary, attributions, nil +} + +// markViolations sets HasViolations on the step each attribution points at. +// The marker goes on the causing step; when that index is missing from the +// trace the detection step keeps it. Duplicate indices (a finalize line echoes +// the last step's index) resolve to the first occurrence, the real step. +func markViolations(steps []StepSummary, attributions []violationAttribution) { + if len(attributions) == 0 { + return + } + positionOf := make(map[int]int, len(steps)) + for position, step := range steps { + if _, ok := positionOf[step.Index]; !ok { + positionOf[step.Index] = position + } + } + for _, attribution := range attributions { + position, ok := positionOf[attribution.attributedStep] + if !ok { + position, ok = positionOf[attribution.detectedStep] + if !ok { + continue + } + } + steps[position].HasViolations = true + } } func swipeDirectionLabel(fromX, fromY, toX, toY int) string { diff --git a/internal/replay/runs_test.go b/internal/replay/runs_test.go index c0ce1aa..2d3b39a 100644 --- a/internal/replay/runs_test.go +++ b/internal/replay/runs_test.go @@ -138,6 +138,66 @@ func TestCacheStep_LazyDecodeReturnsFullStep(t *testing.T) { } } +func TestCacheOpen_ViolationMarkerMovesToCausingStep(t *testing.T) { + // The violation is detected at step 3 but its witness attributes it to + // step 2 (the step that spawned the next obligation). The action-list + // marker belongs on step 2; the full step 3 payload keeps the violation + // record itself. + root := t.TempDir() + startedAt := time.Now().UTC() + steps := []trace.Step{ + {Index: 1, Timestamp: startedAt}, + {Index: 2, Timestamp: startedAt.Add(time.Second)}, + { + Index: 3, + Timestamp: startedAt.Add(2 * time.Second), + Violations: []string{"prop1"}, + Witnesses: map[string]trace.Witness{"prop1": {Reason: "predicate false", Step: 2}}, + }, + } + writeRun(t, root, "r1", trace.Meta{StartedAt: startedAt, EndedAt: timePointer(startedAt.Add(3 * time.Second))}, steps) + + cache := NewCache(root) + run, err := cache.Open("r1") + if err != nil { + t.Fatalf("Open: %v", err) + } + if !run.Steps[1].HasViolations { + t.Error("step 2 (causing step) should carry the violation marker") + } + if run.Steps[2].HasViolations { + t.Error("step 3 (detection step) should not carry the marker") + } + full, err := cache.Step(run, 3) + if err != nil { + t.Fatalf("Step(3): %v", err) + } + if len(full.Violations) != 1 || full.Violations[0] != "prop1" { + t.Errorf("step 3 payload violations = %v, want [prop1]", full.Violations) + } +} + +func TestCacheOpen_ViolationWithoutWitnessStepKeepsDetectionStep(t *testing.T) { + // Traces written before witnesses carried a step field fall back to + // marking the detection step, the old behavior. + root := t.TempDir() + startedAt := time.Now().UTC() + steps := []trace.Step{ + {Index: 1, Timestamp: startedAt}, + {Index: 2, Timestamp: startedAt.Add(time.Second), Violations: []string{"prop1"}}, + } + writeRun(t, root, "r1", trace.Meta{StartedAt: startedAt, EndedAt: timePointer(startedAt.Add(2 * time.Second))}, steps) + + cache := NewCache(root) + run, err := cache.Open("r1") + if err != nil { + t.Fatalf("Open: %v", err) + } + if !run.Steps[1].HasViolations { + t.Error("step 2 should keep the marker when the witness has no step") + } +} + func TestDecodeStepSummary_ActionLabelPerKind(t *testing.T) { cases := []struct { line string