Skip to content

test: cover uncaught JSON parse diagnostics - #4398

Closed
sxlijin wants to merge 2 commits into
canaryfrom
sxlijin/B-356-uncaught-test-diagnostics
Closed

test: cover uncaught JSON parse diagnostics#4398
sxlijin wants to merge 2 commits into
canaryfrom
sxlijin/B-356-uncaught-test-diagnostics

Conversation

@sxlijin

@sxlijin sxlijin commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Summary

Updates the CLI end-to-end regression test for B-356 to use the issue's exact failure mode: an uncaught baml.json.JsonParseError from parsing invalid JSON.

The production path already captures ErrorContext for uncaught throws. This test now verifies that baml test renders the thrown class, exact message, and BAML source stack instead of printing only a bare failure line, while continuing to reject leaked internal Rust span details.

Validation

  • cargo test -p baml_cli --test exit_code_e2e test_json_parse_error_prints_rendered_error_context -- --exact --nocapture
  • cargo test -p baml_cli --lib test_command::tests
  • cargo fmt --all -- --check
  • git diff --check

Linear: B-356

Summary by CodeRabbit

  • Bug Fixes
    • Improved error reporting for invalid numeric values in BAML JSON parsing.
    • Errors now include the appropriate parse error type, clear validation details, and relevant source context.
    • Removed internal debugging details from displayed error messages.
    • Parsing failures now return the expected exit code.
    • Malformed numeric input now produces a clear, actionable parsing error.

@linear

linear Bot commented Aug 13, 2026

Copy link
Copy Markdown

B-356

@vercel

vercel Bot commented Aug 13, 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 13, 2026 7:18pm
promptfiddle2 Ready Ready Preview Aug 13, 2026 7:18pm

Request Review

@coderabbitai

coderabbitai Bot commented Aug 13, 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: 532412ed-356c-4b69-a581-f416073a3b3b

📥 Commits

Reviewing files that changed from the base of the PR and between c2cd96b and f370a93.

📒 Files selected for processing (1)
  • baml_language/crates/baml_cli/tests/exit_code_e2e.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • baml_language/crates/baml_cli/tests/exit_code_e2e.rs

📝 Walkthrough

Walkthrough

The CLI end-to-end test now covers malformed numeric input passed to baml.json.from_string<Foo>. It verifies exit code 2, the rendered baml.json.JsonParseError, the invalid-number message, BAML source context, and the absence of internal debug output.

Changes

JSON parse exit-code regression

Layer / File(s) Summary
Malformed JSON error regression
baml_language/crates/baml_cli/tests/exit_code_e2e.rs
The test replaces the custom ProviderFailure scenario with malformed numeric input. Stderr assertions validate JsonParseError, the invalid-number message, BAML source context, and the absence of internal Span and FileId output.

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

Mergeability Score: ⚪ Minimal · up to f370a

This change only updates CLI regression coverage for JSON parse diagnostics, with no production behavior change; no actionable merge-blocking risk remains beyond normal checks and review.

Possibly related PRs

Poem

A rabbit tests a number gone wrong,
The parse error hops along.
Exit code two, source context clear,
No hidden spans appear near.
JSON guards the trail.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the added regression coverage for uncaught JSON parse diagnostics.
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.
✨ 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/B-356-uncaught-test-diagnostics

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.

@sxlijin
sxlijin marked this pull request as ready for review August 13, 2026 19:04

sxlijin commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor
⚠️ Action not completed

Already reviewed.

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 13, 2026 19:05 Inactive

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c2cd96b2fb

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread baml_language/crates/baml_cli/tests/exit_code_e2e.rs Outdated
@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.

@vercel
vercel Bot temporarily deployed to Preview – beps August 13, 2026 19:10 Inactive
@vercel
vercel Bot temporarily deployed to Preview – promptfiddle2 August 13, 2026 19:18 Inactive

sxlijin commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 13, 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.

@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.2 MB 11.1 MB file 27.4 MB -1.2 MB (-4.4%) OK
packed-program Linux 🔒 16.9 MB 6.8 MB file 18.6 MB -1.7 MB (-8.9%) OK
baml-cli macOS 🔒 20.4 MB 9.7 MB file 21.3 MB -913.9 KB (-4.3%) OK
packed-program macOS 🔒 13.3 MB 6.0 MB file 14.5 MB -1.2 MB (-8.1%) OK
baml-cli Windows 🔒 21.9 MB 9.9 MB file 23.0 MB -1.1 MB (-4.6%) OK
packed-program Windows 🔒 14.1 MB 6.1 MB file 15.5 MB -1.4 MB (-9.1%) OK
bridge_wasm WASM 15.5 MB 🔒 4.2 MB gzip 4.6 MB -437.3 KB (-9.5%) 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 commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

Closing as superseded: B-356 was already fixed by #4308 (cf3adeefe7). The Linear issue is now linked to that merged PR.

@sxlijin sxlijin closed this Aug 13, 2026
@sxlijin
sxlijin deleted the sxlijin/B-356-uncaught-test-diagnostics branch August 13, 2026 22:51
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