Skip to content

fix: restore E0007 for member access on unknown receivers - #4466

Merged
antoniosarosi merged 2 commits into
canaryfrom
agent/reflect-class-instance
Aug 18, 2026
Merged

antoniosarosi merged 2 commits into
canaryfrom
agent/reflect-class-instance

Conversation

@antoniosarosi

@antoniosarosi antoniosarosi commented Aug 17, 2026 •

Copy link
Copy Markdown
Contributor

Summary

The reflect.class.Instance / reflect.class.instance_from design was superseded and has been completely removed from this branch. This PR now retains only two independent fixes discovered during that work:

  • restore E0007 for unresolved method calls on unknown receivers, preventing invalid calls such as unknown_value.get_field<T>("x") from reaching emission and failing inside the VM
  • route the pre-existing reflect.class.get_field non-instance E0001 through the shared runtime diagnostic factory without changing its text or behavior

There is no replacement reflection API in this PR. The separately ratified reflect.AnyClass direction is out of scope.

Commits

  1. fix: restore unknown member diagnostics
  2. refactor: share class reflection mismatch diagnostic

Validation

  • CARGO_BUILD_JOBS=8 CARGO_INCREMENTAL=0 rustup run 1.93.0 cargo insta test --test-runner nextest -p baml_tests -p baml_cli -p baml_lsp2_actions -p baml_lsp2_actions_tests -p baml_surface --all-features --unreferenced=reject — 3,753 passed, 24 skipped; doctests clean; no unreferenced or pending snapshots
  • focused E0007 compile regression passed
  • shared diagnostic constructor oracle passed
  • commit hooks passed, including workspace Clippy and formatting

Superseded-design cleanup verification

All commands were run after the final commits. Empty results are shown explicitly.

$ rg -n --hidden -g '!target/**' -g '!.git/**' 'instance_from|reflect\.class\.Instance|baml\.reflect\.class\.Instance|copy::reflect::class::Instance' baml_language CHANGELOG.md
<empty>

$ rg -n --hidden -g '!target/**' -g '!.git/**' 'class[[:space:]]+Instance' baml_language
<empty>

$ git diff --unified=0 origin/canary...HEAD | rg '^\+.*(instance_from|reflect\.class\.Instance|class[[:space:]]+Instance|cannot_construct_reflection_instance|is_reflect_class_instance)'
<empty>

$ rg --files -g '*.snap.new'
<empty>

The final diff is limited to five files: the inference guard, its regression and changelog entry, the shared diagnostic factory, and the existing reflect.class.get_field implementation. There are no stdlib declarations, generated files, docs, completions, snapshots, or codegen changes.

Summary by CodeRabbit

  • Bug Fixes

    • Restored compile-time diagnostics when accessing unresolved members on unknown values.
    • Improved error messages when class-only operations receive non-class values.
    • Prevented invalid unresolved calls from reaching an internal runtime error.
  • Documentation

    • Added a changelog entry describing the restored diagnostics.

@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.

@coderabbitai

coderabbitai Bot commented Aug 17, 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: b5756029-aa38-49bc-8dc8-83f3d447b14d

📥 Commits

Reviewing files that changed from the base of the PR and between 98591e1 and 25d0afb.

📒 Files selected for processing (5)
  • baml_language/CHANGELOG.md
  • baml_language/crates/baml_compiler2_hir_ty/src/infer.rs
  • baml_language/crates/baml_compiler_diagnostics/src/runtime_type.rs
  • baml_language/crates/baml_tests/tests/runtime_classes_and_composites.rs
  • baml_language/crates/bex_vm/src/package_baml/type_kinds.rs
💤 Files with no reviewable changes (1)
  • baml_language/crates/baml_compiler2_hir_ty/src/infer.rs

Included review availability: Your plan includes up to 8 reviews per rolling hour; 6 remain after this review.


📝 Walkthrough

Walkthrough

This change restores unresolved-member diagnostics for unknown receivers and adds a dedicated diagnostic when get_field receives a non-class-instance value. Tests cover both compiler and runtime diagnostic behavior.

Changes

Reflection diagnostics

Layer / File(s) Summary
Unknown member diagnostic restoration
baml_language/crates/baml_compiler2_hir_ty/src/infer.rs, baml_language/crates/baml_tests/tests/runtime_classes_and_composites.rs, baml_language/CHANGELOG.md
Unknown receivers now produce unresolved-member diagnostics. Tests verify the get_field and unknown details.
Class instance validation diagnostics
baml_language/crates/baml_compiler_diagnostics/src/runtime_type.rs, baml_language/crates/bex_vm/src/package_baml/type_kinds.rs
get_field now reports a dedicated expected-class-instance diagnostic for invalid inputs. Diagnostic tests verify its code and message.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 25d0a

The PR restores the intended runtime-class reflection surface and reports successful targeted and broad validation; no actionable merge-blocking risk remains beyond normal checks and review.

Possibly related PRs

Poem

A rabbit checks the fields with care,
No unknown calls slip through the air.
Class instances pass the gate,
Clear diagnostics name their state.
Hop, hop—the tests now glow!

🚥 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 accurately describes the restored E0007 diagnostic for member access on unknown receivers, which is a documented change in the pull request.
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 agent/reflect-class-instance

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.

@vercel

vercel Bot commented Aug 17, 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 17, 2026 10:50pm
promptfiddle2 Ready Ready Preview Aug 17, 2026 10:50pm

Request Review

@vercel
vercel Bot temporarily deployed to Preview – promptfiddle2 August 17, 2026 12:39 Inactive
@github-actions

github-actions Bot commented Aug 17, 2026 •

Copy link
Copy Markdown

Binary size checks passed

✅ 7 passed

Artifact Platform File Gzip Gated on Baseline Delta Status
✅ baml-cli Linux 🔒 31.7 MB 12.6 MB file 31.7 MB +29.1 KB (+0.1%) OK
✅ packed-program Linux 🔒 24.9 MB 9.1 MB file 24.9 MB +27.9 KB (+0.1%) OK
✅ baml-cli macOS 🔒 25.4 MB 11.1 MB file 25.4 MB +70.9 KB (+0.3%) OK
✅ packed-program macOS 🔒 20.6 MB 8.2 MB file 20.6 MB +41.7 KB (+0.2%) OK
✅ baml-cli Windows 🔒 27.2 MB 11.3 MB file 27.2 MB +37.8 KB (+0.1%) OK
✅ packed-program Windows 🔒 21.7 MB 8.2 MB file 21.7 MB +12.4 KB (+0.1%) OK
✅ bridge_wasm WASM 21.3 MB 🔒 5.4 MB gzip 5.3 MB +33.6 KB (+0.6%) 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

@antoniosarosi

Copy link
Copy Markdown
Contributor Author

Applied the requested one-round SHIP-WITH-FIXES follow-up in d7c8724.

  • Restored committed unknown-member diagnostics, so unknown_value.get_field<T>(...) is rejected as E0007 before emission/VM execution.
  • Sealed baml.reflect.class.Instance object literals with a constructor-local check and recovery guidance to reflect.class.instance_from; the type-kind classifier remains unchanged.
  • Hoisted class-instance mismatch diagnostics into runtime_type, preserved existing wording, and made instance_from(null) report null.
  • Added static-instance/type-identity coverage plus updated unknown docs, describe coverage, and Instance completions.

Validation:

  • Pinned full gate: 3,756/3,756 passed, 24 skipped.
  • Snapshot audit: no unreferenced or pending snapshots.
  • Pre-commit workspace clippy, format, YAML, and hygiene hooks passed.
  • No files under sdks/typescript/bridge_typescript/ were staged or committed.

@antoniosarosi

Copy link
Copy Markdown
Contributor Author

Scope change per Antonio: this PR's revert half was split out to #4469, which merges on its own. This PR now carries ONLY the replacement value-side API (reflect.class.instance_from / reflect.class.Instance + the review fix round in d7c8724). It will be driven to green but must not be merged without Antonio's explicit sign-off. After #4469 lands, this branch will be rebased onto canary to drop the duplicated revert commit.

