Skip to content

Commit 18e8bf3

Browse files
timfennisclaude
andcommitted
fix(vm): combinations on lazy iterators skipped tail combinations πŸ”’
CombinationsIter's pivot algorithm assumed `self.buffer.len()` reflects the full pool, but for lazy sources the buffer is filled on demand. When `indices[k-1]` reached the current buffer end the algorithm would declare the rightmost index "at max" against the not-yet-extended pool and advance an earlier index instead β€” yielding combinations out of lex order and skipping ones whose rightmost index lived in the unpulled tail. Concretely, `[1, 2, 3, 4].enumerate().combinations(2)` yielded five combinations instead of six, missing `((0, 1), (3, 4))`. Eager sources (`[...].combinations(...)`) were unaffected. Fix: pull `indices[k-1] + 1` before computing pivot. The rightmost index then pivots whenever the source still has elements, matching eager ordering. When pulls fail the buffer stays as-is and the pivot logic correctly falls back to advancing an earlier index. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
1 parent 3f2dbc6 commit 18e8bf3

2 files changed

Lines changed: 41 additions & 15 deletions

File tree

β€Žndc_vm/src/iterator.rsβ€Ž

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -462,34 +462,32 @@ impl VmIterator for CombinationsIter {
462462
return None;
463463
}
464464
} else {
465+
// For lazy sources: pull one more element before choosing the
466+
// pivot. Without this, when `indices[k-1]` reaches the end of
467+
// the current buffer the algorithm advances an earlier index
468+
// (because `indices[k-1] < pool_len - 1` is false against the
469+
// not-yet-extended pool), skipping combinations whose rightmost
470+
// index lives in the unrealised tail. Pulling first makes the
471+
// rightmost index pivot whenever the source still has elements,
472+
// matching the eager-source lex order.
473+
if self.source.is_some() {
474+
let _ = self.ensure_index(self.indices[k - 1] + 1);
475+
}
476+
465477
let pool_len = self.buffer.len();
466478
if pool_len < k {
467479
return None;
468480
}
469481

470482
let pivot = match (0..k).rev().find(|&i| self.indices[i] < pool_len - k + i) {
471483
Some(p) => p,
472-
None if self.source.is_none() => return None,
473-
// For lazy sources: pulling one more element always unlocks index k-1.
474-
// When pivot = None, indices[j] = pool_len - k + j for all j, so
475-
// indices[k-1] = pool_len - 1. After one pull, pool_len grows by 1 and
476-
// indices[k-1] < new_pool_len - k + (k-1) holds.
477-
None => {
478-
if !self.ensure_index(self.buffer.len()) {
479-
return None;
480-
}
481-
k - 1
482-
}
484+
None => return None,
483485
};
484486

485487
self.indices[pivot] += 1;
486488
for j in (pivot + 1)..k {
487489
self.indices[j] = self.indices[j - 1] + 1;
488490
}
489-
// For lazy sources, buffer up to the new last index.
490-
if !self.ensure_index(self.indices[k - 1]) {
491-
return None;
492-
}
493491
}
494492

495493
// All indices are in-bounds: direct indexing is safe.
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// CombinationsIter over a lazy source (e.g. `.enumerate()`) used to skip
2+
// combinations whose rightmost index lay in the not-yet-pulled tail.
3+
// `indices[k-1]` would hit the buffer end before the source was drained,
4+
// and the pivot algorithm would advance an earlier index instead of
5+
// pulling one more β€” yielding combinations out of lex order and missing
6+
// some entirely. Fix: pull `indices[k-1] + 1` before computing pivot.
7+
8+
// Eager (list) reference β€” correct for both pre- and post-fix.
9+
assert_eq([1, 2, 3, 4].combinations(2).list(), [(1, 2), (1, 3), (1, 4), (2, 3), (2, 4), (3, 4)]);
10+
11+
// Lazy (iterator) source β€” this is the path the bug lived in.
12+
assert_eq(
13+
[1, 2, 3, 4].enumerate().combinations(2).list(),
14+
[
15+
((0, 1), (1, 2)),
16+
((0, 1), (2, 3)),
17+
((0, 1), (3, 4)),
18+
((1, 2), (2, 3)),
19+
((1, 2), (3, 4)),
20+
((2, 3), (3, 4))
21+
]
22+
);
23+
24+
// Larger k β€” exercise the multi-pull-per-pivot path.
25+
let triples = [10, 20, 30, 40, 50].enumerate().combinations(3).list();
26+
assert_eq(triples.len, 10);
27+
assert_eq(triples[0], ((0, 10), (1, 20), (2, 30)));
28+
assert_eq(triples[-1], ((2, 30), (3, 40), (4, 50)));

0 commit comments

Comments
Β (0)