fix(rowid): keep history allocation monotonic
- commit
- 14c14275d2e46bc9a2d26ada210bcbaa0cfbe8de
- author
- brice · · 2026-08-02 22:10 UTC
- committer
- brice · · 2026-08-03 17:54 UTC
- parent
- 6c7d5d9
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
4 files · +48 -8
M internal/usecase/daemon/copymode_test.go +1 -0
| @@ -113,6 +113,7 @@ func TestCopyModeDocumentCarriesPaneRowIDs(t *testing.T) { | |||
| 113 | 113 | pane.screen.Write([]byte("live")) | |
| 114 | 114 | historyID := pane.history.View().RowID(0) | |
| 115 | 115 | liveID := pane.screen.RowID(0) | |
| 116 | + | require.NotEqual(t, historyID, liveID) | |
| 116 | 117 | ||
| 117 | 118 | d.enterCopyMode(sess, ac) | |
| 118 | 119 | awaitFrame(t, sends, ports.MsgOutput) | |
| @@ -113,6 +113,7 @@ func TestCopyModeDocumentCarriesPaneRowIDs(t *testing.T) { | |||||
| 113 | pane.screen.Write([]byte("live")) | 113 | pane.screen.Write([]byte("live")) | ||
| 114 | historyID := pane.history.View().RowID(0) | 114 | historyID := pane.history.View().RowID(0) | ||
| 115 | liveID := pane.screen.RowID(0) | 115 | liveID := pane.screen.RowID(0) | ||
| 116 | + | require.NotEqual(t, historyID, liveID) | |||
| 116 | 117 | ||||
| 117 | d.enterCopyMode(sess, ac) | 118 | d.enterCopyMode(sess, ac) | ||
| 118 | awaitFrame(t, sends, ports.MsgOutput) | 119 | awaitFrame(t, sends, ports.MsgOutput) | ||
M internal/usecase/daemon/daemon_test.go +2 -1
| @@ -458,7 +458,8 @@ func testRow(text string) []renderer.Cell { | |||
| 458 | 458 | // wrapped out of. Tests that care about wrapped extents build the bound instead. | |
| 459 | 459 | func appendHistoryRow(tb testing.TB, history *vt.History, row []renderer.Cell) { | |
| 460 | 460 | tb.Helper() | |
| 461 | - | require.NoError(tb, history.Append(row, vt.LineBound{End: len(row)})) | |
| 461 | + | id := max(vt.RowID(1_000_000), history.NextRowID()) | |
| 462 | + | require.NoError(tb, history.AppendWithID(row, vt.LineBound{End: len(row)}, id)) | |
| 462 | 463 | } | |
| 463 | 464 | ||
| 464 | 465 | func TestAppendHistoryRowAllocatesUniqueIDsAfterEviction(t *testing.T) { | |
| @@ -458,7 +458,8 @@ func testRow(text string) []renderer.Cell { | |||||
| 458 | // wrapped out of. Tests that care about wrapped extents build the bound instead. | 458 | // wrapped out of. Tests that care about wrapped extents build the bound instead. | ||
| 459 | func appendHistoryRow(tb testing.TB, history *vt.History, row []renderer.Cell) { | 459 | func appendHistoryRow(tb testing.TB, history *vt.History, row []renderer.Cell) { | ||
| 460 | tb.Helper() | 460 | tb.Helper() | ||
| 461 | - | require.NoError(tb, history.Append(row, vt.LineBound{End: len(row)})) | 461 | + | id := max(vt.RowID(1_000_000), history.NextRowID()) |
| 462 | + | require.NoError(tb, history.AppendWithID(row, vt.LineBound{End: len(row)}, id)) | |||
| 462 | } | 463 | } | ||
| 463 | 464 | ||||
| 464 | func TestAppendHistoryRowAllocatesUniqueIDsAfterEviction(t *testing.T) { | 465 | func TestAppendHistoryRowAllocatesUniqueIDsAfterEviction(t *testing.T) { | ||
M pkg/vt/history.go +3 -7
| @@ -155,20 +155,16 @@ func (h *History) appendRow(row []renderer.Cell, bound LineBound, id RowID) erro | |||
| 155 | 155 | return nil | |
| 156 | 156 | } | |
| 157 | 157 | ||
| 158 | + | // allocateRowID relies on appendRow advancing nextRowID above every accepted | |
| 159 | + | // explicit ID; automatic IDs therefore never need to scan retained rows. | |
| 158 | 160 | func (h *History) allocateRowID() (RowID, error) { | |
| 159 | 161 | if h.nextRowID == 0 { | |
| 160 | 162 | h.nextRowID = 1 | |
| 161 | 163 | } | |
| 162 | - | for h.hasRowID(h.nextRowID) { | |
| 163 | - | if h.nextRowID >= ^RowID(0)-1 { | |
| 164 | - | return 0, errInvalidHistoryRowID | |
| 165 | - | } | |
| 166 | - | h.nextRowID++ | |
| 167 | - | } | |
| 168 | - | id := h.nextRowID | |
| 169 | 164 | if h.nextRowID >= ^RowID(0)-1 { | |
| 170 | 165 | return 0, errInvalidHistoryRowID | |
| 171 | 166 | } | |
| 167 | + | id := h.nextRowID | |
| 172 | 168 | h.nextRowID++ | |
| 173 | 169 | return id, nil | |
| 174 | 170 | } | |
| @@ -155,20 +155,16 @@ func (h *History) appendRow(row []renderer.Cell, bound LineBound, id RowID) erro | |||||
| 155 | return nil | 155 | return nil | ||
| 156 | } | 156 | } | ||
| 157 | 157 | ||||
| 158 | + | // allocateRowID relies on appendRow advancing nextRowID above every accepted | |||
| 159 | + | // explicit ID; automatic IDs therefore never need to scan retained rows. | |||
| 158 | func (h *History) allocateRowID() (RowID, error) { | 160 | func (h *History) allocateRowID() (RowID, error) { | ||
| 159 | if h.nextRowID == 0 { | 161 | if h.nextRowID == 0 { | ||
| 160 | h.nextRowID = 1 | 162 | h.nextRowID = 1 | ||
| 161 | } | 163 | } | ||
| 162 | - | for h.hasRowID(h.nextRowID) { | |||
| 163 | - | if h.nextRowID >= ^RowID(0)-1 { | |||
| 164 | - | return 0, errInvalidHistoryRowID | |||
| 165 | - | } | |||
| 166 | - | h.nextRowID++ | |||
| 167 | - | } | |||
| 168 | - | id := h.nextRowID | |||
| 169 | if h.nextRowID >= ^RowID(0)-1 { | 164 | if h.nextRowID >= ^RowID(0)-1 { | ||
| 170 | return 0, errInvalidHistoryRowID | 165 | return 0, errInvalidHistoryRowID | ||
| 171 | } | 166 | } | ||
| 167 | + | id := h.nextRowID | |||
| 172 | h.nextRowID++ | 168 | h.nextRowID++ | ||
| 173 | return id, nil | 169 | return id, nil | ||
| 174 | } | 170 | } | ||
M pkg/vt/row_id_persistence_test.go +42 -0
| @@ -68,6 +68,48 @@ func TestHistoryAppendWithIDRejectsMalformedIDs(t *testing.T) { | |||
| 68 | 68 | } | |
| 69 | 69 | } | |
| 70 | 70 | ||
| 71 | + | func TestHistoryAppendWithIDValidatesOutOfOrderDuplicates(t *testing.T) { | |
| 72 | + | history := NewHistory(HistoryConfig{MaxRows: 4, ChunkRows: 1}) | |
| 73 | + | require.NoError(t, history.AppendWithID(historyRow("high"), LineBound{End: 4}, RowID(100))) | |
| 74 | + | require.NoError(t, history.AppendWithID(historyRow("low"), LineBound{End: 3}, RowID(7))) | |
| 75 | + | require.Equal(t, RowID(101), history.NextRowID()) | |
| 76 | + | ||
| 77 | + | before := history.View() | |
| 78 | + | require.Error(t, history.AppendWithID(historyRow("duplicate"), LineBound{End: 9}, RowID(7))) | |
| 79 | + | require.Equal(t, before.Len(), history.Len()) | |
| 80 | + | require.Equal(t, before.NextRowID(), history.NextRowID()) | |
| 81 | + | } | |
| 82 | + | ||
| 83 | + | func TestHistoryAutomaticAllocationStaysAboveAcceptedIDsAfterEviction(t *testing.T) { | |
| 84 | + | history := NewHistory(HistoryConfig{MaxRows: 2, ChunkRows: 1}) | |
| 85 | + | require.NoError(t, history.AppendWithID(historyRow("high"), LineBound{End: 4}, RowID(100))) | |
| 86 | + | require.NoError(t, history.AppendWithID(historyRow("low"), LineBound{End: 3}, RowID(7))) | |
| 87 | + | require.NoError(t, history.AppendWithID(historyRow("next-low"), LineBound{End: 8}, RowID(8))) | |
| 88 | + | require.NoError(t, history.AppendWithID(historyRow("evict"), LineBound{End: 5}, RowID(9))) | |
| 89 | + | require.Equal(t, RowID(101), history.NextRowID()) | |
| 90 | + | ||
| 91 | + | require.NoError(t, history.Append(historyRow("automatic"), LineBound{End: 9})) | |
| 92 | + | view := history.View() | |
| 93 | + | require.Equal(t, RowID(101), view.RowID(view.Len()-1)) | |
| 94 | + | require.Equal(t, RowID(102), history.NextRowID()) | |
| 95 | + | for i := range view.Len() { | |
| 96 | + | require.Less(t, view.RowID(i), history.NextRowID()) | |
| 97 | + | } | |
| 98 | + | } | |
| 99 | + | ||
| 100 | + | func TestHistoryAutomaticAllocationBoundary(t *testing.T) { | |
| 101 | + | lastPersistableID := ^RowID(0) - 2 | |
| 102 | + | history := NewHistory(HistoryConfig{MaxRows: 2, ChunkRows: 1}) | |
| 103 | + | history.nextRowID = lastPersistableID | |
| 104 | + | ||
| 105 | + | require.NoError(t, history.Append(historyRow("last"), LineBound{End: 4})) | |
| 106 | + | require.Equal(t, lastPersistableID+1, history.NextRowID()) | |
| 107 | + | before := history.View() | |
| 108 | + | require.Error(t, history.Append(historyRow("exhausted"), LineBound{End: 9})) | |
| 109 | + | require.Equal(t, before.Len(), history.Len()) | |
| 110 | + | require.Equal(t, before.NextRowID(), history.NextRowID()) | |
| 111 | + | } | |
| 112 | + | ||
| 71 | 113 | func TestScreenRowIDBoundaryMatchesHistoryPersistence(t *testing.T) { | |
| 72 | 114 | const maxPersistedRowID = ^RowID(0) - 2 | |
| 73 | 115 | for _, test := range []struct { | |
| @@ -68,6 +68,48 @@ func TestHistoryAppendWithIDRejectsMalformedIDs(t *testing.T) { | |||||
| 68 | } | 68 | } | ||
| 69 | } | 69 | } | ||
| 70 | 70 | ||||
| 71 | + | func TestHistoryAppendWithIDValidatesOutOfOrderDuplicates(t *testing.T) { | |||
| 72 | + | history := NewHistory(HistoryConfig{MaxRows: 4, ChunkRows: 1}) | |||
| 73 | + | require.NoError(t, history.AppendWithID(historyRow("high"), LineBound{End: 4}, RowID(100))) | |||
| 74 | + | require.NoError(t, history.AppendWithID(historyRow("low"), LineBound{End: 3}, RowID(7))) | |||
| 75 | + | require.Equal(t, RowID(101), history.NextRowID()) | |||
| 76 | + | ||||
| 77 | + | before := history.View() | |||
| 78 | + | require.Error(t, history.AppendWithID(historyRow("duplicate"), LineBound{End: 9}, RowID(7))) | |||
| 79 | + | require.Equal(t, before.Len(), history.Len()) | |||
| 80 | + | require.Equal(t, before.NextRowID(), history.NextRowID()) | |||
| 81 | + | } | |||
| 82 | + | ||||
| 83 | + | func TestHistoryAutomaticAllocationStaysAboveAcceptedIDsAfterEviction(t *testing.T) { | |||
| 84 | + | history := NewHistory(HistoryConfig{MaxRows: 2, ChunkRows: 1}) | |||
| 85 | + | require.NoError(t, history.AppendWithID(historyRow("high"), LineBound{End: 4}, RowID(100))) | |||
| 86 | + | require.NoError(t, history.AppendWithID(historyRow("low"), LineBound{End: 3}, RowID(7))) | |||
| 87 | + | require.NoError(t, history.AppendWithID(historyRow("next-low"), LineBound{End: 8}, RowID(8))) | |||
| 88 | + | require.NoError(t, history.AppendWithID(historyRow("evict"), LineBound{End: 5}, RowID(9))) | |||
| 89 | + | require.Equal(t, RowID(101), history.NextRowID()) | |||
| 90 | + | ||||
| 91 | + | require.NoError(t, history.Append(historyRow("automatic"), LineBound{End: 9})) | |||
| 92 | + | view := history.View() | |||
| 93 | + | require.Equal(t, RowID(101), view.RowID(view.Len()-1)) | |||
| 94 | + | require.Equal(t, RowID(102), history.NextRowID()) | |||
| 95 | + | for i := range view.Len() { | |||
| 96 | + | require.Less(t, view.RowID(i), history.NextRowID()) | |||
| 97 | + | } | |||
| 98 | + | } | |||
| 99 | + | ||||
| 100 | + | func TestHistoryAutomaticAllocationBoundary(t *testing.T) { | |||
| 101 | + | lastPersistableID := ^RowID(0) - 2 | |||
| 102 | + | history := NewHistory(HistoryConfig{MaxRows: 2, ChunkRows: 1}) | |||
| 103 | + | history.nextRowID = lastPersistableID | |||
| 104 | + | ||||
| 105 | + | require.NoError(t, history.Append(historyRow("last"), LineBound{End: 4})) | |||
| 106 | + | require.Equal(t, lastPersistableID+1, history.NextRowID()) | |||
| 107 | + | before := history.View() | |||
| 108 | + | require.Error(t, history.Append(historyRow("exhausted"), LineBound{End: 9})) | |||
| 109 | + | require.Equal(t, before.Len(), history.Len()) | |||
| 110 | + | require.Equal(t, before.NextRowID(), history.NextRowID()) | |||
| 111 | + | } | |||
| 112 | + | ||||
| 71 | func TestScreenRowIDBoundaryMatchesHistoryPersistence(t *testing.T) { | 113 | func TestScreenRowIDBoundaryMatchesHistoryPersistence(t *testing.T) { | ||
| 72 | const maxPersistedRowID = ^RowID(0) - 2 | 114 | const maxPersistedRowID = ^RowID(0) - 2 | ||
| 73 | for _, test := range []struct { | 115 | for _, test := range []struct { | ||