fix: use the correct index max_local threshold, not table's

.openspec/specs/006-btree/spec.md

@@ -175,19 +175,19 @@ The system SHOULD provide enough key ordering to walk an index b-tree correctly
 
 **Tests:** `src/btree/index.rs::secondary_index_seek_matches_oracle`
 
-### Requirement 7: No Fixture Yet Covers an Overflowing Index Key [SHOULD]
+### Requirement 7: Overflowing Index Keys Use the Index (Not Table) Local-Size Threshold [MUST]
 
-The originating issue's corpus section expects an index fixture with an overflowing key; `tests/corpus/fixtures/btrees/index.db`'s actual generated content (`b TEXT`, max length 15 bytes) does not exercise this — overflow reassembly on index cells reuses the same `reassemble_payload` function already proven byte-identical against table-cell overflow (Requirement 2), so the residual risk is low, but this is a real coverage gap, not a silent oversight.
+The originating issue's corpus section expected an index fixture with an overflowing key; none existed (`tests/corpus/fixtures/btrees/index.db`'s content — `b TEXT`, max length 15 bytes — never exercised it), flagged as a real coverage gap rather than a silent oversight. Building `overflow_index_key.db` (an ~8000-byte indexed TEXT key against a 4096-byte page) surfaced that the gap was hiding an actual bug, not just missing coverage: `local_payload_size`'s `max_local` was computed as `usable_size - 35` unconditionally, but SQLite defines a smaller `max_local` for index cells (leaf AND interior) — `(usable_size - 12) * 64 / 255 - 23` — than for table leaf cells. Every index cell whose payload fell between the two thresholds was read with a `local_size` far larger than what SQLite actually reserved on the page, corrupting the read (`PayloadTooShort`) the moment a fixture forced a payload past the *correct* (smaller) index threshold while still under the incorrect (larger) table one. The system MUST select `max_local` by cell kind, not just by whether the payload overflows.
 
-**Implementation:** `tools/gen_fixtures.sh` (planned — no fixture with an overflowing index key exists yet)
+**Implementation:** `src/btree.rs::local_payload_size` (takes an `is_index` flag); `tools/gen_fixtures.sh` (`overflow_index_key.db`)
 
 #### Scenario: Overflowing index key
 
-- GIVEN an index whose key column is large enough to overflow into one or more overflow pages
+- GIVEN an index whose key column is large enough to overflow into one or more overflow pages under the index (not table) local-size threshold
 - WHEN the cursor reads that entry
 - THEN the reassembled key payload MUST be byte-identical to the pinned oracle's value
 
-**Tests:** `tools/gen_fixtures.sh` (planned)
+**Tests:** `tests/corpus/btree_test.rs::overflowing_index_key_reassembles_byte_identical_to_oracle`
 
 ### Requirement 8: Leaf Cell Insert Without Split [MUST]
 
@@ -419,6 +419,14 @@ The system MUST delete the entry with a given key from an index b-tree. Deleting
 
 **Tests:** `tests/corpus/btree_index_insert_delete_test.rs::delete_all_entries_leaves_an_empty_index`
 
+#### Scenario: Deleting an entry with an overflowing value frees its overflow chain
+
+- GIVEN an index entry whose key/value overflows into one or more overflow pages (per Requirement 7's corrected threshold)
+- WHEN that entry is deleted, whether directly from a leaf or removed outright by the interior-match path (Requirement 17)
+- THEN every page in its overflow chain MUST be returned to the freelist, not leaked — found via Requirement 7's fixture work, since index cells overflow far more readily under the corrected (smaller) threshold than the previous bug allowed
+
+**Tests:** `src/btree/index/delete.rs::tests::deleting_an_entry_with_overflow_frees_its_overflow_chain`, `src/btree/index/delete.rs::tests::deleting_all_entries_orphans_no_page`
+
 ### Requirement 17: Interior-Match Deletion via Predecessor Swap [MUST]
 
 Because index interior cells carry a full entry (Requirement 5), deleting a key that was promoted to interior level by an earlier split MUST NOT simply remove that routing entry — its child pointer is load-bearing, and removing the entry would also discard whichever value it carries. The system MUST instead find that entry's in-order predecessor (the maximum entry within its own left-child subtree, found by recursively descending — preferring the rightmost subtree, falling back to an interior page's own last entry once its rightmost subtree is confirmed drained) and swap the predecessor's value into the matched entry's position, physically removing the predecessor from wherever it actually lived. If the matched entry's subtree is entirely drained (no predecessor available), the entry is removed outright instead.

