fix(vt): enforce history snapshot invariants
- commit
- 4dede25fcc17e81ffd5f7a7f9e60017a1c895995
- author
- brice · · 2026-07-21 18:55 UTC
- parent
- 1fa07cc
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
5 files · +52 -3
M pkg/vt/history.go +10 -0
| @@ -108,6 +108,16 @@ func (h *History) sealTail() { | |||
| 108 | 108 | h.tail = nil | |
| 109 | 109 | } | |
| 110 | 110 | ||
| 111 | + | // normalizeTail seals complete chunks so the mutable tail remains shorter than | |
| 112 | + | // chunkRows. Restored snapshots may have been written with a different chunk | |
| 113 | + | // size than the current history configuration. | |
| 114 | + | func (h *History) normalizeTail() { | |
| 115 | + | for h.chunkRows > 0 && len(h.tail) >= h.chunkRows { | |
| 116 | + | h.chunks = append(h.chunks, &HistoryChunk{rows: h.tail[:h.chunkRows]}) | |
| 117 | + | h.tail = h.tail[h.chunkRows:] | |
| 118 | + | } | |
| 119 | + | } | |
| 120 | + | ||
| 111 | 121 | func (h *History) evict() { | |
| 112 | 122 | h.evictUntil(0, 0) | |
| 113 | 123 | } | |
| @@ -108,6 +108,16 @@ func (h *History) sealTail() { | |||||
| 108 | h.tail = nil | 108 | h.tail = nil | ||
| 109 | } | 109 | } | ||
| 110 | 110 | ||||
| 111 | + | // normalizeTail seals complete chunks so the mutable tail remains shorter than | |||
| 112 | + | // chunkRows. Restored snapshots may have been written with a different chunk | |||
| 113 | + | // size than the current history configuration. | |||
| 114 | + | func (h *History) normalizeTail() { | |||
| 115 | + | for h.chunkRows > 0 && len(h.tail) >= h.chunkRows { | |||
| 116 | + | h.chunks = append(h.chunks, &HistoryChunk{rows: h.tail[:h.chunkRows]}) | |||
| 117 | + | h.tail = h.tail[h.chunkRows:] | |||
| 118 | + | } | |||
| 119 | + | } | |||
| 120 | + | ||||
| 111 | func (h *History) evict() { | 121 | func (h *History) evict() { | ||
| 112 | h.evictUntil(0, 0) | 122 | h.evictUntil(0, 0) | ||
| 113 | } | 123 | } | ||
M pkg/vt/history_codec_test.go +3 -3
| @@ -137,9 +137,9 @@ func TestChunkCodecRejectsDimensionsBeyondSupportedLimits(t *testing.T) { | |||
| 137 | 137 | data: historyPayloadWithDimensions(supportedRows+1, 0), | |
| 138 | 138 | }, | |
| 139 | 139 | { | |
| 140 | - | name: "too many aggregate cells", | |
| 141 | - | view: historyViewWithDimensions(maxHistoryCells+1, 0), | |
| 142 | - | data: historyPayloadWithDimensions(maxHistoryRows+1, 0), | |
| 140 | + | name: "too many aggregate cells within row budget", | |
| 141 | + | view: historyViewWithDimensions(maxHistoryRows, maxHistoryRowCells+1), | |
| 142 | + | data: historyPayloadWithDimensions(maxHistoryRows, maxHistoryRowCells+1), | |
| 143 | 143 | }, | |
| 144 | 144 | } | |
| 145 | 145 | ||
| @@ -137,9 +137,9 @@ func TestChunkCodecRejectsDimensionsBeyondSupportedLimits(t *testing.T) { | |||||
| 137 | data: historyPayloadWithDimensions(supportedRows+1, 0), | 137 | data: historyPayloadWithDimensions(supportedRows+1, 0), | ||
| 138 | }, | 138 | }, | ||
| 139 | { | 139 | { | ||
| 140 | - | name: "too many aggregate cells", | 140 | + | name: "too many aggregate cells within row budget", |
| 141 | - | view: historyViewWithDimensions(maxHistoryCells+1, 0), | 141 | + | view: historyViewWithDimensions(maxHistoryRows, maxHistoryRowCells+1), |
| 142 | - | data: historyPayloadWithDimensions(maxHistoryRows+1, 0), | 142 | + | data: historyPayloadWithDimensions(maxHistoryRows, maxHistoryRowCells+1), |
| 143 | }, | 143 | }, | ||
| 144 | } | 144 | } | ||
| 145 | 145 | ||||
M pkg/vt/history_test.go +21 -0
| @@ -217,6 +217,27 @@ func TestHistoryBoundsRowsAndCellsWithExactRowEviction(t *testing.T) { | |||
| 217 | 217 | } | |
| 218 | 218 | } | |
| 219 | 219 | ||
| 220 | + | func TestHistoryAppendIsNoOpForNilAndZeroValue(t *testing.T) { | |
| 221 | + | tests := []struct { | |
| 222 | + | name string | |
| 223 | + | history *History | |
| 224 | + | }{ | |
| 225 | + | {name: "nil history"}, | |
| 226 | + | {name: "zero-value history", history: &History{}}, | |
| 227 | + | } | |
| 228 | + | ||
| 229 | + | for _, tt := range tests { | |
| 230 | + | t.Run(tt.name, func(t *testing.T) { | |
| 231 | + | if err := tt.history.Append(historyRow("row")); err != nil { | |
| 232 | + | t.Fatalf("append error = %v, want nil", err) | |
| 233 | + | } | |
| 234 | + | if got := tt.history.Len(); got != 0 { | |
| 235 | + | t.Fatalf("retained rows = %d, want 0", got) | |
| 236 | + | } | |
| 237 | + | }) | |
| 238 | + | } | |
| 239 | + | } | |
| 240 | + | ||
| 220 | 241 | func TestHistoryRejectsRowWiderThanCellBudgetWithoutMutation(t *testing.T) { | |
| 221 | 242 | history := NewHistory(HistoryConfig{MaxRows: 2, MaxCells: 2, ChunkRows: 2}) | |
| 222 | 243 | if err := history.Append(historyRow("ok")); err != nil { | |
| @@ -217,6 +217,27 @@ func TestHistoryBoundsRowsAndCellsWithExactRowEviction(t *testing.T) { | |||||
| 217 | } | 217 | } | ||
| 218 | } | 218 | } | ||
| 219 | 219 | ||||
| 220 | + | func TestHistoryAppendIsNoOpForNilAndZeroValue(t *testing.T) { | |||
| 221 | + | tests := []struct { | |||
| 222 | + | name string | |||
| 223 | + | history *History | |||
| 224 | + | }{ | |||
| 225 | + | {name: "nil history"}, | |||
| 226 | + | {name: "zero-value history", history: &History{}}, | |||
| 227 | + | } | |||
| 228 | + | ||||
| 229 | + | for _, tt := range tests { | |||
| 230 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 231 | + | if err := tt.history.Append(historyRow("row")); err != nil { | |||
| 232 | + | t.Fatalf("append error = %v, want nil", err) | |||
| 233 | + | } | |||
| 234 | + | if got := tt.history.Len(); got != 0 { | |||
| 235 | + | t.Fatalf("retained rows = %d, want 0", got) | |||
| 236 | + | } | |||
| 237 | + | }) | |||
| 238 | + | } | |||
| 239 | + | } | |||
| 240 | + | ||||
| 220 | func TestHistoryRejectsRowWiderThanCellBudgetWithoutMutation(t *testing.T) { | 241 | func TestHistoryRejectsRowWiderThanCellBudgetWithoutMutation(t *testing.T) { | ||
| 221 | history := NewHistory(HistoryConfig{MaxRows: 2, MaxCells: 2, ChunkRows: 2}) | 242 | history := NewHistory(HistoryConfig{MaxRows: 2, MaxCells: 2, ChunkRows: 2}) | ||
| 222 | if err := history.Append(historyRow("ok")); err != nil { | 243 | if err := history.Append(historyRow("ok")); err != nil { | ||
M pkg/vt/snapshot.go +1 -0
| @@ -106,6 +106,7 @@ func HistoryFromBlobs(config HistoryConfig, sealed [][]byte, tail []byte) (*Hist | |||
| 106 | 106 | h.cells += view.Cells() | |
| 107 | 107 | } | |
| 108 | 108 | h.evict() | |
| 109 | + | h.normalizeTail() | |
| 109 | 110 | return h, nil | |
| 110 | 111 | } | |
| 111 | 112 | ||
| @@ -106,6 +106,7 @@ func HistoryFromBlobs(config HistoryConfig, sealed [][]byte, tail []byte) (*Hist | |||||
| 106 | h.cells += view.Cells() | 106 | h.cells += view.Cells() | ||
| 107 | } | 107 | } | ||
| 108 | h.evict() | 108 | h.evict() | ||
| 109 | + | h.normalizeTail() | |||
| 109 | return h, nil | 110 | return h, nil | ||
| 110 | } | 111 | } | ||
| 111 | 112 | ||||
M pkg/vt/snapshot_test.go +17 -0
| @@ -4,6 +4,7 @@ import ( | |||
| 4 | 4 | "strings" | |
| 5 | 5 | "testing" | |
| 6 | 6 | ||
| 7 | + | "github.com/bnema/vev/pkg/renderer" | |
| 7 | 8 | "github.com/stretchr/testify/require" | |
| 8 | 9 | ) | |
| 9 | 10 | ||
| @@ -48,6 +49,22 @@ func TestHistorySnapshotViewDoesNotSealTailAndCopiesIt(t *testing.T) { | |||
| 48 | 49 | require.Equal(t, []string{"CCCC"}, historyViewTexts(decoded)) | |
| 49 | 50 | } | |
| 50 | 51 | ||
| 52 | + | func TestHistoryFromBlobsNormalizesFullTailBeforeAppend(t *testing.T) { | |
| 53 | + | fullTail, err := MarshalHistory(HistoryView{ | |
| 54 | + | chunks: []*HistoryChunk{{rows: [][]renderer.Cell{historyRow("AAAA"), historyRow("BBBB")}}}, | |
| 55 | + | rows: 2, | |
| 56 | + | }) | |
| 57 | + | require.NoError(t, err) | |
| 58 | + | ||
| 59 | + | restored, err := HistoryFromBlobs(HistoryConfig{MaxRows: 4, ChunkRows: 2}, nil, fullTail) | |
| 60 | + | require.NoError(t, err) | |
| 61 | + | require.NoError(t, restored.Append(historyRow("CCCC"))) | |
| 62 | + | ||
| 63 | + | require.Equal(t, []string{"AAAA", "BBBB", "CCCC"}, historyViewTexts(restored.View())) | |
| 64 | + | require.Len(t, restored.chunks, 1, "a full restored tail must become sealed") | |
| 65 | + | require.Len(t, restored.tail, 1, "the appended row must be the mutable tail") | |
| 66 | + | } | |
| 67 | + | ||
| 51 | 68 | func TestHistoryFromBlobsRestoresCellAccountingAndEvictsToBothBudgets(t *testing.T) { | |
| 52 | 69 | history := NewHistory(HistoryConfig{MaxRows: 4, MaxCells: 10, ChunkRows: 2}) | |
| 53 | 70 | for _, text := range []string{"a", "b", "cc"} { | |
| @@ -4,6 +4,7 @@ import ( | |||||
| 4 | "strings" | 4 | "strings" | ||
| 5 | "testing" | 5 | "testing" | ||
| 6 | 6 | ||||
| 7 | + | "github.com/bnema/vev/pkg/renderer" | |||
| 7 | "github.com/stretchr/testify/require" | 8 | "github.com/stretchr/testify/require" | ||
| 8 | ) | 9 | ) | ||
| 9 | 10 | ||||
| @@ -48,6 +49,22 @@ func TestHistorySnapshotViewDoesNotSealTailAndCopiesIt(t *testing.T) { | |||||
| 48 | require.Equal(t, []string{"CCCC"}, historyViewTexts(decoded)) | 49 | require.Equal(t, []string{"CCCC"}, historyViewTexts(decoded)) | ||
| 49 | } | 50 | } | ||
| 50 | 51 | ||||
| 52 | + | func TestHistoryFromBlobsNormalizesFullTailBeforeAppend(t *testing.T) { | |||
| 53 | + | fullTail, err := MarshalHistory(HistoryView{ | |||
| 54 | + | chunks: []*HistoryChunk{{rows: [][]renderer.Cell{historyRow("AAAA"), historyRow("BBBB")}}}, | |||
| 55 | + | rows: 2, | |||
| 56 | + | }) | |||
| 57 | + | require.NoError(t, err) | |||
| 58 | + | ||||
| 59 | + | restored, err := HistoryFromBlobs(HistoryConfig{MaxRows: 4, ChunkRows: 2}, nil, fullTail) | |||
| 60 | + | require.NoError(t, err) | |||
| 61 | + | require.NoError(t, restored.Append(historyRow("CCCC"))) | |||
| 62 | + | ||||
| 63 | + | require.Equal(t, []string{"AAAA", "BBBB", "CCCC"}, historyViewTexts(restored.View())) | |||
| 64 | + | require.Len(t, restored.chunks, 1, "a full restored tail must become sealed") | |||
| 65 | + | require.Len(t, restored.tail, 1, "the appended row must be the mutable tail") | |||
| 66 | + | } | |||
| 67 | + | ||||
| 51 | func TestHistoryFromBlobsRestoresCellAccountingAndEvictsToBothBudgets(t *testing.T) { | 68 | func TestHistoryFromBlobsRestoresCellAccountingAndEvictsToBothBudgets(t *testing.T) { | ||
| 52 | history := NewHistory(HistoryConfig{MaxRows: 4, MaxCells: 10, ChunkRows: 2}) | 69 | history := NewHistory(HistoryConfig{MaxRows: 4, MaxCells: 10, ChunkRows: 2}) | ||
| 53 | for _, text := range []string{"a", "b", "cc"} { | 70 | for _, text := range []string{"a", "b", "cc"} { | ||