Repository navigation
fix(stdlib): ArrayIterator nullable elements + eager Array.filter_map + live for-loop tests - #3749
Conversation
…filter_map; pin live for-loop semantics Follow-ups to #3747 (lazy array iterators): - Fix ArrayIterator.next to bounds-check by index instead of matching `at()` against null, so arrays of nullable T containing null elements iterate all elements rather than terminating early at the first null. - Add eager `Array.filter_map` returning `U[]`, restoring symmetry with eager `filter`/`map` (lazy version remains `iter().filter_map(fn)`). - Add baml_src tests pinning that `for` loops are a live view of the source array (pushes mid-loop are visited; pops mid-loop terminate cleanly), plus coverage for the nullable-element fix and eager filter_map. Co-Authored-By: Claude Fable 5 <[email protected]>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (9)
📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR adds ChangesArray iteration and filter_map enhancement
Sequence DiagramsequenceDiagram
participant Caller
participant Array as Array[T]
participant Iterator as ArrayIterator[T]
participant Callback as fn(T)->U?
participant Result as Array[U]
Caller->>Array: filter_map(fn)
loop for each element
Array->>Iterator: next()
Iterator-->>Array: element (or Done)
Array->>Callback: fn(element)
Callback-->>Array: U? (mapped value)
Array->>Result: append if not null
end
Array-->>Caller: return Array[U]
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 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):
|
Co-Authored-By: Claude Fable 5 <[email protected]>
|
No description provided. |
Summary
Follow-ups to #3747 (lazy array iterators):
ArrayIterator.nextmatchedarr.at(idx)againstnullto detect the end of iteration, so an array of nullableTcontaining anullelement stopped iterating at the firstnull. It now bounds-checks by index, so storednullelements are yielded. This was pre-existing, but live iterators flowing through more code paths (including everyforloop) made it likelier to bite.Array.filter_map. Eagerfilter/mapreturn arrays, butfilter_maponly existed via.iter().filter_map(fn).collect(). This adds an eager array-returning form, implemented in BAML and mirroringfor_each's snapshot-via-sliceidiom; the lazy version remains onIterator.for-loop live-view semantics. [codex] Make array iterators lazy #3747 madeArrayIteratorhold the source array instead of a snapshot, andforloops lower throughIterable.iter(), so loops now see mid-loop mutations. That was only tested via explicit.iter(); this adds tests pinning that apushmid-loop is visited and apopmid-loop terminates cleanly.Validation
INSTA_UPDATE=always cargo insta test --test-runner nextest -p baml_tests -p baml_clicargo 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
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.nextnow ends on an index past the length instead of treatingat()returningnullas exhaustion, so nullable arrays yield every slot including storednulls.Adds eager
Array.filter_map, building a new array via aslice-boundedforloop (same snapshot pattern asfor_each); lazyiter().filter_mapis unchanged.New stdlib tests lock in
for-loop behavior over live array views (push mid-loop is seen; repeated pop shortens iteration), eagerfilter_map, and nullable-element iteration/count.Reviewed by Cursor Bugbot for commit fc9a51f. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
filter_mapmethod for arrays to combine mapping and filtering operations in a single operation.Bug Fixes
Tests