Add LlmCompletionEvent typed event variant#137
Merged
Conversation
Define LlmCompletionEvent as a new typed variant on the observer event union per proposal 0049. Fields carry the LLM call's identity, scoping, and outcome data as a structured record; observers filter via isinstance() rather than the impl-current sentinel-namespace string match. The Usage field reuses the existing openarmature.llm.response.Usage class via a TYPE_CHECKING import and string forward reference, sidestepping the runtime cycle through openarmature.llm. The OpenAI provider emits the typed event alongside the existing sentinel NodeEvent pair during the dual-emit transition window per spec section 5.5.7's SHOULD-emit-both framing. Failure paths emit only the sentinel pair; the typed event is completion-only per the proposal's v1 scope. Wall-clock latency is measured at the adapter boundary with time.perf_counter(). The opt-in caller_invocation_metadata field is wired via a new populate_caller_metadata constructor kwarg on OpenAIProvider per spec Q2 ack (default False; snapshot via dict() when True). The OTel and Langfuse observers gain an isinstance early-return branch for LlmCompletionEvent to stay Protocol-compatible with the widened ObserverEvent union. Migration to type-discrimination filtering of the typed event lands in a subsequent PR; this PR keeps observer behavior unchanged (they continue driving spans / generations off the sentinel NodeEvent pair). Foundation for the OTel and Langfuse observer migrations to type-discrimination filtering in subsequent PRs of the v0.13.0 LLM hardening cycle.
There was a problem hiding this comment.
Pull request overview
Adds a new typed observer event variant (LlmCompletionEvent) to represent successful LLM completions (proposal 0049), and updates the OpenAI provider and observers to support a v0.13.0 dual-emit transition (legacy sentinel NodeEvent pair + new typed event).
Changes:
- Introduces
LlmCompletionEventand widens theObserverEventunion to include it. - Updates
OpenAIProvider.complete()to dual-emit (success: sentinel pair + typed event; failure: sentinel pair only) and records adapter-boundary latency. - Updates OTel and Langfuse observers to remain Protocol-compatible by explicitly ignoring
LlmCompletionEventfor now; adds unit tests covering dual-emit behavior and field sourcing.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/test_llm_provider.py | Adds tests asserting dual-emission, typed event field sourcing, opt-in caller metadata, ordering, and ContextVar scoping. |
| src/openarmature/observability/otel/observer.py | Extends accepted event union and ignores LlmCompletionEvent during the transition window. |
| src/openarmature/observability/langfuse/observer.py | Extends accepted event union and ignores LlmCompletionEvent during the transition window. |
| src/openarmature/observability/correlation.py | Widens the active dispatch callable type to accept LlmCompletionEvent. |
| src/openarmature/llm/providers/openai.py | Dual-emits the new typed event on success and measures adapter-boundary latency; adds provider opt-in knob for typed metadata snapshotting. |
| src/openarmature/graph/observer.py | Widens ObserverEvent to include LlmCompletionEvent and documents its role. |
| src/openarmature/graph/events.py | Defines the new LlmCompletionEvent dataclass (with a Usage forward reference to avoid import cycles). |
| src/openarmature/graph/init.py | Re-exports LlmCompletionEvent from the public graph API. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+558
to
+562
| if self._populate_caller_metadata: | ||
| # Snapshot via dict() so downstream consumers see a stable | ||
| # frozen view; if a node body mutates metadata after the | ||
| # snapshot, the event still carries the at-emission view. | ||
| caller_metadata = dict(current_invocation_metadata()) |
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
LlmCompletionEventas a new typed variant on the observer event union per proposal 0049. Carries the LLM call's identity, scoping, and outcome data as a structured record; observers filter viaisinstance()rather than the impl-current sentinel-namespace string match. Reuses the existingopenarmature.llm.response.Usageshape via a TYPE_CHECKING import + string forward reference to sidestep the runtime cycle throughopenarmature.llm.NodeEventpair continues firing for backwards-compat observability; the new typed event fires on the success path only (completion-only per spec v1 scope). Failure paths emit only the sentinel pair. Wall-clock latency measured at the adapter boundary withtime.perf_counter().populate_caller_metadata: bool = Falseconstructor kwarg onOpenAIProviderwires the opt-in caller-metadata snapshot per spec Q2 ack (option c — provider-side knob). Default off matches the spec's "default absent" semantics.isinstance(event, LlmCompletionEvent)early-return to stay Protocol-compatible with the widenedObserverEventunion. Migration to actual type-discrimination consumption lands in the next PR; this PR keeps observer behavior unchanged.Test plan
uv run pytest tests/ -q(1142 passing; 8 new tests intest_llm_provider.pycovering dual-emit on success, no typed event on failure, outcome-field sourcing, opt-in default + on, request_id None case, arrival-order assertion, node-identity ContextVar sourcing)uv run pyright src examples tests(clean over the whole tree)uv run ruff check src examples tests(clean)src/openarmature/graph/events.py(the typed event field set + TYPE_CHECKING pattern for theUsageforward reference)src/openarmature/llm/providers/openai.py(the dispatch site around line 522 + the_build_llm_completion_eventhelper)Notes
LlmEventPayloadsentinel pattern stays in place through v0.13.0. v0.15.0 will drop the sentinel emission per spec Q3 ack (CHANGELOG callout to pin in the release-prep PR).