CHANGELOG.md

@@ -13,6 +13,17 @@ All notable changes to sqlite-rs. Format follows [Keep a Changelog](https://keep
   pages were freed. Table leaf, index leaf, and index interior cells now
   have their first overflow page located and their whole chain walked
   and deallocated before the tree structure itself is freed.
+- Index cells (leaf and interior) were reading their local-payload size
+  with the table leaf cell's `max_local` formula (`usable_size - 35`)
+  instead of the smaller one SQLite defines for index cells
+  (`(usable_size - 12) * 64 / 255 - 23`), corrupting reads on any index
+  cell whose payload landed between the two thresholds. Found while
+  closing 006-btree Req 7's documented "no overflowing-index-key
+  fixture" coverage gap — adding one immediately hit `PayloadTooShort`.
+  Fixing the threshold also surfaced that index entry delete never freed
+  a removed entry's overflow chain at all (table delete already did);
+  index entries essentially never overflowed under the old, too-generous
+  threshold, so the gap was never exercised.
 
 ### Docs
 

src/btree.rs

@@ -279,6 +279,7 @@ impl<P: PageSource> TableCursor<P> {
             &cur.page,
             cur.tail_start,
             cur.payload_len,
+            false,
         )
     }
 
@@ -606,14 +607,24 @@ impl<P: PageSource> TableCursor<P> {
 }
 
 /// SQLite's overflow local-size formula (fileformat2.html "Cell Payload
