keyviz: deep-copy snapshot rows + slot bounds

Codex round-2 P2: Snapshot consumers were aliasing live sampler state.
snapshotMeta returned Start/End by reference (so flushed rows tracked
later RegisterRoute mutations) and ringBuffer.Range only copied
MatrixColumn structs (so concurrent Flush could rotate the underlying
Rows slice, and caller mutation could corrupt stored history).

Clone Start/End in snapshotMeta and add a cloneColumn helper that
ringBuffer.Range uses to materialise a fully-owned column per call.
Regression test TestSnapshotReturnsDeepCopy mutates a returned rows
bounds and verifies a follow-up snapshot is unaffected.

Author: bootjp

keyviz/ring_buffer.go

@@ -42,8 +42,10 @@ func (r *ringBuffer) Push(col MatrixColumn) {
 
 // Range returns the columns whose At falls in [from, to), oldest
 // first. Either bound may be the zero Time, meaning unbounded on that
-// side. The returned slice is freshly allocated; callers may mutate
-// it freely.
+// side. The returned slice is a deep copy: each column has its own
+// Rows slice and each row owns its Start/End byte slices, so callers
+// may mutate any field without corrupting stored history or racing
+// with concurrent flushes.
 func (r *ringBuffer) Range(from, to time.Time) []MatrixColumn {
 	all := r.snapshotOrdered()
 	// snapshotOrdered is already chronologically ordered.
@@ -63,10 +65,27 @@ func (r *ringBuffer) Range(from, to time.Time) []MatrixColumn {
 		return nil
 	}
 	out := make([]MatrixColumn, hi-lo)
-	copy(out, all[lo:hi])
+	for i, src := range all[lo:hi] {
+		out[i] = cloneColumn(src)
+	}
 	return out
 }
 
+// cloneColumn returns a deep copy of col: a fresh Rows slice with
+// each row's Start/End and MemberRoutes independently allocated.
+func cloneColumn(col MatrixColumn) MatrixColumn {
+	rows := make([]MatrixRow, len(col.Rows))
+	for i, row := range col.Rows {
+		rows[i] = row
+		rows[i].Start = cloneBytes(row.Start)
+		rows[i].End = cloneBytes(row.End)
+		if len(row.MemberRoutes) > 0 {
+			rows[i].MemberRoutes = append([]uint64(nil), row.MemberRoutes...)
+		}
+	}
+	return MatrixColumn{At: col.At, Rows: rows}
+}
+
 // snapshotOrdered returns a chronologically ordered (oldest first)
 // view of the buffer contents in a fresh slice.
 func (r *ringBuffer) snapshotOrdered() []MatrixColumn {

keyviz/sampler.go

@@ -161,13 +161,14 @@ type routeSlot struct {
 
 // snapshotMeta returns a defensive copy of the slot's metadata under
 // the read lock. Used by Flush so the row it emits doesn't share
-// MemberRoutes with the live slot (which a later RegisterRoute may
-// extend).
+// Start/End/MemberRoutes with the live slot (which a later
+// RegisterRoute may extend, and which the snapshot API exports to
+// external consumers that may mutate the bounds).
 func (s *routeSlot) snapshotMeta() (start, end []byte, aggregate bool, members []uint64) {
 	s.metaMu.RLock()
 	defer s.metaMu.RUnlock()
-	start = s.Start
-	end = s.End
+	start = cloneBytes(s.Start)
+	end = cloneBytes(s.End)
 	aggregate = s.Aggregate
 	if len(s.MemberRoutes) > 0 {
 		members = append([]uint64(nil), s.MemberRoutes...)

keyviz/sampler_test.go

@@ -454,3 +454,34 @@ func columnTimes(cols []MatrixColumn) []time.Time {
 	}
 	return out
 }
+
+// TestSnapshotReturnsDeepCopy guards the public-API contract that the
+// Snapshot result is fully owned by the caller: mutating row bounds or
+// member-route slices must not corrupt later snapshots, and must not
+// race with concurrent Flush/RegisterRoute.
+func TestSnapshotReturnsDeepCopy(t *testing.T) {
+	t.Parallel()
+	s, _ := newTestSampler(t, MemSamplerOptions{Step: time.Second, HistoryColumns: 4})
+	if !s.RegisterRoute(1, []byte("aaaa"), []byte("bbbb")) {
+		t.Fatal("RegisterRoute(1) returned false")
+	}
+	s.Observe(1, OpRead, 1, 2)
+	s.Flush()
+
+	first := s.Snapshot(time.Time{}, time.Time{})
+	if len(first) == 0 || len(first[0].Rows) == 0 {
+		t.Fatalf("expected at least one row in first snapshot, got %+v", first)
+	}
+	first[0].Rows[0].Start[0] = 'X'
+	first[0].Rows[0].End[0] = 'Y'
+	first[0].Rows = nil
+
+	second := s.Snapshot(time.Time{}, time.Time{})
+	if len(second) == 0 || len(second[0].Rows) == 0 {
+		t.Fatalf("second snapshot lost rows after caller mutation: %+v", second)
+	}
+	r := second[0].Rows[0]
+	if string(r.Start) != "aaaa" || string(r.End) != "bbbb" {
+		t.Fatalf("snapshot bounds aliased live state: start=%q end=%q", r.Start, r.End)
+	}
+}