fix(fmt): accept contextual keywords as identifiers in name positions - #4341
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe formatter now classifies parser-permitted keywords as word-like tokens. ChangesContextual-keyword formatter support
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (23)
baml_language/_plan/ai_agents/baml_src/ns_ai/session.baml-205-209 (1)
205-209: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftRe-folding history re-applies budget throws and blocks resume.
resume_sessionrebuilds policy state by passing every journal entry throughpolicy.update.ToolLoop.updatethrowsStepBudgetExceededwhenst.stepsreachesmax_steps, at lines 103-105 ofbaml_language/_plan/ai_agents/baml_src/ns_ai/policy.baml.WithBudget.updatethrowsCostBudgetExceededon the same replay path, at lines 193-195.A session that consumed most of its budget before the snapshot will therefore throw during restore instead of restoring. The throw escapes
resume_session, so the session cannot be recovered at all. Replay is a state-rebuild step, not live execution, and should not re-trigger budget enforcement.Add a replay mode to
update, or catch and discard budget errors in this loop while keeping the accumulated state.🤖 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 `@baml_language/_plan/ai_agents/baml_src/ns_ai/session.baml` around lines 205 - 209, Update resume_session’s history-rebuild loop so replaying entries does not abort when policy.update encounters StepBudgetExceeded or CostBudgetExceeded. Prefer a replay mode in policy.update that applies accumulated state without enforcing live budget limits, and use it from the loop over j.entries; preserve normal budget enforcement during live execution.baml_language/_plan/ai_agents/baml_src/ns_ai/openai.baml-92-92 (1)
92-92: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
self.callsrestarts on resume and produces duplicate tool call ids.
self.callsis client-local. It is passed toself.parser.parse(content, self.calls), andPlanTripParser.parsebuilds the call id as`t${call_seq}`inbaml_language/_plan/ai_agents/baml_src/shared/parser.bamlline 47. The call id is the join key betweenToolRequested,ToolCompleted, andToolFailedin the journal.A resumed session supplies a fresh client. The scenario at
baml_language/_plan/ai_agents/baml_src/scenarios/s02_sessions.bamlbuilds a new client for the resumed session, socallsrestarts at0while the restored journal already containst1,t2, and so on. New requests then reuse those ids.find_requestinbaml_language/_plan/ai_agents/baml_src/ns_ai/session.bamlline 152 keeps the last match, so the live loop still resolves, but the journal now holds two distinctToolRequestedentries under one id. Any later audit, replay, or retry over history cannot tell them apart.Derive the call sequence from the journal, for example from
last_seq(), instead of from a client-local counter.🤖 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 `@baml_language/_plan/ai_agents/baml_src/ns_ai/openai.baml` at line 92, Replace the client-local self.calls counter used by the parser in the OpenAI client flow with a sequence derived from the restored journal, such as last_seq(), so resumed sessions continue with a unique call id. Update the self.parser.parse invocation and remove the increment in the surrounding logic, preserving the existing ToolRequested, ToolCompleted, and ToolFailed correlation behavior.baml_language/_plan/ai_agents/baml_src/ns_ai/policy.baml-159-171 (1)
159-171: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
WithSteeringappends to the journal insideupdate, which breaks purity and corrupts resume.Lines 1-2 state that a policy is pure logic and that the runner performs all IO. Line 162 calls
j.append(...)directly. Three consequences follow.First, the appended
UserMessageevents never pass throughpolicy.update. The runner folds only the events it appends itself, at line 84-87 ofbaml_language/_plan/ai_agents/baml_src/ns_ai/session.baml. The inner policy therefore never observes these messages.Second, resume amplifies the journal.
resume_sessionat lines 207-209 ofsession.bamlre-folds every entry throughpolicy.updateto rebuild state. During that replay, each persistedUserMessagehits the arm at lines 155-158 and is buffered again. The next persistedAssistantMessagethen re-appends every buffered message toj. Each resume grows the journal with duplicateUserMessageentries.Third, that same replay loop iterates
j.entrieswhile this arm appends toj. Mutation during iteration is undefined or non-terminating depending on the iterator semantics.Return a command that instructs the runner to inject the queued messages, instead of writing to the journal from the policy.
🤖 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 `@baml_language/_plan/ai_agents/baml_src/ns_ai/policy.baml` around lines 159 - 171, Update WithSteering’s AssistantMessage handling and the runner flow so policy.update remains pure: return a command or equivalent effect describing the queued UserMessage injections, and have the runner append those messages and fold them through policy.update. Remove the direct j.append call from WithSteering, ensuring normal execution and resume replay never mutate the journal while it is being iterated.baml_language/_plan/ai_agents/baml_src/ns_ai/session.baml-36-39 (1)
36-39: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
interruptdoes not stop an in-flightrun.
interruptcancels the token and queues anInterruptedevent. Therunloop never readsself.cancel, andself.queued_eventsis drained only at lines 74-76, before the loop starts. An interrupt raised whilerunis executing is therefore observed only on the next call torun.The control lane is documented as separate from the data lane for this reason. As written it has the same latency as the data lane.
Test
self.cancelat the top of each loop iteration, and drainself.queued_eventsinside the loop.🤖 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 `@baml_language/_plan/ai_agents/baml_src/ns_ai/session.baml` around lines 36 - 39, Update the run loop to check self.cancel at the start of every iteration and terminate promptly when cancellation is requested. Move draining of self.queued_events into the loop so interrupts raised during an active run are processed immediately, while preserving the existing interrupt event behavior in interrupt.baml_language/_plan/ai_agents/baml_src/ns_ai/session.baml-100-106 (1)
100-106: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftTool failures are reported as successes, so
ToolFailedis never produced.Toolbox.callencodes every failure as a JSON string result rather than signaling failure, and theRunToolarm wraps whatever it returns inToolCompleted. The policy therefore receives a successful completion whose payload happens to contain{"error": ...}, and theToolFailedarm inbaml_language/_plan/ai_agents/baml_src/ns_ai/policy.bamlis unreachable from the runner. A policy cannot branch on tool failure, retry logic cannot trigger, and theToolFailedevent type is dead in practice.
baml_language/_plan/ai_agents/baml_src/ns_ai/session.baml#L100-L106: inspect the outcome ofself.toolbox.calland pushToolFailed { call_id, error }when the tool failed, instead of always pushingToolCompleted.baml_language/_plan/ai_agents/baml_src/ns_ai/toolbox.baml#L53-L66: return a discriminated outcome fromcall, or add a companion method that reports failure separately, so the runner can tell a failure from a successful result that contains the worderror.🤖 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 `@baml_language/_plan/ai_agents/baml_src/ns_ai/session.baml` around lines 100 - 106, The tool execution flow currently reports failures as successful completions. In baml_language/_plan/ai_agents/baml_src/ns_ai/toolbox.baml:53-66, update Toolbox.call or add a companion API to return a discriminated success/failure outcome; in baml_language/_plan/ai_agents/baml_src/ns_ai/session.baml:100-106, update the RunTool handler to push ToolFailed with the call ID and error for failures, and ToolCompleted only for successful results. Ensure failures are detected structurally rather than by inspecting JSON content.baml_language/_plan/ai_agents/baml_src/ns_ai/policy.baml-112-123 (1)
112-123: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winThe
ToolFailedarm incrementsstepsbut never enforcesmax_steps.The
ToolCompletedarm at lines 102-105 incrementsst.stepsand then throwsStepBudgetExceededwhen the budget is reached. TheToolFailedarm incrementsst.stepsat line 117 and omits that check. It then emitsCallModeloncepending_toolsdrains.A tool that fails on every attempt produces the cycle
ToolFailed→CallModel→ToolRequested→RunTool→ToolFailed.st.stepsgrows without bound and no arm stops it. This is the failure path that the step budget exists to bound.The runner in
baml_language/_plan/ai_agents/baml_src/ns_ai/session.bamlcurrently converts every tool outcome intoToolCompleted, becauseToolbox.callreturns an error string instead of throwing.ToolFailedtherefore reaches this arm only from a clientingest. The asymmetry is still a defect and will become reachable as soon as the runner emitsToolFailed.🐛 Proposed fix to apply the same budget check
let t: ToolFailed => { st.pending_tools = st.pending_tools.filter((id) -> { id != t.call_id }); st.steps += 1; + if (st.steps >= self.max_steps) { + throw StepBudgetExceeded { message: `step budget exhausted after ${st.steps} steps`, steps: st.steps }; + } if (st.pending_tools.length() == 0) { [CallModel { }] } else { [] } },🤖 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 `@baml_language/_plan/ai_agents/baml_src/ns_ai/policy.baml` around lines 112 - 123, Apply the same max_steps enforcement used by the ToolCompleted arm to the ToolFailed arm after incrementing st.steps: when the budget is reached, raise StepBudgetExceeded before emitting CallModel or continuing the retry cycle. Preserve the pending_tools cleanup and only emit CallModel when the budget remains available and all pending tools are drained.baml_language/_plan/ai_agents/baml_src/ns_ai/openai.baml-91-99 (1)
91-99: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winCheck
resp.statusbefore you parse the body.
ingestdecodesresp.body_jsonintoOaResponseand only reports an error when decoding fails. An HTTP error response from OpenAI is still valid JSON, for example{"error": {"message": "...", "type": "..."}}. If the decoder tolerates missingchoices, the decode succeeds,choices.at(0)yields null,contentbecomes"", andingestappends an emptyAssistantMessage. The session then treats a failed call as a normal empty reply. Thestatusfield is carried onProviderResponsefor this purpose but is never read.Test the status first, then decode.
🐛 Proposed fix to reject non-success responses
function ingest(self, resp: ProviderResponse) -> Event[] { self.calls += 1; let out: Event[] = []; + if (resp.status < 200 || resp.status >= 300) { + throw baml.errors.InvalidArgument { message: `provider error ${resp.status}: ${resp.body_json}` } + } let parsed = baml.json.from_string<OaResponse>(resp.body_json) catch_all (e) { _ => { throw baml.errors.InvalidArgument { message: `provider error ${resp.status}: ${resp.body_json}` } } };🤖 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 `@baml_language/_plan/ai_agents/baml_src/ns_ai/openai.baml` around lines 91 - 99, Update ingest to validate resp.status before calling baml.json.from_string<OaResponse> or appending any event. For non-success responses, immediately throw baml.errors.InvalidArgument with the provider status and response body; retain the existing parsing and content handling only for successful responses.baml_language/_plan/ai_agents/baml_src/ns_ai/openai.baml-77-89 (1)
77-89: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winBound the OpenAI provider request with a timeout.
baml.http.senddefaults totimeout: nulland documents no deadline. Add aroot.time.Durationbudget here so an unresponsive OpenAI request does not hang the session loop indefinitely.🤖 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 `@baml_language/_plan/ai_agents/baml_src/ns_ai/openai.baml` around lines 77 - 89, Add a finite root.time.Duration timeout to the HTTP request created in invoke, applying it to the baml.http.send call while preserving the existing request fields and ProviderResponse mapping.baml_language/_plan/pages/04_advanced/01_errors_and_retries.md-46-50 (1)
46-50: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winQualify the serialization guarantee.
Lines 46-50 state that every value serializes, but
baml_language/_plan/linear_issue_draft.mddocuments a runtime failure whenbaml.json.to_stringreceives a value typed asunknown. Fix the runtime issue before keeping this guarantee, or document theunknownlimitation and supported workaround.🤖 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 `@baml_language/_plan/pages/04_advanced/01_errors_and_retries.md` around lines 46 - 50, Qualify the serialization guarantee in the discussion of tool output validation and serialization to acknowledge that values typed as unknown can fail at runtime with baml.json.to_string. Either fix the unknown serialization path before retaining the unconditional guarantee, or document the limitation and specify the supported workaround, using the existing linear_issue_draft guidance.baml_language/_plan/pages/02_guides/01_agents.md-56-63 (1)
56-63: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftAlign tool execution documentation with the runner.
The supplied
Session.runimplementation processes one command at a time and callstoolbox.callsynchronously. It does not execute a concurrent tool batch. Implement parallel execution with defined ordering and error semantics, or change this text to describe sequential execution.🤖 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 `@baml_language/_plan/pages/02_guides/01_agents.md` around lines 56 - 63, Update the iteration description to state that tool calls execute sequentially, matching Session.run’s one-command-at-a-time toolbox.call behavior; remove the claim that calls run concurrently within a turn and preserve the journal result-appending flow.baml_language/_plan/pages/02_guides/03_steering.md-52-65 (1)
52-65: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winUse
send_eventfor custom events and prevent built-in event injection.The supplied API defines
send(msg: string)andsend_event(e: Event | X), but this example callssendwithPermissionGranted. TheEvent | Xsignature also permits callers to submit built-in events, which conflicts with the statement that callers cannot forge history. Update the example and narrow the API toX, or enforce rejection of built-in events.🤖 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 `@baml_language/_plan/pages/02_guides/03_steering.md` around lines 52 - 65, Update the guide’s custom-event example to call send_event with PermissionGranted instead of send. Align the documented API signature so send_event accepts only custom event type X, or explicitly validate and reject built-in Event values, preserving the stated restriction that callers cannot inject runner-produced history events.baml_language/_plan/pages/01_introduction/03_concepts.md-69-74 (1)
69-74: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winThe documentation uses a generic type-alias syntax that the parser does not support.
baml_language/_plan/reference_notes.mdrecords this as a known parser limitation.
baml_language/_plan/pages/01_introduction/03_concepts.md#L69-L74: replaceTurn<T>with an inline union or mark the example as future syntax.baml_language/_plan/pages/02_guides/02_sessions.md#L67-L74: replaceTurn<T>with an inline union or mark the example as future syntax.🤖 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 `@baml_language/_plan/pages/01_introduction/03_concepts.md` around lines 69 - 74, Replace the unsupported generic syntax Turn<T> with an inline union describing Done<T> or Replied in baml_language/_plan/pages/01_introduction/03_concepts.md lines 69-74 and baml_language/_plan/pages/02_guides/02_sessions.md lines 67-74; alternatively, explicitly mark the syntax as future syntax at both sites. Keep the documented Turn behavior unchanged.baml_language/_plan/pages/02_guides/02_sessions.md-34-43 (1)
34-43: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winThe queue lifecycle is documented inconsistently with the supplied runner.
sendstores data inqueued, andrun()controls when that data becomes journaled and visible to the model.
baml_language/_plan/pages/02_guides/02_sessions.md#L34-L43: state thatsendqueues input untilrun()processes it.baml_language/_plan/pages/02_guides/03_steering.md#L17-L29: add a queue-drain point before the next model call, or document that messages sent during a run apply on the nextrun()invocation.🤖 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 `@baml_language/_plan/pages/02_guides/02_sessions.md` around lines 34 - 43, Update baml_language/_plan/pages/02_guides/02_sessions.md lines 34-43 to document that send queues input in queued and run() drains it into the journal before model processing, rather than journalizing immediately. Update baml_language/_plan/pages/03_steering.md lines 17-29 to either specify a queue-drain point before the next model call or explicitly state that messages sent during a run are applied only on the next run() invocation.baml_language/_plan/pages/01_introduction/03_concepts.md-56-59 (1)
56-59: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftThe documented interruption guarantee exceeds the supplied runtime.
The runtime sets a cancellation token and queues an event, but the model, tool, and child-session paths do not consume the token.
baml_language/_plan/pages/01_introduction/03_concepts.md#L56-L59: describe interruption as cooperative and deferred until active I/O returns, or implement immediate cancellation.baml_language/_plan/pages/02_guides/03_steering.md#L42-L48: do not claim that in-flight tools and child sessions are cancelled until those paths propagate cancellation.baml_language/_plan/pages/05_appendix/02_design_principles.md#L33-L38: align the two-lane principle with the implemented cancellation guarantee.🤖 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 `@baml_language/_plan/pages/01_introduction/03_concepts.md` around lines 56 - 59, Align the interruption documentation with the runtime’s cooperative cancellation behavior: in baml_language/_plan/pages/01_introduction/03_concepts.md lines 56-59, describe interruption as deferred until active I/O returns; in baml_language/_plan/pages/02_guides/03_steering.md lines 42-48, remove claims that in-flight tools and child sessions are cancelled; and in baml_language/_plan/pages/05_appendix/02_design_principles.md lines 33-38, update the two-lane principle to reflect that cancellation is observed cooperatively rather than immediately.baml_language/_plan/pages/02_guides/02_sessions.md-113-124 (1)
113-124: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDo not send the first message twice.
On the no-snapshot branch,
request = msg. The laters.send(msg)adds the same value again as a user message. Send the message only on the resume branch, or use a distinct standing request.🤖 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 `@baml_language/_plan/pages/02_guides/02_sessions.md` around lines 113 - 124, The handle_turn function currently sends msg twice on the initial turn because the no-snapshot PlanTrip@session is created with request = msg before s.send(msg). Update the session setup so the initial branch does not duplicate the request, while retaining s.send(msg) for resumed sessions and preserving the existing run and response handling.baml_language/_plan/pages/02_guides/10_journal.md-41-45 (1)
41-45: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAdd
Compactedto the runtimeEventunion.The guide lists
Compactedas built-in and says the runner appends it. However,baml_language/_plan/ai_agents/baml_src/ns_ai/events.baml:1-85defines the class while itstype Eventunion stops atUsage. Without| Compacted, compaction events cannot pass throughJournal<Event>, policies, or resume as documented.Also applies to: 48-69, 151-157
🤖 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 `@baml_language/_plan/pages/02_guides/10_journal.md` around lines 41 - 45, Extend the runtime Event union in ns_ai/events.baml to include the existing Compacted class alongside Usage. Ensure all Journal<Event>, policy, and resume paths use this updated union without introducing a separate event type.baml_language/_plan/pages/02_guides/06_mcp.md-53-59 (1)
53-59: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winCorrect the resume semantics.
Replay should use recorded tool results. A resumed session should contact the MCP server only for uncommitted or unfinished tool work. Committed
ToolCompletedresults must not execute again. The current wording conflicts with the Tier 2 semantics inbaml_language/_plan/pages/02_guides/11_durability.md:21-33.🤖 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 `@baml_language/_plan/pages/02_guides/06_mcp.md` around lines 53 - 59, Update the “Replay caveat” section to state that resumed sessions contact the MCP server only for uncommitted or unfinished tool work, while committed ToolCompleted results are reused without execution. Align the wording with the Tier 2 semantics described in the durability guide and retain the existing name-stability caveat.baml_language/_plan/pages/02_guides/09_policies.md-184-186 (1)
184-186: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAlign the composition explanation with the budget event model.
WithBudgetupdatesspentonly forUsageevents at Lines 101-105. Holding aRunToolinWithApprovaldoes not delay aUsageevent. PlacingWithBudgetinsideWithApprovaltherefore does not defer the cost of held tool calls as stated. Document the actual ordering effect or emit usage for the resource being budgeted.🤖 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 `@baml_language/_plan/pages/02_guides/09_policies.md` around lines 184 - 186, Update the composition explanation to match the WithBudget event model: since spent changes only on Usage events and WithApproval does not delay those events for held RunTool calls, remove or correct the claim that placing WithBudget inside WithApproval defers their cost. Document the actual ordering effect, or revise the implementation to emit Usage for the budgeted resource.baml_language/_plan/pages/02_guides/09_policies.md-93-108 (1)
93-108: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftRebuild middleware state from journal events.
WithBudget.spentandWithApproval.heldare mutable fields on policy objects. The resume implementation inbaml_language/_plan/ai_agents/baml_src/ns_ai/session.baml:170-215rebuildsSessionStateby replaying events, but it does not rebuild these fields. A resumed session can reset its budget and drop an approval that was already requested. Store this state inSessionStateor derive it from journal events. This also violates the purity rule documented above.Also applies to: 121-123, 128-129, 134-145, 150-153
🤖 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 `@baml_language/_plan/pages/02_guides/09_policies.md` around lines 93 - 108, Refactor the WithBudget and WithApproval policy state so it is not stored in mutable policy fields such as spent and held. Persist the required values in SessionState or derive them from replayed Journal events, and update the policy methods to read and update that session-derived state. Ensure the resume flow in SessionState reconstruction preserves accumulated budget usage and outstanding approvals.baml_language/_plan/pages/02_guides/09_policies.md-130-133 (1)
130-133: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winKeep built-in event writes in the runner.
ToolFailedis a built-in event, but this policy appends it directly. The page states that built-in events are the runner's responsibility at Lines 68-71. Return a command or custom denial event for the runner to record instead.🤖 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 `@baml_language/_plan/pages/02_guides/09_policies.md` around lines 130 - 133, The PermissionDenied policy branch should not append the built-in ToolFailed event directly. Update the PermissionDenied handler to return a runner-consumable command or custom denial event, while preserving the call_id, denial information, and existing [CallModel {}] behavior as appropriate.baml_language/_plan/pages/02_guides/12_serving.md-54-70 (1)
54-70: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftDo not present
baml servereceipt guarantees as implemented.
baml servedocuments admission receipts, durable read/recovery paths, and re-read recovery.11_durability.mddocuments receipts and recovery as Tier 2 target behavior. Mark this protocol as Tier 2 or future unless the implementation and tests provide durable admission and receipt semantics.🤖 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 `@baml_language/_plan/pages/02_guides/12_serving.md` around lines 54 - 70, Update the baml serve documentation around the session endpoint descriptions to label receipt durability, recovery, and re-read semantics as Tier 2 or future behavior rather than implemented guarantees. Ensure the asynchronous submission and receipt text does not claim durable admission unless corresponding implementation and tests exist.baml_language/_plan/ai_agents/baml_src/scenarios/s09_errors.baml-16-23 (1)
16-23: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
output_schemadoes not describe thePlanOutcomeunion.The session is typed
new_session<PlanOutcome, never>, wherePlanOutcome = Itinerary | CannotPlan. The call passesoutput_schema = itinerary_schema(), which describesItineraryonly. The model is therefore never told that aCannotPlanresponse is permitted.Test 3 on lines 80-84 still passes, because
ScriptedClientreturns a cannedCannotPlanpayload and never renders the schema. Against a live client the refusal path could not be produced.This contradicts the comment on lines 10-11, which states that the session is typed over the union. Supply a schema derived from
PlanOutcome.🐛 Proposed fix
policy = root.ai.ToolLoop<never> { max_steps: 12, task_mode: true }, - output_schema = itinerary_schema(), + output_schema = plan_outcome_schema(), );Add the matching helper next to
itinerary_schema()inshared/types.baml, rendering theItinerary | CannotPlanunion.🤖 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 `@baml_language/_plan/ai_agents/baml_src/scenarios/s09_errors.baml` around lines 16 - 23, Update the session construction around new_session<PlanOutcome, never> to pass a schema representing the full PlanOutcome union instead of itinerary_schema(). Add and use a matching helper alongside itinerary_schema() in shared/types.baml that renders both Itinerary and CannotPlan.baml_language/_plan/ai_agents/baml_src/scenarios/s06_policies.baml-38-48 (1)
38-48: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winClear the released
call_idfromself.held.Neither the
PermissionGrantedarm nor thePermissionDeniedarm removes the entry fromself.held.Two consequences follow:
- A repeated
PermissionGrantedfor the samecall_idre-issues the sameRunTool. The gated tool then runs twice. For an approval gate this defeats the control.self.heldgrows for the lifetime of the session.This also disagrees with
s07_custom_events.bamllines 15-33.pending_permissionfolds the journal and treats any answeredcall_idas resolved, while the gate still holds a releasable command for it.Separately, the
PermissionDeniedarm callsj.append(...)directly. The file header describes policies as pure folds, andSession.runinns_ai/session.bamlis the component that appends events. Consider returning a command instead so the append stays in the runner.🔒️ Proposed fix to clear held entries
match (e) { let g: PermissionGranted => { if let held: root.ai.RunTool = self.held.get(g.call_id) { + let _ = self.held.remove(g.call_id); return [held]; } return []; }, let d: PermissionDenied => { + let _ = self.held.remove(d.call_id); j.append(root.ai.ToolFailed { call_id: d.call_id, error: "denied by operator" }); return [root.ai.CallModel { }]; },🤖 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 `@baml_language/_plan/ai_agents/baml_src/scenarios/s06_policies.baml` around lines 38 - 48, Update the policy fold’s PermissionGranted and PermissionDenied arms to remove the matching call_id from self.held before handling the result, ensuring repeated approvals cannot re-issue the tool and resolved entries do not accumulate. In the PermissionDenied arm, stop appending directly through j.append; return the appropriate event-producing command so Session.run remains responsible for journal updates.
🟡 Minor comments (14)
baml_language/_plan/ai_agents/baml_src/ns_ai/session.baml-125-128 (1)
125-128: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winA malformed final value throws out of
run.
baml.json.from_string<T>(c.result_json)decodes model-controlled text. The call has nocatch_all. If the model emits afinalwhose value does not matchT, the decode error propagates out ofrunand past theDone/Repliedmatch inplan_trip.Convert the decode failure into a typed outcome, so a caller can distinguish a schema violation from a transport error.
🤖 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 `@baml_language/_plan/ai_agents/baml_src/ns_ai/session.baml` around lines 125 - 128, Update the FinishTurn handling in run to catch failures from baml.json.from_string<T>(c.result_json) and convert them into a typed schema-violation outcome instead of allowing them to escape. Ensure plan_trip can distinguish this decode failure from transport errors while preserving the successful Done result path.baml_language/_plan/ai_agents/baml_src/ns_ai/session.baml-49-49 (1)
49-49: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
snapshotdrops queued messages and queued events.
snapshotserializes only the journal.self.queuedandself.queued_eventsare not included. A caller that runssend("...")and thensnapshot()without an interveningrun()loses that message. The restored session has no record of it.The scenario in
baml_language/_plan/ai_agents/baml_src/scenarios/s02_sessions.bamlcallsrun()beforesnapshot(), which drains both queues, so the scenario does not expose the loss.Either serialize the queues with the journal, or state the drain precondition in the comment.
🤖 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 `@baml_language/_plan/ai_agents/baml_src/ns_ai/session.baml` at line 49, Update Session.snapshot to preserve pending state by serializing self.queued and self.queued_events along with self.journal, and ensure restoration reconstructs both queues so messages and events queued before snapshot are not lost. If queue serialization is not supported, document and enforce that callers must drain both queues via run() before snapshot.baml_language/_plan/ai_agents/baml_src/ns_ai/toolbox.baml-57-62 (1)
57-62: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winEscape the error text before you embed it in JSON.
The success path uses
baml.json.stringify. The two error paths build JSON by string interpolation.bad.to_string()ande.to_string()are arbitrary error text. A reflect argument error typically names the offending parameter and often quotes it. A double quote, a backslash, or a newline in that text produces a string that is not valid JSON.The tool result goes back to the model as a JSON document. Malformed JSON here degrades the loop that this error path exists to support.
🐛 Proposed fix to serialize the error message
baml.json.stringify(baml.json.to_json(reflect.call_any(t.f, args))) catch_all (e) { let bad: reflect.InvalidArgumentError => { - `{"error": "bad arguments: ${bad.to_string()}"}` + baml.json.to_string({ "error": `bad arguments: ${bad.to_string()}` }) }, - _ => `{"error": "${e.to_string()}"}`, + _ => baml.json.to_string({ "error": e.to_string() }), } } else { - `{"error": "no such tool: ${name}"}` + baml.json.to_string({ "error": `no such tool: ${name}` }) }🤖 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 `@baml_language/_plan/ai_agents/baml_src/ns_ai/toolbox.baml` around lines 57 - 62, Update the error branches around reflect.call_any in toolbox.baml to serialize error messages with baml.json.stringify rather than interpolating bad.to_string() or e.to_string() directly into JSON. Preserve the existing error response shape and ensure both InvalidArgumentError and fallback errors produce valid JSON when their text contains quotes, backslashes, or newlines.baml_language/_plan/ai_agents/readme.md-7-7 (1)
7-7: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse language-tagged fences in both directory-tree examples.
Both fences trigger MD040. Change each opening fence to
```textor another suitable language.
baml_language/_plan/ai_agents/readme.md#L7-L7: add a language to the directory-tree fence.baml_language/_plan/outline.md#L6-L6: add a language to the directory-tree fence.🤖 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 `@baml_language/_plan/ai_agents/readme.md` at line 7, Update the directory-tree code-fence openings in baml_language/_plan/ai_agents/readme.md lines 7-7 and baml_language/_plan/outline.md lines 6-6 to include a language tag such as text; leave the directory-tree contents unchanged.Source: Linters/SAST tools
baml_language/_plan/pages/03_examples/01_claude_code.md-125-126 (1)
125-126: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winIgnore permission responses when no request is pending.
pending_permission(s)returns""when no unanswered request exists. Lines 125-126 still sendPermissionGrantedorPermissionDeniedwith that empty identifier. Read the value once and send the event only when the identifier is non-empty.🤖 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 `@baml_language/_plan/pages/03_examples/01_claude_code.md` around lines 125 - 126, Update the permission response handling for the “y” and “n” branches to read pending_permission(s) once, then send PermissionGranted or PermissionDenied only when the returned call_id is non-empty; ignore the response when no request is pending.baml_language/_plan/pages/04_advanced/02_evals.md-101-102 (1)
101-102: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winHandle an empty evaluation suite.
If
casesis empty,scores.length()is zero and Line 102 divides by zero. Reject an empty suite or return a defined result before calculating the average.🤖 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 `@baml_language/_plan/pages/04_advanced/02_evals.md` around lines 101 - 102, Update the evaluation score calculation around scores.reduce to handle an empty cases suite before dividing by scores.length(); either reject the empty suite or return the defined empty-suite result, while preserving the existing average calculation for non-empty suites.baml_language/_plan/pages/01_introduction/03_concepts.md-10-17 (1)
10-17: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd a language to the tree code fence.
markdownlintreports MD040 for this fence. Usetextfor this diagram.🤖 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 `@baml_language/_plan/pages/01_introduction/03_concepts.md` around lines 10 - 17, Update the fenced diagram in the concepts documentation to declare the text language identifier, changing the opening fence to use text while preserving the Session tree content unchanged.Source: Linters/SAST tools
baml_language/_plan/pages/02_guides/10_journal.md-16-16 (1)
16-16: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd a language to the sequence fence.
Change the fence on Line 16 to
textor another suitable language.🤖 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 `@baml_language/_plan/pages/02_guides/10_journal.md` at line 16, Update the fenced code block at the indicated location in the journal documentation to specify text syntax (or another appropriate language) instead of leaving the sequence fence unlabeled.Source: Linters/SAST tools
baml_language/_plan/pages/02_guides/12_serving.md-59-59 (1)
59-59: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd a language to the endpoint fence.
Change the fence on Line 59 to
textor another suitable language.🤖 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 `@baml_language/_plan/pages/02_guides/12_serving.md` at line 59, Update the code fence around the serving endpoint example in the page content to specify text or another appropriate language identifier instead of leaving the fence untagged.Source: Linters/SAST tools
baml_language/_plan/pages/02_guides/09_policies.md-35-35 (1)
35-35: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse standard wording.
Replace “what a policy wants done” with “what a policy wants to be done”.
🤖 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 `@baml_language/_plan/pages/02_guides/09_policies.md` at line 35, Update the sentence beginning “A command is” to replace “what a policy wants done” with “what a policy wants to be done,” preserving the surrounding wording.Source: Linters/SAST tools
baml_language/_plan/pages/02_guides/05_tools.md-8-14 (1)
8-14: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winEncode
originanddestbefore callingbaml.http.fetch.When tool arguments are interpolated into the URL, values containing
&,?,#,/, or%can change the query parameters or fragment sent tohttps://api.flights.dev/q. This can return different flight results than intended. Build the query with URL-encoded values orURLSearchParams.🤖 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 `@baml_language/_plan/pages/02_guides/05_tools.md` around lines 8 - 14, Update search_flights to URL-encode origin and dest before interpolating them into the baml.http.fetch URL, using the available URL encoding or URLSearchParams mechanism while preserving the existing endpoint and response parsing.baml_language/_plan/ai_agents/baml_src/scenarios/s07_custom_events.baml-24-32 (1)
24-32: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
pending_permissionreturns the newest unanswered request, not the oldest.The second loop keeps overwriting
pending, so the last unansweredPermissionRequestedwins. Ift1andt2are both unanswered, the function reportst2andt1is never surfaced. An approval queue normally resolves the oldest request first.Test 3 does not detect this, because it answers
t1before appendingt2.If last-wins is intended, state that in the comment on line 13.
🐛 Proposed fix for first-unanswered semantics
let pending = ""; for (let en in j.entries) { if let p: PermissionRequested = en.event { - if (!answered.includes(p.call_id)) { + if (!answered.includes(p.call_id) && pending == "") { pending = p.call_id; } } } pending🤖 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 `@baml_language/_plan/ai_agents/baml_src/scenarios/s07_custom_events.baml` around lines 24 - 32, Update the pending permission selection logic in the loop over j.entries so it preserves the first unanswered PermissionRequested call_id instead of overwriting it with later requests. Only assign pending when it is still empty and the request is not present in answered, keeping the existing return value and event filtering.baml_language/_plan/ai_agents/baml_src/scenarios/s08_subagents.baml-1-5 (1)
1-5: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRepair the file header.
Two problems exist here.
- The header sentence is split across three
//#comments separated by blank lines. Every other scenario file uses one contiguous//block.- The header omits the
Mirrors pages/02_guides/08_subagents.md.cross-reference. Filess01throughs07ands09all carry that reference, and the guide page exists.📝 Proposed header
-//# an agent wrapped as a tool (closure captures the child's client — in the - -//# real feature the child session and ChildSpawned/ChildFinished events are - -//# wired by the runtime) +// Scenario 08 — Subagents: an agent wrapped as a tool. The closure captures +// the child's client. In the real feature the child session and the +// ChildSpawned/ChildFinished events are wired by the runtime. +// Mirrors pages/02_guides/08_subagents.md.🤖 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 `@baml_language/_plan/ai_agents/baml_src/scenarios/s08_subagents.baml` around lines 1 - 5, Repair the header in s08_subagents.baml by using one contiguous `//` comment block without blank lines between the sentence fragments, and add the cross-reference stating that it mirrors pages/02_guides/08_subagents.md. Match the header format used by the neighboring scenario files.baml_language/_plan/ai_agents/baml_src/scenarios/s09_errors.baml-12-12 (1)
12-12: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winTerminate the type alias with a semicolon.
s06_policies.bamllines 20 and 22 ands07_custom_events.bamlline 9 all end their type aliases with;. This alias does not.📝 Proposed fix
-type PlanOutcome = Itinerary | CannotPlan +type PlanOutcome = Itinerary | CannotPlan;🤖 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 `@baml_language/_plan/ai_agents/baml_src/scenarios/s09_errors.baml` at line 12, Terminate the PlanOutcome type alias with a semicolon, matching the existing alias syntax used in the related scenario files.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 9160dbdf-595a-4914-895c-42b83024f0eb
📒 Files selected for processing (51)
baml_language/_plan/ai_agents/baml.tomlbaml_language/_plan/ai_agents/baml_src/ns_ai/client.bamlbaml_language/_plan/ai_agents/baml_src/ns_ai/errors.bamlbaml_language/_plan/ai_agents/baml_src/ns_ai/events.bamlbaml_language/_plan/ai_agents/baml_src/ns_ai/openai.bamlbaml_language/_plan/ai_agents/baml_src/ns_ai/policy.bamlbaml_language/_plan/ai_agents/baml_src/ns_ai/session.bamlbaml_language/_plan/ai_agents/baml_src/ns_ai/toolbox.bamlbaml_language/_plan/ai_agents/baml_src/scenarios/s01_agents.bamlbaml_language/_plan/ai_agents/baml_src/scenarios/s02_sessions.bamlbaml_language/_plan/ai_agents/baml_src/scenarios/s03_steering.bamlbaml_language/_plan/ai_agents/baml_src/scenarios/s04_models.bamlbaml_language/_plan/ai_agents/baml_src/scenarios/s05_tools.bamlbaml_language/_plan/ai_agents/baml_src/scenarios/s06_policies.bamlbaml_language/_plan/ai_agents/baml_src/scenarios/s07_custom_events.bamlbaml_language/_plan/ai_agents/baml_src/scenarios/s08_subagents.bamlbaml_language/_plan/ai_agents/baml_src/scenarios/s09_errors.bamlbaml_language/_plan/ai_agents/baml_src/shared/parser.bamlbaml_language/_plan/ai_agents/baml_src/shared/plan_trip.bamlbaml_language/_plan/ai_agents/baml_src/shared/tools.bamlbaml_language/_plan/ai_agents/baml_src/shared/types.bamlbaml_language/_plan/ai_agents/readme.mdbaml_language/_plan/linear_issue_draft.mdbaml_language/_plan/outline.mdbaml_language/_plan/pages/01_introduction/01_getting_started.mdbaml_language/_plan/pages/01_introduction/02_why.mdbaml_language/_plan/pages/01_introduction/03_concepts.mdbaml_language/_plan/pages/02_guides/01_agents.mdbaml_language/_plan/pages/02_guides/02_sessions.mdbaml_language/_plan/pages/02_guides/03_steering.mdbaml_language/_plan/pages/02_guides/04_models.mdbaml_language/_plan/pages/02_guides/05_tools.mdbaml_language/_plan/pages/02_guides/06_mcp.mdbaml_language/_plan/pages/02_guides/07_skills.mdbaml_language/_plan/pages/02_guides/08_subagents.mdbaml_language/_plan/pages/02_guides/09_policies.mdbaml_language/_plan/pages/02_guides/10_journal.mdbaml_language/_plan/pages/02_guides/11_durability.mdbaml_language/_plan/pages/02_guides/12_serving.mdbaml_language/_plan/pages/03_examples/01_claude_code.mdbaml_language/_plan/pages/03_examples/02_background_jobs.mdbaml_language/_plan/pages/04_advanced/01_errors_and_retries.mdbaml_language/_plan/pages/04_advanced/02_evals.mdbaml_language/_plan/pages/04_advanced/03_observability.mdbaml_language/_plan/pages/05_appendix/01_comparisons.mdbaml_language/_plan/pages/05_appendix/02_design_principles.mdbaml_language/_plan/readme.mdbaml_language/_plan/reference_notes.mdbaml_language/crates/baml_fmt/src/ast/expressions.rsbaml_language/crates/baml_fmt/src/ast/tokens.rsbaml_language/crates/baml_fmt/src/lib.rs
9f567bd to
ffbbe87
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
⏭️ 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):
|
Binary size checks passed✅ 7 passed
Generated by |
The lexer emits dedicated keyword kinds for words like `client` (KW_CLIENT), and the parser deliberately keeps them valid as class field, parameter, method, member-access, and object-literal-key names (BEP-049 §10 `ctx.client`). The formatter's strong AST rejected those CSTs with "Expected token/node of kind WORD, but found KW_CLIENT", so `baml fmt` died on any file using `client` as a field or parameter name even though the checker and runtime accept it. Widen `Word::from_cst` (and the object-field key parser) to accept the same word-like keyword set the parser allows in name positions, via a shared `is_word_like` predicate. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ffbbe87 to
0f4a865
Compare
1 participant
Summary
Canary already accepts
client(KW_CLIENT) as an identifier in the formatter (Word::from_cstandObjectFieldKey::from_cstspecial-case it). This PR generalizes that special case to the full set of contextual keywords the parser allows in name positions, and centralizes it in one predicate.Problem
The parser accepts more than
clientas names:at_member_name/parse_functioninbaml_compiler_parseralso keepimplements,implement,extends,requires, andinterfacevalid as method and member-access names (e.g.dog_t.implements(animal_t)on the reflectiontypevalue), andspawn/awaitare valid path segments. The formatter still crashed on those with:Fix
is_word_likepredicate inbaml_fmt::ast::tokensmirroring the parser's name-position keyword set (WORD,KW_CLIENT,KW_IMPLEMENTS,KW_IMPLEMENT,KW_EXTENDS,KW_REQUIRES,KW_INTERFACE,KW_SPAWN,KW_AWAIT).Word::from_cstandObjectFieldKey::from_cstuse it instead of theirKW_CLIENT-only special cases.Tests
contextual_keyword_name_tests:clientas class field name, parameter name, member-access name, object-literal key, andimplementsas a method name — each asserting round-trip + idempotency. Complements the existingcontextual_keyword_identifier_tests.cargo test -p baml_fmt: 107 passed.target/debug/baml-cli fmtformats a real file usingclient: Client?as a class field cleanly and idempotently, preserving all identifiers.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests
Note
Low Risk
Formatter-only CST→AST acceptance change with targeted regression tests; no runtime, auth, or data-path impact.
Overview
Fixes baml_fmt crashing when formatting valid BAML that uses contextual keywords as names (e.g.
clientfields,dog_t.implements(...)), which the parser already accepts but the strong AST rejected with “expectedWORD, foundKW_*”.Introduces shared
is_word_likeinast/tokens, aligned with the parser’s name-position keyword set (client,implements,implement,extends,requires,interface,spawn,await, plus ordinaryWORD).Word::from_cstandObjectFieldKey::from_cstnow use that predicate instead of aKW_CLIENT-only special case.Adds
contextual_keyword_name_testsfor round-trip/idempotent formatting ofclientas field, parameter, member access, object key, andimplementsas a method name.Reviewed by Cursor Bugbot for commit 0f4a865. Bugbot is set up for automated code reviews on this repo. Configure here.