-/// Overflow"), shared by table leaf cells, index leaf cells, and index
-/// interior cells (table interior cells have no payload at all). All
-/// arithmetic saturates rather than panics — a pathological `usable_size`
-/// degrades to a safe (wrong but non-panicking) answer, caught by the
-/// length checks around the call site instead of an arithmetic panic
-/// here.
-fn local_payload_size(usable_size: u32, payload_len: u64) -> u64 {
-    let max_local = usable_size.saturating_sub(35) as u64;
+/// Overflow"). `min_local` is shared by every cell kind, but `max_local`
+/// is NOT: table leaf cells use `usable_size - 35`, while index cells
+/// (leaf AND interior — table interior cells have no payload at all) use
+/// `(usable_size - 12) * 64 / 255 - 23`, a smaller threshold. Passing the
+/// wrong one for an index cell computes a `local_size` that doesn't fit
+/// the space SQLite actually reserved on the page — caught in practice
+/// once a fixture forces an index cell's payload past the *correct*
+/// (smaller) index threshold while still under the table one (#Req 7).
+/// All arithmetic saturates rather than panics — a pathological
+/// `usable_size` degrades to a safe (wrong but non-panicking) answer,
+/// caught by the length checks around the call site instead of an
+/// arithmetic panic here.
+fn local_payload_size(usable_size: u32, payload_len: u64, is_index: bool) -> u64 {
+    let max_local = if is_index {
+        ((usable_size.saturating_sub(12) as u64).saturating_mul(64) / 255).saturating_sub(23)
+    } else {
+        usable_size.saturating_sub(35) as u64
+    };
     if payload_len <= max_local {
         return payload_len;
     }
@@ -643,8 +654,9 @@ fn first_overflow_page(
     payload_len: u64,
     usable_size: u32,
     page_num: u32,
+    is_index: bool,
 ) -> Result<Option<u32>, BtreeError> {
-    let local_size = local_payload_size(usable_size, payload_len) as usize;
+    let local_size = local_payload_size(usable_size, payload_len, is_index) as usize;
     if (local_size as u64) < payload_len {
         Ok(Some(read_u32(
             buf,
@@ -702,6 +714,7 @@ fn reassemble_payload<P: PageSource>(
     page: &Rc<[u8]>,
     tail_start: usize,
     payload_len: u64,
+    is_index: bool,
 ) -> Result<Payload, BtreeError> {
     if payload_len > MAX_PAYLOAD_LEN {
         return Err(BtreeError::PayloadTooLarge {
@@ -712,7 +725,7 @@ fn reassemble_payload<P: PageSource>(
     let cell_tail = page
         .get(tail_start..)
         .ok_or(BtreeError::PayloadTooShort { page_num })?;
-    let local_size = local_payload_size(usable_size, payload_len) as usize;
+    let local_size = local_payload_size(usable_size, payload_len, is_index) as usize;
     let local_bytes = cell_tail
         .get(..local_size)
         .ok_or(BtreeError::PayloadTooShort { page_num })?;
@@ -877,9 +890,14 @@ fn free_btree_pages_inner(
                 let ptr_off = cell_ptr_offset(ptr_base, i);
                 let cell_start = read_cell_pointer(&buf, ptr_off, page_num, i)?;
                 let (_, payload_len, tail_start) = decode_cell_head(&buf, cell_start, page_num)?;
-                if let Some(overflow_page) =
-                    first_overflow_page(&buf, tail_start, payload_len, usable_size, page_num)?
-                {
+                if let Some(overflow_page) = first_overflow_page(
+                    &buf,
+                    tail_start,
+                    payload_len,
+                    usable_size,
+                    page_num,
+                    false,
+                )? {
                     free_overflow_chain(pager, page_num, overflow_page, visited)?;
                 }
             }
@@ -900,7 +918,7 @@ fn free_btree_pages_inner(
                 let (payload_len, tail_start) =
                     index::decode_payload_len(&buf, cell_start, page_num)?;
                 if let Some(overflow_page) =
-                    first_overflow_page(&buf, tail_start, payload_len, usable_size, page_num)?
+                    first_overflow_page(&buf, tail_start, payload_len, usable_size, page_num, true)?
                 {
                     free_overflow_chain(pager, page_num, overflow_page, visited)?;
                 }
@@ -916,7 +934,7 @@ fn free_btree_pages_inner(
                 let (payload_len, tail_start) =
                     index::decode_payload_len(&buf, value_start, page_num)?;
                 if let Some(overflow_page) =
-                    first_overflow_page(&buf, tail_start, payload_len, usable_size, page_num)?
+                    first_overflow_page(&buf, tail_start, payload_len, usable_size, page_num, true)?
                 {
                     free_overflow_chain(pager, page_num, overflow_page, visited)?;
                 }
@@ -1022,7 +1040,7 @@ pub(super) fn collect_leaf_cells(
         let ptr_off = cell_ptr_offset(ptr_base, i);
         let cell_start = read_cell_pointer(buf, ptr_off, page_num, i)?;
         let (rowid, payload_len, tail_start) = decode_cell_head(buf, cell_start, page_num)?;
-        let local_size = local_payload_size(usable_size, payload_len) as usize;
+        let local_size = local_payload_size(usable_size, payload_len, false) as usize;
         let has_overflow = (local_size as u64) < payload_len;
         let cell_end = tail_start
             .saturating_add(local_size)
@@ -1063,7 +1081,7 @@ pub(super) fn scan_leaf_cells(
         if cell_rowid > rowid && insert_pos == num_cells {
             insert_pos = i;
         }
-        let local_size = local_payload_size(usable_size, payload_len) as usize;
+        let local_size = local_payload_size(usable_size, payload_len, false) as usize;
         let has_overflow = (local_size as u64) < payload_len;
         let cell_end = tail_start
             .saturating_add(local_size)
@@ -1097,7 +1115,7 @@ pub(super) fn find_leaf_cell(
         if cell_rowid != rowid {
             continue;
         }
-        let local_size = local_payload_size(usable_size, payload_len) as usize;
+        let local_size = local_payload_size(usable_size, payload_len, false) as usize;
         let overflow_page = if (local_size as u64) < payload_len {
             read_u32(buf, tail_start.saturating_add(local_size), page_num)?
         } else {
@@ -1526,7 +1544,7 @@ pub(super) fn splice_delete_cell(
     } else {
         index::decode_payload_len(buf, cell_start, page_num)?
     };
-    let local_size = local_payload_size(usable_size, payload_len) as usize;
+    let local_size = local_payload_size(usable_size, payload_len, !has_rowid) as usize;
     let has_overflow = (local_size as u64) < payload_len;
     let cell_end = tail_start
         .saturating_add(local_size)
@@ -2326,7 +2344,7 @@ mod tests {
         // denom` remainder on opposite sides of a denom (508) multiple,
         // making the two paths diverge to entirely different results (70
         // vs 167) instead of coincidentally agreeing.
-        assert_eq!(local_payload_size(512, 5150), 70);
+        assert_eq!(local_payload_size(512, 5150, false), 70);
     }
 
     #[test]
@@ -2335,7 +2353,8 @@ mod tests {
             pages: HashMap::new(),
         };
         let page: Rc<[u8]> = Rc::from(Vec::new().as_slice());
-        let err = reassemble_payload(&source, 512, 2, &page, 0, MAX_PAYLOAD_LEN).unwrap_err();
+        let err =
+            reassemble_payload(&source, 512, 2, &page, 0, MAX_PAYLOAD_LEN, false).unwrap_err();
         assert!(!matches!(err, BtreeError::PayloadTooLarge { .. }));
     }
 
@@ -2361,7 +2380,7 @@ mod tests {
         let mut cell = Vec::new();
         cell.extend_from_slice(&encode_varint_for_test(5000));
         cell.extend_from_slice(&encode_varint_for_test(1));
-        let local_size = local_payload_size(512, 5000) as usize;
+        let local_size = local_payload_size(512, 5000, false) as usize;
         cell.extend(std::iter::repeat_n(0u8, local_size));
         cell.extend_from_slice(&99u32.to_be_bytes());
         page[cell_start..cell_start.saturating_add(cell.len())].copy_from_slice(&cell);

src/btree/index.rs

@@ -360,6 +360,7 @@ impl<P: PageSource> IndexCursor<P> {
             &frame.page,
             tail_start,
             payload_len,
+            true,
         )?;
         Ok(IndexRow { payload })
     }
@@ -388,6 +389,7 @@ impl<P: PageSource> IndexCursor<P> {
             &frame.page,
             tail_start,
             payload_len,
+            true,
         )?;
         Ok(IndexRow { payload })
     }
@@ -525,7 +527,7 @@ fn decode_value_cell(
     encoding: TextEncoding,
 ) -> Result<(Vec<Value>, Vec<u8>), BtreeError> {
     let (payload_len, tail_start) = decode_payload_len(buf, value_start, page_num)?;
-    let local_size = local_payload_size(usable_size, payload_len) as usize;
+    let local_size = local_payload_size(usable_size, payload_len, true) as usize;
     let has_overflow = (local_size as u64) < payload_len;
     let cell_end = tail_start
         .saturating_add(local_size)
@@ -542,6 +544,7 @@ fn decode_value_cell(
         &page,
         tail_start,
         payload_len,
+        true,
     )?;
     let key = decode_record(&payload, encoding)?;
     Ok((key, cell_bytes))