Repository navigation
feat(stdlib): baml.csv — streaming CSV readers/writers (BEP-060) - #3744
Merged
Merged
Conversation
…-060) Add the baml.csv standard library: streaming CsvReader/CsvWriter handles built around baml.iter.Iterator, typed decode (rows<T>/decode<T>/read<T>) with the baml.json column-matching family, structured positional CsvError throws with per-record resumability and on_error="skip" diagnostics, the cardinality one-shots (decode_optional/decode_one), a full writer (quote_style, terminator, BOM, sanitize_formulas, atomic write_rows batches), and to_markdown prompt rendering. All parsing/encoding is VM-native (bex_vm/package_baml/csv.rs); file IO is pumped through the existing baml.fs sysops from thin BAML method bodies, so no new sysops and no engine/wasm changes are needed. Cell decode covers string/int/bigint/float/bool/enums plus baml.time Instant, PlainDate, and PlainDateTime. CSV is the first stdlib namespace with generic native methods and the first cross-namespace implementor of baml.iter.Iterator, which exposed three latent compiler bugs fixed here (none CSV-specific, each with independent regression tests): - TIR: explicit call-site type args on method calls (x.foo<int>()) were silently dropped — the declared-generic-params lookup missed path-shaped callees and now falls back to path_member_resolutions, accepting only method-like entries so abstract-interface-method arity checks survive. - MIR: interface dispatch guards dropped implementors whose associated bindings are typevar unions (type Error = E | E2) against normalized requests; assoc bindings now fall back to unpinned wildcard guards (frame seeded from the runtime instance) while positional args keep strict pairwise pinning. - LSP check: implements-signature matching now lowers class-level generics semantically instead of degrading to string equality, so cross-namespace generic implementors (CsvRows<T>) validate cleanly. Deviations from the BEP forced by current language features are documented in csv.baml doc comments (ReaderOptions? = null instead of class-literal defaults, runtime Options error for non-flat decode targets, header parse errors exhaust the reader, rows<T> returns the Iterator interface type).
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
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 (1)
📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughAdds a full BAML CSV standard library (public API + native VM engine), registers the builtin, widens time helper visibility, updates compiler/LSP generic binding and callee-resolution logic, and adds extensive BAML and Rust tests for CSV read/write, decoding, error handling, markdown rendering, and regressions. ChangesCSV Standard Library and VM
Compiler & LSP Adjustments
Tests
Sequence DiagramsequenceDiagram
participant User
participant CsvReader
participant Scanner
participant Decoder
participant VmNamespace
User->>CsvReader: rows<T>() / next()
CsvReader->>Scanner: scan_record()
Scanner->>CsvReader: Record / Skip / NeedData
CsvReader->>Decoder: decode_record()
Decoder->>VmNamespace: construct instance or emit CsvError
User->>CsvReader: close()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
baml_language/crates/bex_vm/src/package_baml/csv.rs (1)
802-985: ⚡ Quick winConsider adding Rust unit tests for the tokenizer.
The
scan_recordfunction contains complex quote-aware parsing logic with many edge cases. While the BAML-level tests provide good end-to-end coverage, unit tests for the tokenizer would make it easier to verify edge cases in isolation and catch regressions during future refactoring.Example test cases that would benefit from direct unit testing:
- Unterminated quotes at various positions
- RFC 4180 quote doubling vs escape-character dialects
- Stray quotes in unquoted fields
- Data after closing quote
As per coding guidelines: "Prefer writing Rust unit tests over integration tests where possible".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/crates/bex_vm/src/package_baml/csv.rs` around lines 802 - 985, Add focused Rust unit tests for the tokenizer by exercising scan_record directly: create tests in a #[cfg(test)] mod that construct ReaderOpts (set delimiter, quote, escape, quoting, trim, skip_blank) and call scan_record with tailored byte buffers and eof flags to assert returned Scan variants and fields. Include tests for unterminated quotes (expect Scan::Malformed with Kind::Quote and exhausted true/false as appropriate), RFC4180 quote doubling vs escape-character dialects (double-quote consumes vs escape uses next byte), stray quotes in unquoted fields (expect Scan::Malformed Kind::Quote), and unexpected data after closing quote (err then Record with cell contents or Malformed depending on consumed). Use finish_cell/scan_record behavior to verify consumed, lines, cells, and exhausted flags so future refactors catch regressions.Source: Coding guidelines
baml_language/crates/baml_lsp2_actions/src/check.rs (1)
1001-1014: ⚡ Quick winAdd a unit regression for outer-generic signature matching.
This matcher now depends on enclosing generics being threaded into semantic lowering, but the local
#[cfg(test)]module does not cover the new path. A focused unit test for bothclass C<T> implements I<T>and out-of-bodyimplements<U> ... for C<U>would pin this behavior down and catch any future fallback to string-only matching. As per coding guidelines, "Prefer writing Rust unit tests over integration tests where possible."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/crates/baml_lsp2_actions/src/check.rs` around lines 1001 - 1014, Add a focused unit test in the local #[cfg(test)] module that exercises the new outer-generic path used by matches(&self, other: &Self, ctx: SignatureMatchContext) in check.rs: create cases for a type declared with enclosing generics (e.g., class C<T> implements I<T>) and an out-of-body impl with its own generics (e.g., implements<U> ... for C<U>), invoking the semantic lowering/path that populates ctx.outer_generic_params so the gp_all branch is hit; assert that signature matching succeeds (and also include a negative case that would fail if matching fell back to string-only), thereby preventing regressions where outer_generic_params are ignored.Source: Coding guidelines
baml_language/crates/baml_tests/baml_src/ns_csv/csv.baml (1)
1256-1261: ⚡ Quick winInline
catchexpressions instead of storing them in localletvariables.This
let caught = ({...} catch ...)pattern is a known test fragility point in BAML sources; keep thecatchexpression inline in the returned/asserted expression to avoid boxed-value behavior.Suggested pattern update
- let caught = { - w.write_row<MdRow>(MdRow { metric: "bad", value: float.nan() }); - "no-throw" - } catch (e) { - baml.csv.CsvError => kind_name(e.kind), - }; - w.write_row<MdRow>(MdRow { metric: "ok", value: 1.5 }); - caught + "#" + w.text() + "#" + baml.json.stringify(w.records_written()) + let result = + ({ + w.write_row<MdRow>(MdRow { metric: "bad", value: float.nan() }); + "no-throw" + } catch (e) { + baml.csv.CsvError => kind_name(e.kind), + }); + w.write_row<MdRow>(MdRow { metric: "ok", value: 1.5 }); + result + "#" + w.text() + "#" + baml.json.stringify(w.records_written())Based on learnings, avoid binding the result of a
catchexpression to aletin BAML test sources because boxed values can break downstream assertions/comparisons.Also applies to: 1281-1286, 1302-1310
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/crates/baml_tests/baml_src/ns_csv/csv.baml` around lines 1256 - 1261, The test binds a catch expression to a local `let caught` which can create boxed values and break assertions; instead, remove the `let caught =` binding and place the `({...} catch (e) { baml.csv.CsvError => kind_name(e.kind) })` expression directly where the value is used (inline the catch) — locate the pattern around the `w.write_row<MdRow>(MdRow { metric: "bad", value: float.nan() }); "no-throw" } catch (e) { baml.csv.CsvError => kind_name(e.kind) }` usage (and the similar occurrences near the other ranges) and replace the `let` assignment with the catch expression itself so comparisons/assertions operate on the raw catch result instead of a boxed local.Source: Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@baml_language/crates/baml_compiler2_mir/src/lower.rs`:
- Around line 521-533: The assoc_union_wildcard fallback currently allows
matching when associated members remain unresolved, letting fparts/aparts logic
succeed without pinning class parameters and leaving impl_ty with typevars;
update the condition around assoc_union_wildcard (and the later
associated-binding check that consults impl_ty) so that you either require at
least one class parameter to be pinned (i.e., ensure a concrete class-arg match
for the class-param members) before treating the associated-union as a wildcard
OR add a runtime re-check against the receiver’s concrete class args before
selecting the arm: modify the code around assoc_union_wildcard, fparts, aparts
and is_class_param (and the use of
baml_compiler2_tir::normalize::is_same_normalized_type) to ensure unresolved
associated-union members do not permit a match unless class args are pinned,
otherwise perform a final runtime comparison of the receiver’s concrete class
args to the requested union binding to reject unsafe matches; also apply the
same guard where impl_ty is examined for typevars (the later associated-binding
check) so unresolved typevars cannot bypass this check.
In `@baml_language/crates/baml_tests/tests/interfaces.rs`:
- Around line 11003-11064: The CI test failure is due to running `cargo test
--lib` from the wrong directory; rerun the tests from the crate or workspace
root that contains the Cargo.toml so the baml_tests crate is discovered (this
will run tests like positional_typevar_union_interface_args_dispatch and
field_chain_abstract_interface_method_arity_checked successfully); ensure your
test command is executed in the repository root (or the specific crate directory
containing Cargo.toml) or update your CI/job script to cd into the correct
workspace/crate root before running `cargo test --lib`.
---
Nitpick comments:
In `@baml_language/crates/baml_lsp2_actions/src/check.rs`:
- Around line 1001-1014: Add a focused unit test in the local #[cfg(test)]
module that exercises the new outer-generic path used by matches(&self, other:
&Self, ctx: SignatureMatchContext) in check.rs: create cases for a type declared
with enclosing generics (e.g., class C<T> implements I<T>) and an out-of-body
impl with its own generics (e.g., implements<U> ... for C<U>), invoking the
semantic lowering/path that populates ctx.outer_generic_params so the gp_all
branch is hit; assert that signature matching succeeds (and also include a
negative case that would fail if matching fell back to string-only), thereby
preventing regressions where outer_generic_params are ignored.
In `@baml_language/crates/baml_tests/baml_src/ns_csv/csv.baml`:
- Around line 1256-1261: The test binds a catch expression to a local `let
caught` which can create boxed values and break assertions; instead, remove the
`let caught =` binding and place the `({...} catch (e) { baml.csv.CsvError =>
kind_name(e.kind) })` expression directly where the value is used (inline the
catch) — locate the pattern around the `w.write_row<MdRow>(MdRow { metric:
"bad", value: float.nan() }); "no-throw" } catch (e) { baml.csv.CsvError =>
kind_name(e.kind) }` usage (and the similar occurrences near the other ranges)
and replace the `let` assignment with the catch expression itself so
comparisons/assertions operate on the raw catch result instead of a boxed local.
In `@baml_language/crates/bex_vm/src/package_baml/csv.rs`:
- Around line 802-985: Add focused Rust unit tests for the tokenizer by
exercising scan_record directly: create tests in a #[cfg(test)] mod that
construct ReaderOpts (set delimiter, quote, escape, quoting, trim, skip_blank)
and call scan_record with tailored byte buffers and eof flags to assert returned
Scan variants and fields. Include tests for unterminated quotes (expect
Scan::Malformed with Kind::Quote and exhausted true/false as appropriate),
RFC4180 quote doubling vs escape-character dialects (double-quote consumes vs
escape uses next byte), stray quotes in unquoted fields (expect Scan::Malformed
Kind::Quote), and unexpected data after closing quote (err then Record with cell
contents or Malformed depending on consumed). Use finish_cell/scan_record
behavior to verify consumed, lines, cells, and exhausted flags so future
refactors catch regressions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 22b5c53f-6756-4bd2-9125-caa0293c3973
⛔ Files ignored due to path filters (18)
baml_language/crates/baml_cli/src/snapshots/baml_cli__describe_command_tests__render_builtin_package_listing.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/_root.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/csv.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/__baml_std__/baml_tests__compiles____baml_std____03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/__baml_std__/baml_tests__compiles____baml_std____04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/__baml_std__/baml_tests__compiles____baml_std____04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/__baml_std__/baml_tests__compiles____baml_std____06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/method_explicit_type_args/baml_tests__compiles__method_explicit_type_args__01_lexer__method_explicit_type_args.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/method_explicit_type_args/baml_tests__compiles__method_explicit_type_args__02_parser__method_explicit_type_args.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/method_explicit_type_args/baml_tests__compiles__method_explicit_type_args__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/method_explicit_type_args/baml_tests__compiles__method_explicit_type_args__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/method_explicit_type_args/baml_tests__compiles__method_explicit_type_args__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/method_explicit_type_args/baml_tests__compiles__method_explicit_type_args__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/method_explicit_type_args/baml_tests__compiles__method_explicit_type_args__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/method_explicit_type_args/baml_tests__compiles__method_explicit_type_args__10_formatter__method_explicit_type_args.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__phase5__snapshot_baml_package_items.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/tests/bytecode_format/snapshots/bytecode_format__bytecode_display_expanded.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/tests/bytecode_format/snapshots/bytecode_format__bytecode_display_expanded_unoptimized.snapis excluded by!**/*.snap
📒 Files selected for processing (12)
baml_language/crates/baml_builtins2/baml_std/baml/ns_csv/csv.bamlbaml_language/crates/baml_builtins2/src/lib.rsbaml_language/crates/baml_compiler2_mir/src/lower.rsbaml_language/crates/baml_compiler2_tir/src/builder.rsbaml_language/crates/baml_lsp2_actions/src/check.rsbaml_language/crates/baml_tests/baml_src/ns_csv/csv.bamlbaml_language/crates/baml_tests/projects/compiles/method_explicit_type_args/method_explicit_type_args.bamlbaml_language/crates/baml_tests/tests/csv.rsbaml_language/crates/baml_tests/tests/interfaces.rsbaml_language/crates/bex_vm/src/package_baml/csv.rsbaml_language/crates/bex_vm/src/package_baml/mod.rsbaml_language/crates/bex_vm/src/package_baml/time.rs
…0114 for assoc-binding duplicates The assoc-binding wildcard in interface dispatch-guard unification leaves unpinnable typevar-union bindings (type Error = E1 | E2 against a normalized request) as wildcard guard entries. Document the three invariants that make this safe — concrete-class identity check on every arm, E0114 rejection of same-class blocks differing only in assoc bindings, and frame args seeding from the runtime instance — and pin the E0114 invariant with a compile-error test, since the safety argument leans on it. Also note that the typevar skip in the guard's assoc post-check is only reachable after binding inference structurally vetted the same pairs.
# Conflicts: # baml_language/crates/baml_tests/tests/bytecode_format/snapshots/bytecode_format__bytecode_display_expanded.snap # baml_language/crates/baml_tests/tests/bytecode_format/snapshots/bytecode_format__bytecode_display_expanded_unoptimized.snap
⏭️ 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):
|
|
No description provided. |
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
Implements BEP-060: a
baml.csvstandard library namespace for reading and writing CSV, built around streamingCsvReader/CsvWriterhandles that implementbaml.iter.Iterator(BEP-051).Surface
CsvReaderis a lazy iterator overCsvRecords; one cursor shared by raw iteration,rows<T>(), andheaders(). Every BEP-051 adapter/default method (map,filter,collect, …) works on it.rows<T>/decode<T>/read<T>/decode_optional<T>/decode_one<T>) follows thebaml.jsonverb family andbaml.sqlcardinality split; cells coverstring/int/bigint/float/bool/enums plusbaml.time.Instant,PlainDate,PlainDateTime. Strict, locale-free conversion — no value sniffing.CsvError { kind, message, line, record, field, column, expected, found }; malformed records throw per record and the reader resumes at the next record.on_error: "skip"withskipped()/skipped_count()/on_skipcovers dirty-input pipelines.trim,skip_lines,skip_blank_records,ragged(strict/pad/truncate),null_values,utf8/utf8-lossy, BOM handling,limit.quote_style(minimal/all/never),lf/crlf, auto/explicit headers,null_value, BOM,sanitize_formulas(OWASP CSV-injection guard);write_rows<T>batches are atomic — a thrownEncodeerror emits and counts nothing.to_markdown<T>/to_markdown_recordsfor prompt context, withmax_rowstruncation.Architecture
All tokenization, typed decoding, and encoding is VM-native (
bex_vm/src/package_baml/csv.rs). File IO is pumped through the existingbaml.fssysops (File.read_bytes/File.write) from thin BAML method bodies — thebaml.llm.Streampump pattern — so this adds zero new sysops and needs no engine, sys_native, or wasm changes. The compiler has no CSV-specific code; the namespace is discovered by compilingns_csv/csv.bamlthrough the normal pipeline like any other stdlib file.Compiler fixes (CSV-agnostic, individually regression-tested)
CSV is the first stdlib namespace with generic native methods and the first cross-namespace implementor of
baml.iter.Iterator, which exposed three latent bugs:x.foo<int>()) were silently dropped — the declared-generic-params lookup missed path-shaped callees. Now falls back topath_member_resolutions, accepting only method-like entries so abstract-interface-method arity checking is preserved.type Error = E | E2) against normalized requests. Assoc bindings now fall back to unpinned wildcard guards (frame seeded from the runtime instance); positional interface args keep strict pairwise pinning.implementssignature matching now lowers class-level generics semantically instead of degrading to string equality, so cross-namespace generic implementors validate cleanly.Per ARCHITECTURE.md's contribution guidance, the TIR/MIR diffs deserve owner eyes — they are deliberately small and carry CSV-independent tests in
tests/interfaces.rsandprojects/compiles/method_explicit_type_args/.Documented deviations from the BEP
Forced by current language features and noted in
csv.bamldoc comments:options: ReaderOptions? = null(class-literal parameter defaults unsupported), non-flat decode targets are a runtimeCsvError { kind: Options }(no custom compile-time checks), header parse errors exhaust the reader (continuing would silently install the next data row as the header),rows<T>returns thebaml.iter.Iteratorinterface type, and loop-attachedcatch(a BEP open question) does not exist yet — the reader is resumable so explicitcatchworks today.Test plan
baml_tests/baml_src/ns_csv/csv.baml: dialect matrix (line endings, quoting, escape dialects, TSV, BOM, comments, trim, blank records), the BEP null-semantics table, typed decode across all cell types, ragged policies, resumability/skip-mode/on_skip, cardinality one-shots, record/reader/writer APIs, file round-trips, Excel profile, markdown rendering, and regressions for every issue found in review (header poisoning, writer state atomicity, caller-owned handle lifecycle, PlainDate round-trip).baml_tests/tests/csv.rs.Summary by CodeRabbit
New Features
Bug Fixes
Tests