Skip to content

fix(csharp): align release verifier with generated surface - #4446

Merged
sxlijin merged 1 commit into
canaryfrom
sxlijin/fix-baml-language-0.17.0-csharp-release-verifier
Aug 15, 2026
Merged

fix(csharp): align release verifier with generated surface#4446
sxlijin merged 1 commit into
canaryfrom
sxlijin/fix-baml-language-0.17.0-csharp-release-verifier

Conversation

@sxlijin

@sxlijin sxlijin commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

Summary

Root cause

The phase-12 fixture is normally ignored because of B-1059, but the production full-surface package verifier compiles and runs it. Its stale calls to CsvReader.Iter/Next and CsvRows.Iter/Next no longer compile after #4366 removed interface-implementation methods from generated class method collections.

Validation

  • cargo nextest run -p sdk_test_csharp --all-features --run-ignored only -E test(test_phase12_executes_native_typed_resource_apis_lifetimes_and_state)
  • dotnet build sdk_tests/crates/csharp/phase12_resources/Phase12Resources.csproj --configuration Release --no-restore
  • git diff --check

Release follow-up

After this merges and post-merge canary CI succeeds, 0.17.0 must be dispatched from the new immutable baml-language source tag. This PR does not publish or dispatch the release.

Summary by CodeRabbit

  • Tests
    • Updated C# CSV resource validation to verify headers, reader availability, and the initial reading position.
    • Simplified resource cleanup validation to ensure the active CSV reader is properly closed.
    • Removed checks for iterator identity, typed row decoding, and raw record access.

@vercel

vercel Bot commented Aug 15, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
beps Ready Ready Preview Aug 15, 2026 4:34am
promptfiddle2 Ready Ready Preview Aug 15, 2026 4:34am

Request Review

@coderabbitai

coderabbitai Bot commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 16424d2d-e829-4794-a20f-55cd85dacea9

📥 Commits

Reviewing files that changed from the base of the PR and between e3301d1 and 2573d8c.

📒 Files selected for processing (1)
  • baml_language/sdk_tests/crates/csharp/phase12_resources/Program.cs

📝 Walkthrough

Walkthrough

The C# CSV resource test removes iterator, typed decoding, and raw-record assertions. It now checks headers, CsvRows.Reader, skipped-record state, and an initial position of 0. Cleanup closes csvReader.

Changes

CSV resource test updates

Layer / File(s) Summary
Reader contract validation
baml_language/sdk_tests/crates/csharp/phase12_resources/Program.cs
The test validates headers, the public reader, skipped-record state, and initial position. It removes row iteration and raw-record checks. Cleanup closes csvReader.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 2573d

This localized change updates the C# release verifier to match the generated API surface while retaining coverage for supported methods and fields. The targeted test and Release build are provided, and no actionable merge-blocking risk remains.

Suggested reviewers: rossirpaulo

Poem

A rabbit checks the reader’s start,
With headers lined in tidy art.
No rows are spun, no records roam,
csvReader closes, safe at home.
The CSV test hops on.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the C# release verifier fix and its alignment with the generated surface.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch sxlijin/fix-baml-language-0.17.0-csharp-release-verifier

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown

⏭️ Performance benchmarks were skipped

Perf benchmarks (CodSpeed) are opt-in on pull requests — they no longer run on every push. They always run automatically after merge to canary/main.

To run them on this PR, do any of the following, then push a commit (or re-run CI):

  • Add RUN_CODSPEED=1 to the PR description, or
  • Include run-perf or /perf in the PR title or any commit message.

@sxlijin

sxlijin commented Aug 15, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 15, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@vercel
vercel Bot temporarily deployed to Preview – beps August 15, 2026 04:26 Inactive
@sxlijin
sxlijin marked this pull request as ready for review August 15, 2026 04:28
@vercel
vercel Bot temporarily deployed to Preview – promptfiddle2 August 15, 2026 04:34 Inactive
@github-actions

Copy link
Copy Markdown

Binary size checks passed

7 passed

Artifact Platform File Gzip Gated on Baseline Delta Status
baml-cli Linux 🔒 26.5 MB 11.2 MB file 27.4 MB -912.8 KB (-3.3%) OK
packed-program Linux 🔒 17.2 MB 6.9 MB file 18.6 MB -1.4 MB (-7.4%) OK
baml-cli macOS 🔒 20.6 MB 9.7 MB file 21.3 MB -697.9 KB (-3.3%) OK
packed-program macOS 🔒 13.5 MB 6.1 MB file 14.5 MB -950.1 KB (-6.6%) OK
baml-cli Windows 🔒 22.2 MB 10.0 MB file 23.0 MB -770.7 KB (-3.4%) OK
packed-program Windows 🔒 14.4 MB 6.1 MB file 15.5 MB -1.1 MB (-7.4%) OK
bridge_wasm WASM 15.8 MB 🔒 4.2 MB gzip 4.6 MB -404.7 KB (-8.8%) OK

🔒 = the size this artifact is GATED on (ceiling + delta). Binaries gate on file size (installed binary); WASM gates on gzip (download size). The other size is shown for information only.


Generated by cargo size-gate · workflow run

@sxlijin

sxlijin commented Aug 15, 2026

Copy link
Copy Markdown
Contributor Author

Attempt 2 of the 0.17.0 release run confirmed the same deterministic failure: https://github.com/BoundaryML/baml/actions/runs/31857280879/attempts/2?pr=4446

The phase-12 consumer fails to compile because generated CsvReader has no Iter/Next and generated CsvRows has no Iter/Next/NextAsync. This PR removes those stale interface-implementation calls and has already passed the exact phase-12 native integration test locally.

@sxlijin
sxlijin added this pull request to the merge queue Aug 15, 2026
Merged via the queue into canary with commit d8ad5e5 Aug 15, 2026
75 checks passed
@sxlijin
sxlijin deleted the sxlijin/fix-baml-language-0.17.0-csharp-release-verifier branch August 15, 2026 07:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant