fix(release): stamp C++ verifier version - #4347
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
⏭️ 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):
|
📝 WalkthroughWalkthroughThe C++ verification workflow now stamps ChangesC++ release-plan verification
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
scripts/tests/test_release_pipeline_contract.py (1)
663-680: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winCover the complete C++ verification order.
The test does not verify that stamping occurs before the
jqversion extraction at Line 96 of.github/workflows/verify-cpp-sdk.reusable.yaml. It also checks only the forward-slash CMake command, so it does not cover the Windows command at Line 157. Use theverifyjob block and assert the order for every CMake path.Proposed assertion update
- self.assertLess( - workflow.index("scripts/baml-language-version stamp"), - workflow.index("cmake -S baml_language/sdks/cpp/bridge_cpp/tests"), - ) + stamp_index = verify.index( + "scripts/baml-language-version stamp --plan release-plan.json" + ) + self.assertLess( + stamp_index, + verify.index( + 'expected="$(jq -r .canonical_version release-plan.json)"' + ), + ) + configure_positions = [ + match.start() + for match in re.finditer( + r"cmake -S baml_language[/\\]sdks[/\\]cpp[/\\]bridge_cpp[/\\]tests", + verify, + ) + ] + self.assertTrue(configure_positions) + self.assertTrue( + all(stamp_index < position for position in configure_positions) + )🤖 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 `@scripts/tests/test_release_pipeline_contract.py` around lines 663 - 680, Extend test_cpp_verifier_stamps_the_frozen_release_plan to assert that the release-plan JSON printf and scripts/baml-language-version stamp occur before the jq version extraction within the verify job block. Also assert the stamp precedes both the forward-slash and Windows CMake test commands, using verify rather than the full workflow for all ordering checks.
🤖 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.
Nitpick comments:
In `@scripts/tests/test_release_pipeline_contract.py`:
- Around line 663-680: Extend test_cpp_verifier_stamps_the_frozen_release_plan
to assert that the release-plan JSON printf and scripts/baml-language-version
stamp occur before the jq version extraction within the verify job block. Also
assert the stamp precedes both the forward-slash and Windows CMake test
commands, using verify rather than the full workflow for all ordering checks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d52c8949-e63d-4a93-bca6-0f122f375bb9
📒 Files selected for processing (2)
.github/workflows/verify-cpp-sdk.reusable.yamlscripts/tests/test_release_pipeline_contract.py
Binary size checks passed✅ 7 passed
Generated by |
Issue Reference
Failed release job: https://github.com/BoundaryML/baml/actions/runs/31158395652/job/92807446666
Changes
The native runtime was stamped as 0.15.1-nightly.20260806.a, but the checked-out C++ headers still identified the bridge and required toolchain as 0.15.0.
Testing
Screenshots
Not applicable.
PR Checklist
Additional Notes
No data structures or runtime behavior changed.
Summary by CodeRabbit
Bug Fixes
Tests