Skip to content

Record canonical wrapper commands in BCS span context - #5624

Closed
sxlijin wants to merge 1 commit into
canaryfrom
sxlijin/baml-bcs-command-context
Closed

sxlijin wants to merge 1 commit into
canaryfrom
sxlijin/baml-bcs-command-context

Conversation

@sxlijin

@sxlijin sxlijin commented Nov 12, 2026 •

Copy link
Copy Markdown
Contributor

Attach the wrapper’s existing canonical PostHog command label to the foreground user.dispatch BCS span before parsing, and inherit it into descendants. This works with PostHog disabled and composes with existing value-capture options; detached telemetry/replacement helpers clear the foreground label.

Process launch context stays unchanged. Document the process_id join from the single dispatch span to process outcomes, including delegated exits and the ambiguous canonical baml label.

Validation: packed-wrapper local-file recording/query regression (22 foreground cases, both helpers, disabled recording/PostHog, inheritance, bounded labels, and success/error outcomes); existing explicit-boundary recording regression; 4 runtime context tests; 33 PostHog regressions; shared wrapper CI profile on BAML and Rust (53 tests each); Clippy and formatting/Markdown checks. Standard CI passed, including Windows, and CodeRabbit approved the exact head. Two non-required Kiln shadow jobs still fail after one retry because their runners lack prek and cargo insta; their workflow concludes successfully.

@vercel

vercel Bot commented Nov 12, 2026 •

Copy link
Copy Markdown

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

Project Deployment Actions Updated
developer-docs Ready Ready Preview Nov 12, 2026 8:44pm UTC

Request Review

@coderabbitai

coderabbitai Bot commented Nov 12, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

The wrapper now adds a canonical command value to foreground user.dispatch trace context, except for internal helper invocations. An end-to-end test checks command metadata and recorded outcomes. The telemetry documentation explains command labels, joins, recording behavior, and query examples.

Changes: Wrapper telemetry

Layer / File(s) Summary
Attach and verify command metadata
baml_language/wrapper/baml_src/entry.baml, baml_language/crates/baml_cli/tests/wrapper_command_context_e2e.rs
main supplies dispatch with a command value from WrapperTelemetry.command(args), or null for internal helpers. The end-to-end test checks dispatch spans, process outcomes, inherited context, exclusions, and recording-off behavior.
Document command metadata
TELEMETRY.md, baml_language/wrapper/README.md
The documentation describes canonical command labels, span inheritance, process joins, recording requirements, and local and cloud query examples.

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

Change: Feature

Possibly related PRs

  • BoundaryML/baml#5621: Changes capture behavior for the same dispatch trace contract and updates related telemetry documentation.
  • BoundaryML/baml#5422: Introduces the wrapper implementation and telemetry contracts used by this change.
  • BoundaryML/baml#5583: Changes command normalization and names documented and tested here.

Suggested reviewers: hellovai, aaronvg, 2kai2kai2

Merge Risk: Minimal · up to 43d7b

Foreground wrapper invocations now record a bounded command label on the dispatch span, and helper invocations do not. No concrete merge-blocking risk remains. Process launch metadata is unchanged, and the new behavior is covered by an end-to-end test and documented.

Security architecture risk: Minimal · up to 43d7b

The new field contains fixed-vocabulary command labels rather than raw arguments. The change uses existing recording controls and does not introduce credentials, command authority, or a new delivery destination. No material security risk introduced or worsened by this PR was identified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The incremental disclosure is a canonical command category on existing enabled wrapper recordings and descendant spans. PostHog opt-out does not suppress this separate recording channel; the documented control is BAML_TELEMETRY=off. The new test asserts recording-off behavior and absence of PostHog collection and delivery files, but was not executed during this review.

Trust Boundaries and Controls

  • observed — Attacker-controlled argv reaches the new metadata field only through the existing canonicalizer. Every return is a fixed label or a label containing an allowlisted command or toolchain subcommand. Unknown tokens fall back to baml or a recognized parent; arbitrary argument strings are not returned into this field.

Resilience and Maintainability Implications

  • observed — The context mechanism clones tracing options and applies patches to immutable context versions rather than modifying ancestor metadata. Null metadata becomes a removal marker, and invocation entry derives context from the current frame. This provides structural isolation for repeated calls and helper clearing without a shared command variable or compensating rollback operation.
Pre-merge checks: Passed 4

Check name Status Explanation
Title check Passed The title names the main change: the wrapper's canonical command label is now recorded on the BCS span context. The raw summary shows the foreground user.dispatch span gets the canonical command, desc…
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.
Full details: Title check

Explanation

The title names the main change: the wrapper's canonical command label is now recorded on the BCS span context. The raw summary shows the foreground user.dispatch span gets the canonical command, descendants inherit it, and helpers clear it. The title is specific and matches the objectives, and it does not describe anything absent from the changeset.

Finishing Touches

Generate unit tests (beta)

  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

A rabbit checks each label in the trace,
And follows dispatch at a steady pace.
The spans record the command in view,
While helpers keep their context true.
Then quiet hops complete the review.

Recent review info

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 16feaa41-5791-4782-948d-aee445557dca
Commits

Reviewing files that changed from the base of the PR and between cbbb3da and 43d7b61.

Files selected for processing: 4

  • TELEMETRY.md
  • baml_language/crates/baml_cli/tests/wrapper_command_context_e2e.rs
  • baml_language/wrapper/README.md
  • baml_language/wrapper/baml_src/entry.baml

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 0 remain after this review.

@sxlijin

sxlijin commented Nov 12, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by the combined implementation in #5621, as requested by the user. That PR now includes canonical command context plus explicit operational error capture, omits full input and argument-bearing result snapshots, and retains only child exit code/signal. Both local recording/query regressions pass; CI/review are being refreshed on the combined head. This branch and worktree are retained; nothing was merged into canary.

@sxlijin sxlijin closed this Nov 12, 2026

This branch was successfully deployed

1 active deployment
Preview – developer-docs — 43d7b61c Deployed Nov 12, 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