@antoniosarosi antoniosarosi changed the title feat: add runtime class instance reflection feat: reflect.class.instance_from value-side reflection (do not merge without sign-off) Aug 17, 2026
meefs pushed a commit to meefs/baml that referenced this pull request Aug 17, 2026
…L#4469)

## Summary

Byte-perfect revert of BoundaryML#4464's \`baml.Unknown\` companion: \`unknown\`
returns to having no member surface, and runtime field reads return to
the free-function spelling \`reflect.class.get_field<T>(obj, name)\`.

Split out of BoundaryML#4466 at Antonio's direction: this revert half merges now;
the replacement value-side API (\`reflect.class.instance_from\`) stays
on BoundaryML#4466 for explicit human sign-off.

## Verification

- \`git diff 15ea8c9 <this branch>\` is **empty** — the tree is
byte-identical to pre-BoundaryML#4464 canary (\`15ea8c91c\`), which was CI-green
as canary tip.
- The single commit is the same revert commit that passed the full
pinned five-package gate (3,750 tests) as the base of BoundaryML#4466's branch.

## Notes

- BoundaryML#4464's completion/describe/keyword-docs additions and snapshots are
all restored to their pre-BoundaryML#4464 state by construction (byte-identical
tree).
- The E0007 behavioral pin for \`unknown\` member access lands with
BoundaryML#4466's replacement surface, not here — this PR is a pure revert.

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

- **Breaking Changes**
- Removed the `Unknown` built-in type and its instance-based field
access method.
- Unknown values no longer provide instance-method completions for
dynamic field access.

- **Changes**
  - Runtime class field access now uses the reflective access API.
- Updated diagnostics and documentation to reflect the supported
field-access behavior.
- Removed obsolete completion and regression coverage for the retired
API.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
@antoniosarosi
antoniosarosi force-pushed the agent/reflect-class-instance branch from d7c8724 to 6b94f74 Compare August 17, 2026 17:59
@vercel
vercel Bot temporarily deployed to Preview – beps August 17, 2026 18:01 Inactive
@vercel
vercel Bot temporarily deployed to Preview – promptfiddle2 August 17, 2026 18:09 Inactive
@antoniosarosi
antoniosarosi force-pushed the agent/reflect-class-instance branch from 6b94f74 to 25d0afb Compare August 17, 2026 22:41
@antoniosarosi antoniosarosi changed the title feat: reflect.class.instance_from value-side reflection (do not merge without sign-off) fix: restore E0007 for member access on unknown receivers Aug 17, 2026
@antoniosarosi

Copy link
Copy Markdown
Contributor Author

Force-pushed the human-ratified stripped scope in 25d0afb1a.

  • Removed the superseded reflect.class.Instance / reflect.class.instance_from design in full.
  • Retained only the independent unknown-receiver E0007 fix and the separable pre-existing reflect.class.get_field diagnostic-factory hoist.
  • Rebased the two clean commits onto current canary a90dbec68.
  • Full pinned gate is green: 3,753/3,753 passed, 24 skipped; doctests clean; no pending or unreferenced snapshots.
  • Cleanup greps in the updated PR body are empty, and the final diff is limited to five intended files.

The previous do-not-merge design hold no longer applies. This ready PR is back in the normal coordinator review/queue pipeline.

@vercel
vercel Bot temporarily deployed to Preview – beps August 17, 2026 22:42 Inactive
@vercel
vercel Bot temporarily deployed to Preview – promptfiddle2 August 17, 2026 22:50 Inactive
@blacksmith-sh

This comment has been minimized.

@antoniosarosi
antoniosarosi added this pull request to the merge queue Aug 17, 2026
Merged via the queue into canary with commit 0797006 Aug 18, 2026
128 of 130 checks passed
@antoniosarosi
antoniosarosi deleted the agent/reflect-class-instance branch August 18, 2026 00:15

This branch was previously deployed

2 inactive deployments
Preview – promptfiddle2 — 25d0afb1 Deployed Aug 17, 2026 by vercel[bot]
Preview – beps — 25d0afb1 Deployed Aug 17, 2026 by vercel[bot]
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