diff --git a/ndc_vm/src/iterator.rs b/ndc_vm/src/iterator.rs index c50b89ee..f4d2c25d 100644 --- a/ndc_vm/src/iterator.rs +++ b/ndc_vm/src/iterator.rs @@ -462,6 +462,18 @@ impl VmIterator for CombinationsIter { return None; } } else { + // For lazy sources: pull one more element before choosing the + // pivot. Without this, when `indices[k-1]` reaches the end of + // the current buffer the algorithm advances an earlier index + // (because `indices[k-1] < pool_len - 1` is false against the + // not-yet-extended pool), skipping combinations whose rightmost + // index lives in the unrealised tail. Pulling first makes the + // rightmost index pivot whenever the source still has elements, + // matching the eager-source lex order. + if self.source.is_some() { + let _ = self.ensure_index(self.indices[k - 1] + 1); + } + let pool_len = self.buffer.len(); if pool_len < k { return None; @@ -469,27 +481,13 @@ impl VmIterator for CombinationsIter { let pivot = match (0..k).rev().find(|&i| self.indices[i] < pool_len - k + i) { Some(p) => p, - None if self.source.is_none() => return None, - // For lazy sources: pulling one more element always unlocks index k-1. - // When pivot = None, indices[j] = pool_len - k + j for all j, so - // indices[k-1] = pool_len - 1. After one pull, pool_len grows by 1 and - // indices[k-1] < new_pool_len - k + (k-1) holds. - None => { - if !self.ensure_index(self.buffer.len()) { - return None; - } - k - 1 - } + None => return None, }; self.indices[pivot] += 1; for j in (pivot + 1)..k { self.indices[j] = self.indices[j - 1] + 1; } - // For lazy sources, buffer up to the new last index. - if !self.ensure_index(self.indices[k - 1]) { - return None; - } } // All indices are in-bounds: direct indexing is safe. diff --git a/tests/functional/programs/900_bugs/bug0021_combinations_lazy_source.ndc b/tests/functional/programs/900_bugs/bug0021_combinations_lazy_source.ndc new file mode 100644 index 00000000..788304d0 --- /dev/null +++ b/tests/functional/programs/900_bugs/bug0021_combinations_lazy_source.ndc @@ -0,0 +1,28 @@ +// CombinationsIter over a lazy source (e.g. `.enumerate()`) used to skip +// combinations whose rightmost index lay in the not-yet-pulled tail. +// `indices[k-1]` would hit the buffer end before the source was drained, +// and the pivot algorithm would advance an earlier index instead of +// pulling one more — yielding combinations out of lex order and missing +// some entirely. Fix: pull `indices[k-1] + 1` before computing pivot. + +// Eager (list) reference — correct for both pre- and post-fix. +assert_eq([1, 2, 3, 4].combinations(2).list(), [(1, 2), (1, 3), (1, 4), (2, 3), (2, 4), (3, 4)]); + +// Lazy (iterator) source — this is the path the bug lived in. +assert_eq( + [1, 2, 3, 4].enumerate().combinations(2).list(), + [ + ((0, 1), (1, 2)), + ((0, 1), (2, 3)), + ((0, 1), (3, 4)), + ((1, 2), (2, 3)), + ((1, 2), (3, 4)), + ((2, 3), (3, 4)) + ] +); + +// Larger k — exercise the multi-pull-per-pivot path. +let triples = [10, 20, 30, 40, 50].enumerate().combinations(3).list(); +assert_eq(triples.len, 10); +assert_eq(triples[0], ((0, 10), (1, 20), (2, 30))); +assert_eq(triples[-1], ((2, 30), (3, 40), (4, 50)));