Repository navigation
[codex] Make array iterators lazy - #3747
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughArray had its convenience iterator methods removed; callers are migrated to baml.iter.Iterable.iter(). ArrayIterator now stores the original array (no clone), and tests are updated/added to validate eager and lazy iterator behaviors and the new adapter usage. ChangesIterator API Refactoring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
⏭️ Performance benchmarks were skippedPerf benchmarks (CodSpeed) are opt-in on pull requests — they no longer run on every push. They always run automatically after merge to To run them on this PR, do any of the following, then push a commit (or re-run CI):
|
aaronvg
marked this pull request as ready for review
June 11, 2026 22:27
|
No description provided. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
meefs
pushed a commit
to meefs/baml
that referenced
this pull request
Jun 12, 2026
… + live for-loop tests (BoundaryML#3749) ## Summary Follow-ups to BoundaryML#3747 (lazy array iterators): - **Fix nullable-element early termination.** `ArrayIterator.next` matched `arr.at(idx)` against `null` to detect the end of iteration, so an array of nullable `T` containing a `null` element stopped iterating at the first `null`. It now bounds-checks by index, so stored `null` elements are yielded. This was pre-existing, but live iterators flowing through more code paths (including every `for` loop) made it likelier to bite. - **Add eager `Array.filter_map`.** Eager `filter`/`map` return arrays, but `filter_map` only existed via `.iter().filter_map(fn).collect()`. This adds an eager array-returning form, implemented in BAML and mirroring `for_each`'s snapshot-via-`slice` idiom; the lazy version remains on `Iterator`. - **Pin `for`-loop live-view semantics.** BoundaryML#3747 made `ArrayIterator` hold the source array instead of a snapshot, and `for` loops lower through `Iterable.iter()`, so loops now see mid-loop mutations. That was only tested via explicit `.iter()`; this adds tests pinning that a `push` mid-loop is visited and a `pop` mid-loop terminates cleanly. ## Validation - `INSTA_UPDATE=always cargo insta test --test-runner nextest -p baml_tests -p baml_cli` - `cargo nextest run -p baml_tests --test baml_src` (2 passed) - `cargo nextest run -p baml_tests --lib --test bytecode_format` (1599 passed) - `cargo nextest run -p baml_cli describe` (83 passed) - `git diff --check` 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes core iterator/`for`-loop semantics for nullable element types and documents live-array mutation during iteration; incorrect bounds logic could affect many call sites. > > **Overview** > Fixes **array iteration** when elements can be `null`: `ArrayIterator.next` now ends on an index past the length instead of treating `at()` returning `null` as exhaustion, so nullable arrays yield every slot including stored `null`s. > > Adds **eager `Array.filter_map`**, building a new array via a `slice`-bounded `for` loop (same snapshot pattern as `for_each`); lazy `iter().filter_map` is unchanged. > > New **stdlib tests** lock in `for`-loop behavior over live array views (push mid-loop is seen; repeated pop shortens iteration), eager `filter_map`, and nullable-element iteration/`count`. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit fc9a51f. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added `filter_map` method for arrays to combine mapping and filtering operations in a single operation. * **Bug Fixes** * Fixed iterator behavior to correctly yield null elements in arrays with nullable types, rather than treating them as iteration endpoints. * **Tests** * Added comprehensive iterator test coverage for array mutations and nullable element handling. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Fable 5 <[email protected]>
This branch was previously deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
baml.iter.ArrayIteratorkeep the source array instead of copying withsliceIteratorbaml_srccoverage for eager arrayfilter().map()withoutcollect(), lazy.iter().filter().map().collect(), and iterators seeing source-array mutations after creationValidation
INSTA_UPDATE=always cargo insta test --test-runner nextest -p baml_tests --test baml_src -- bytecodecargo nextest run -p baml_tests --test baml_srcgit diff --checkNote
Medium Risk
Breaking change to the standard library Array API and iterator semantics (live array vs snapshot); core language/stdlib behavior with broad test updates but user code calling removed methods will fail at compile time.
Overview
Arrays no longer expose iterator convenience methods (
filter_map,step_by,chain,peekable,collect,count). Lazy chaining usesarray.iter()and methods onbaml.iter.Iteratorinstead; eagerfilter/mapstill return arrays.ArrayIterator.newholds the source array instead ofslice(0, length), so iteration is lazy and sees later mutations (e.g.pushafter creating an iterator). The blanketIterableimpl forT[]initer.bamlis unchanged in spirit but now wires through that non-copying constructor.Tests and compiler snapshots are updated for the new call pattern, with added coverage for eager
filter().map(), lazy.iter()…collect(), and mutation visibility.Reviewed by Cursor Bugbot for commit 1a8c7e6. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
Refactor
Tests