session/replaytest: address review feedback

Author: Wsp030914

session/replaytest/cases_test.go

@@ -217,6 +217,147 @@ func TestConcurrentSnapshotInvariant(t *testing.T) {
 	}
 }
 
+func TestReplayInvariantValidationErrors(t *testing.T) {
+	tests := []struct {
+		name     string
+		check    func(Snapshot) error
+		snapshot Snapshot
+		want     string
+	}{
+		{
+			name: "summary replay session count", check: validateSummaryReplayWindow,
+			want: "session count",
+		},
+		{
+			name: "summary replay events", check: validateSummaryReplayWindow,
+			snapshot: Snapshot{Sessions: []SessionSnapshot{{ID: standardSessionID}}},
+			want:     "retained events",
+		},
+		{
+			name: "event count", check: validateEvents(eventExpectation{content: "expected"}),
+			snapshot: Snapshot{Sessions: []SessionSnapshot{{ID: standardSessionID}}},
+			want:     "event count",
+		},
+		{
+			name: "tool event count", check: validateToolCall,
+			snapshot: Snapshot{Sessions: []SessionSnapshot{{ID: standardSessionID}}},
+			want:     "event count",
+		},
+		{
+			name: "state value", check: validateStateUpdate,
+			snapshot: Snapshot{Sessions: []SessionSnapshot{{ID: standardSessionID}}},
+			want:     "state",
+		},
+		{
+			name: "summary update session count", check: validateSummaryUpdate,
+			want: "session count",
+		},
+		{
+			name: "tracks missing", check: validateTracks,
+			snapshot: Snapshot{Sessions: []SessionSnapshot{{ID: standardSessionID}}},
+			want:     "tracks",
+		},
+		{
+			name: "concurrent unknown session", check: validateConcurrentSnapshot,
+			snapshot: Snapshot{Sessions: []SessionSnapshot{{ID: "unknown"}, {ID: "session-2"}}},
+			want:     "unexpected session",
+		},
+		{
+			name: "recovery event contents", check: validateRecoverySnapshot,
+			snapshot: Snapshot{
+				Sessions: []SessionSnapshot{{
+					Events: []EventSnapshot{{Content: "wrong"}, {Content: "retried"}},
+					State:  map[string]StateValueSnapshot{"status": JSONStateValue("recovered")},
+				}},
+				Memories: []MemorySnapshot{{}},
+			},
+			want: "event contents",
+		},
+		{
+			name: "wrong only session id",
+			check: func(snapshot Snapshot) error {
+				_, err := onlySession(snapshot, standardSessionID)
+				return err
+			},
+			snapshot: Snapshot{Sessions: []SessionSnapshot{{ID: "wrong"}}},
+			want:     "session id",
+		},
+		{
+			name: "missing named session",
+			check: func(snapshot Snapshot) error {
+				_, err := findSessionSnapshot(snapshot, standardSessionID)
+				return err
+			},
+			want: "not found",
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			err := test.check(test.snapshot)
+			if err == nil || !strings.Contains(err.Error(), test.want) {
+				t.Fatalf("invariant error = %v, want %q", err, test.want)
+			}
+		})
+	}
+}
+
+func TestValidateSummaryRejectsInvalidPersistedFields(t *testing.T) {
+	valid := SummarySnapshot{
+		SessionID: standardSessionID,
+		FilterKey: "branch/main",
+		Text:      "summary",
+		Version:   1,
+		UpdatedAt: standardTime,
+		Boundary: map[string]any{
+			"filter_key": "branch/main", "last_event_id": "event-1", "cutoff_at": standardTime,
+		},
+	}
+	tests := []struct {
+		name   string
+		mutate func(*SummarySnapshot)
+		want   string
+	}{
+		{name: "main fields", mutate: func(summary *SummarySnapshot) {
+			summary.Version = 0
+		}, want: "summary ="},
+		{name: "boundary filter", mutate: func(summary *SummarySnapshot) {
+			summary.Boundary["filter_key"] = "wrong"
+		}, want: "filter_key"},
+		{name: "boundary event", mutate: func(summary *SummarySnapshot) {
+			delete(summary.Boundary, "last_event_id")
+		}, want: "last_event_id"},
+		{name: "boundary cutoff", mutate: func(summary *SummarySnapshot) {
+			delete(summary.Boundary, "cutoff_at")
+		}, want: "cutoff_at"},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			summary := valid
+			summary.Boundary = make(map[string]any, len(valid.Boundary))
+			for key, value := range valid.Boundary {
+				summary.Boundary[key] = value
+			}
+			test.mutate(&summary)
+			err := validateSummary(summary, standardSessionID, "summary")
+			if err == nil || !strings.Contains(err.Error(), test.want) {
+				t.Fatalf("validateSummary() error = %v, want %q", err, test.want)
+			}
+		})
+	}
+}
+
+func TestSameStringSetRejectsInvalidValues(t *testing.T) {
+	if sameStringSet([]any{"one"}, "one", "two") {
+		t.Fatal("sameStringSet() accepted mismatched lengths")
+	}
+	if sameStringSet([]any{1}, "one") {
+		t.Fatal("sameStringSet() accepted a non-string value")
+	}
+	if sameStringSet([]any{"two"}, "one") {
+		t.Fatal("sameStringSet() accepted an unexpected string")
+	}
+}
+
 func replayCaseByName(t *testing.T, cases []ReplayCase, name string) ReplayCase {
 	t.Helper()
 	for _, replayCase := range cases {

session/replaytest/compare.go

@@ -10,6 +10,7 @@
 package replaytest
 
 import (
+	"bytes"
 	"encoding/json"
 	"fmt"
 	"math"
@@ -88,7 +89,9 @@ func snapshotValue(snapshot Snapshot) (any, error) {
 		return nil, err
 	}
 	var value any
-	if err := json.Unmarshal(encoded, &value); err != nil {
+	decoder := json.NewDecoder(bytes.NewReader(encoded))
+	decoder.UseNumber()
+	if err := decoder.Decode(&value); err != nil {
 		return nil, err
 	}
 	return value, nil
@@ -320,8 +323,8 @@ func scoreValuesEqual(path string, baseline, actual any, tolerance float64) bool
 	if itemPath == path || !isMemoryItemPath(itemPath) {
 		return false
 	}
-	baselineScore, baselineOK := baseline.(float64)
-	actualScore, actualOK := actual.(float64)
+	baselineScore, baselineOK := numericFloat64(baseline)
+	actualScore, actualOK := numericFloat64(actual)
 	return baselineOK && actualOK && math.Abs(baselineScore-actualScore) <= tolerance
 }
 
@@ -330,11 +333,23 @@ func durationValuesEqual(path string, baseline, actual any, tolerance time.Durat
 		!strings.Contains(path, "].events[") {
 		return false
 	}
-	baselineDuration, baselineOK := baseline.(float64)
-	actualDuration, actualOK := actual.(float64)
+	baselineDuration, baselineOK := numericFloat64(baseline)
+	actualDuration, actualOK := numericFloat64(actual)
 	return baselineOK && actualOK && math.Abs(baselineDuration-actualDuration) <= float64(tolerance)
 }
 
+func numericFloat64(value any) (float64, bool) {
+	switch typed := value.(type) {
+	case json.Number:
+		parsed, err := typed.Float64()
+		return parsed, err == nil
+	case float64:
+		return typed, true
+	default:
+		return 0, false
+	}
+}
+
 func isMemoryItemPath(path string) bool {
 	if isRootCollectionItem(path, "memories") {
 		return true

session/replaytest/compare_test.go

@@ -108,6 +108,26 @@ func TestCompareSnapshotsReportsMissingCollectionItems(t *testing.T) {
 	}
 }
 
+func TestCompareSnapshotsReportsAdditionalCollectionItems(t *testing.T) {
+	differences, err := CompareSnapshots(CompareInput{
+		Case:     "additional-memory",
+		Backend:  "sqlite",
+		Baseline: Snapshot{},
+		Actual: Snapshot{Memories: []MemorySnapshot{{
+			ID: "memory-1", AppName: "app", UserID: "user", Content: "content",
+		}},
+		},
+	})
+	if err != nil {
+		t.Fatalf("CompareSnapshots() error = %v", err)
+	}
+	differenceAt(t, differences, "$.memories.length")
+	additional := differenceAt(t, differences, "$.memories[0]")
+	if additional.Locator.MemoryID != "memory-1" || additional.Baseline != missingValue {
+		t.Fatalf("additional memory difference = %#v", additional)
+	}
+}
+
 func TestCompareSnapshotsRejectsInvalidAllowedDiffRules(t *testing.T) {
 	_, err := CompareSnapshots(CompareInput{
 		Case: "case", Backend: "sqlite",
@@ -158,6 +178,26 @@ func TestCompareSnapshotsReturnsEncodingErrors(t *testing.T) {
 	}
 }
 
+func TestCompareSnapshotsPreservesLargeIntegerPrecision(t *testing.T) {
+	baseline := Snapshot{Sessions: []SessionSnapshot{{State: map[string]StateValueSnapshot{
+		"large": JSONStateValue(int64(9007199254740992)),
+	}}}}
+	actual := Snapshot{Sessions: []SessionSnapshot{{State: map[string]StateValueSnapshot{
+		"large": JSONStateValue(int64(9007199254740993)),
+	}}}}
+
+	differences, err := CompareSnapshots(CompareInput{
+		Case: "large-integer", Backend: "sqlite", Baseline: baseline, Actual: actual,
+	})
+	if err != nil {
+		t.Fatalf("CompareSnapshots() error = %v", err)
+	}
+	if len(differences) == 0 {
+		t.Fatal("distinct integers above 2^53 compared equal")
+	}
+	differenceAt(t, differences, "$.sessions[0].state.large.value")
+}
+
 func TestCompareSnapshotsUsesAbsoluteScoreTolerance(t *testing.T) {
 	baseline := Snapshot{Memories: []MemorySnapshot{{Score: 0.5000009}}}
 	actual := Snapshot{Memories: []MemorySnapshot{{Score: 0.5000011}}}
@@ -234,6 +274,37 @@ func TestCompareSnapshotsDoesNotApplyScoreToleranceToState(t *testing.T) {
 	}
 }
 
+func TestRuleMatchesRequiresExactScope(t *testing.T) {
+	rule := AllowedDiffRule{Case: "case", Backend: "sqlite", Path: "$.sessions"}
+	if ruleMatches(rule, "other", "sqlite", "$.sessions") ||
+		ruleMatches(rule, "case", "other", "$.sessions") ||
+		ruleMatches(rule, "case", "sqlite", "$.sessions[0]") {
+		t.Fatal("exact rule matched outside its case, backend, or path")
+	}
+	if !ruleMatches(rule, "case", "sqlite", "$.sessions") {
+		t.Fatal("exact rule did not match its configured path")
+	}
+	rule.PathPrefix = true
+	if !ruleMatches(rule, "case", "sqlite", "$.sessions[0]") ||
+		!ruleMatches(rule, "case", "sqlite", "$.sessions.length") ||
+		ruleMatches(rule, "case", "sqlite", "$.session") {
+		t.Fatal("path-prefix rule matched an incorrect set of paths")
+	}
+}
+
+func TestComparisonNumericAndMemoryHelpers(t *testing.T) {
+	if got, ok := numericFloat64(float64(1.5)); !ok || got != 1.5 {
+		t.Fatalf("numericFloat64(float64) = %v, %v", got, ok)
+	}
+	if _, ok := numericFloat64("1.5"); ok {
+		t.Fatal("numericFloat64(string) unexpectedly succeeded")
+	}
+	appName, userID := memoryScope(map[string]any{"app_name": "app", "user_id": "user"})
+	if appName != "app" || userID != "user" {
+		t.Fatalf("memoryScope() = %q, %q", appName, userID)
+	}
+}
+
 func TestCompareSnapshotsLocatesMemoryScope(t *testing.T) {
 	baseline := comparisonFixture()
 	baseline.Memories[0].Scope = MemoryScope{AppName: "app", UserID: "user"}

session/replaytest/normalize_test.go

@@ -227,6 +227,64 @@ func TestNormalizeJSONLikeHandlesRawRepresentations(t *testing.T) {
 	}
 }
 
+func TestNormalizeJSONLikeHandlesFallbackRepresentations(t *testing.T) {
+	options := DefaultNormalizeOptions()
+	if got := normalizeJSONLike(nil, options); got != nil {
+		t.Fatalf("normalizeJSONLike(nil) = %#v", got)
+	}
+	for _, value := range []any{
+		json.RawMessage(`{"invalid"`),
+		[]byte(`{"invalid"`),
+	} {
+		if _, ok := normalizeJSONLike(value, options).(string); !ok {
+			t.Fatalf("invalid JSON representation was not preserved as text: %#v", value)
+		}
+	}
+	if _, ok := normalizeJSONLike(make(chan int), options).(string); !ok {
+		t.Fatal("unencodable value was not converted to diagnostic text")
+	}
+
+	scalars := []struct {
+		value any
+		want  any
+	}{
+		{value: uint64(42), want: uint64(42)},
+		{value: float32(1.5), want: float64(1.5)},
+	}
+	for _, scalar := range scalars {
+		if got := normalizeJSONLike(scalar.value, options); !reflect.DeepEqual(got, scalar.want) {
+			t.Fatalf("normalizeJSONLike(%T) = %#v, want %#v", scalar.value, got, scalar.want)
+		}
+	}
+}
+
+func TestNormalizeJSONNumbersPreservesInvalidNumbers(t *testing.T) {
+	tests := []json.Number{"1e+", "not-a-number"}
+	for _, value := range tests {
+		if got := normalizeJSONNumbers(value); got != value.String() {
+			t.Fatalf("normalizeJSONNumbers(%q) = %#v", value, got)
+		}
+	}
+}
+
+func TestCloneJSONLikeCopiesReferenceValues(t *testing.T) {
+	raw := json.RawMessage(`{"value":1}`)
+	bytesValue := []byte("bytes")
+	input := []any{raw, bytesValue, []any{map[string]any{"key": "value"}}}
+	cloned := cloneJSONLike(input).([]any)
+
+	cloned[0].(json.RawMessage)[0]++
+	cloned[1].([]byte)[0]++
+	cloned[2].([]any)[0].(map[string]any)["key"] = "changed"
+	if reflect.DeepEqual(input, cloned) {
+		t.Fatal("cloneJSONLike() retained aliases to reference values")
+	}
+	if string(raw) != `{"value":1}` || string(bytesValue) != "bytes" ||
+		input[2].([]any)[0].(map[string]any)["key"] != "value" {
+		t.Fatalf("cloneJSONLike() mutated input: %#v", input)
+	}
+}
+
 func TestNormalizeSnapshotFlagsInvalidSessionMetadataOrder(t *testing.T) {
 	created := time.Unix(20, 0)
 	snapshot := Snapshot{Sessions: []SessionSnapshot{{