Activate proposal 0049 conformance fixtures 050-056#139
Merged
chris-colinsky merged 2 commits intoJun 8, 2026
Conversation
Adds harness support for the typed_event_collector directive introduced by proposal 0049's seven conformance fixtures (050-056). The directive declares one or more typed event collectors with optional filter_event_type and include_caller_metadata flags; the parser aggregates the latter into the provider's populate_caller_metadata kwarg. Adds eight assertion shape handlers consumed by the fixtures: contains_event, contains_exactly_one_event_of_type, contains_event_of_type, contains_exactly_n_events_of_type, does_not_contain_event_of_type, captured_event_field_values_cover, every_captured_event_has, relative_order_of_events_matching. The captured_event_field_values_cover shape uses set equality (matching the spec text's "cover" semantic) rather than sorted-by-str (which silently masks None-vs-primitive ordering ambiguity). An unknown-key guard rejects fixture typos before any assertion runs. Three graph builders cover the fixture topologies: a simple entry-ask-END shape (050, 051, 052, 053, 056), a fan-out builder (054), and a parallel-branches builder (055). The failure-path runner (053) sources the calling-node NodeEvent from an unfiltered named collector when one exists; otherwise attaches an AllEventsCollector specifically for that assertion. The cause-chain walk handles the NodeException wrapper the engine layers over the underlying ProviderUnavailable. Adds tests/conformance/test_typed_event_harness.py with 31 unit tests covering the harness helpers' edge cases: typo detection in assertion keys, set-equality semantics, namespace tuple-vs-list normalization, usage Mapping-vs-Usage comparison, cause-chain walking, populate_caller_metadata aggregation, and the mock_model resolution. These would otherwise only surface when a future fixture trips them. Flips conformance.toml's [proposals."0049"] from not-yet to implemented since 0.13.0. The leading-comment block notes that the OTel + Langfuse observers continue driving their span / generation surface off the sentinel NodeEvent pair during the dual-emit transition window; type-discrimination migration of those observers waits on the request-side-fields extension currently being drafted.
There was a problem hiding this comment.
Pull request overview
This PR activates proposal 0049 conformance fixtures (050–056) by extending the conformance harness in tests/conformance/test_observability.py to support typed_event_collector, and adds a dedicated unit test suite to validate the harness helpers’ edge cases. It also updates conformance.toml to mark proposal 0049 as implemented since 0.13.0.
Changes:
- Add typed-event conformance harness support and wire up fixture runners for 050–056.
- Add
tests/conformance/test_typed_event_harness.pyunit tests for typed-event harness helper behavior. - Mark proposal
0049as implemented inconformance.toml.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| tests/conformance/test_typed_event_harness.py | New unit tests covering typed-event harness helper edge cases. |
| tests/conformance/test_observability.py | Adds typed-event harness, assertion shapes, and runners for fixtures 050–056. |
| conformance.toml | Marks proposal 0049 as implemented (since 0.13.0). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Two distinct issues across five review threads:
1. Three sites used a truthy check on caller_metadata (if metadata:)
that collapsed an explicit empty mapping (caller_metadata: {}) to
the "no metadata key" case and dropped the metadata= kwarg from
graph.invoke(). Switched all three sites
(_invoke_typed_fixture, _run_typed_event_fanout_case,
_run_typed_event_branches_case) to is-not-None checks so empty
mappings flow through to the engine.
2. Two harness helpers (_event_fields_match and the
every_captured_event_has branch in _assert_observer_expectations)
used getattr(event, name, None), conflating a missing attribute
with an attribute present and set to None. A fixture-side
field-name typo (node_nam vs node_name) would silently match
against any None expectation. Added hasattr() preambles that
raise AssertionError naming the missing field, since upstream
type filtering guarantees attribute presence on the captured
set.
Two new harness unit tests pin the missing-attribute contract: a
typo'd field on _event_fields_match raises a clear error, and the
every_captured_event_has branch does the same.
chris-colinsky
added a commit
that referenced
this pull request
Jun 8, 2026
Address PR review feedback: _run_llm_cache_fixture_case constructed an OpenAIProvider but never awaited aclose(), leaking the underlying httpx.AsyncClient connection pool. Fixture 005 and 038 runners elsewhere in this file close the provider in a finally block; the new cache-fixture runner now follows the same convention. Extending the fix to the typed-event runners introduced in PR #139 (_build_simple_llm_graph, _run_typed_event_fanout_case, _run_typed_event_branches_case) — same bug class, same file, CoPilot missed them in the original review. _build_simple_llm_graph now returns (graph, state_cls, provider) so the caller can close the provider in a finally; the fan-out and parallel-branches runners close their inline-constructed providers symmetrically. No behavior change for fixture pass/fail outcomes; the leak was warning-level rather than failure-inducing. Full suite stays at 1191 pass.
chris-colinsky
added a commit
that referenced
this pull request
Jun 8, 2026
* Emit OA-namespace cache attributes on the LLM span Wire the §5.5.3.1 cache-attribute emission from proposal 0047 into the existing OTel observer's sentinel NodeEvent handler: - Extend LlmEventPayload with cached_tokens and cache_creation_tokens, defaulting to None to preserve the absent-vs-reported-zero distinction the spec mandates. - Have the OpenAI provider's _make_llm_event populate both fields from Response.usage (the fields PR 136 of 0047 added). The cache_creation_tokens field stays None for the OpenAI-compat mapping per spec §8.1.2 but the wiring is symmetric for future providers that source it. - In the OTel observer's _handle_llm_event completed phase, emit openarmature.llm.cache_read.input_tokens when cached_tokens is not None and openarmature.llm.cache_creation.input_tokens when cache_creation_tokens is not None. The conditional emission honors the §5.5.3 convention: absent (None) means the provider did not report; 0 means the provider reported zero hits. Cover the new behavior at three levels: - Four OTel observer unit tests drive the cache-attribute emission through synthetic sentinel started/completed NodeEvent pairs: cache hit, reported zero, absent, both fields populated. - Two provider-side unit tests verify _make_llm_event populates the payload's cache fields from Response.usage at the provider-payload boundary, so a regression here surfaces independently of the observer rendering layer. - Three conformance fixtures (040, 041, 042) activated via a new _run_llm_cache_fixture handler. The handler builds a single-LLM- call graph, captures the response, and asserts on response_usage + llm_span_attributes + llm_span_attributes_absent expectations. The dispatcher uses a set-membership check matching the precedent set by _run_llm_payload_fixture. The conformance.toml 0047 status stays at not-yet — wire-byte canonicalization and prompt-management substring stability are still ahead in the v0.13.0 cycle. * Close OpenAIProvider httpx clients in typed-event runners Address PR review feedback: _run_llm_cache_fixture_case constructed an OpenAIProvider but never awaited aclose(), leaking the underlying httpx.AsyncClient connection pool. Fixture 005 and 038 runners elsewhere in this file close the provider in a finally block; the new cache-fixture runner now follows the same convention. Extending the fix to the typed-event runners introduced in PR #139 (_build_simple_llm_graph, _run_typed_event_fanout_case, _run_typed_event_branches_case) — same bug class, same file, CoPilot missed them in the original review. _build_simple_llm_graph now returns (graph, state_cls, provider) so the caller can close the provider in a finally; the fan-out and parallel-branches runners close their inline-constructed providers symmetrically. No behavior change for fixture pass/fail outcomes; the leak was warning-level rather than failure-inducing. Full suite stays at 1191 pass.
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
typed_event_collectorharness directive support totests/conformance/test_observability.pyand activates all 7 conformance fixtures (8 cases) from proposal 0049:050-llm-completion-event-dispatch,051-llm-completion-event-type-discrimination,052-llm-completion-event-caller-metadata-opt-in(2 cases),053-llm-completion-event-no-event-on-failure,054-llm-completion-event-fan-out-index-population,055-llm-completion-event-branch-name-population, and056-llm-completion-event-strict-serial-ordering.contains_event,contains_exactly_one_event_of_type,contains_event_of_type,contains_exactly_n_events_of_type,does_not_contain_event_of_type,captured_event_field_values_cover(set-equality semantic),every_captured_event_has,relative_order_of_events_matching. An unknown-key guard rejects fixture typos before any assertion runs.NodeExceptioncause chain to reach the underlyingProviderUnavailablecategory.tests/conformance/test_typed_event_harness.pyadds 31 unit tests for the harness helpers' edge cases (typo detection, set-equality, namespace tuple-vs-list normalization, usage Mapping-vs-Usage comparison, cause-chain walking, opt-in aggregation).conformance.toml's[proposals."0049"]fromnot-yettoimplementedwithsince = "0.13.0". The OTel + Langfuse observers continue driving their span/generation surface off the sentinel NodeEvent pair during the dual-emit transition window; type-discrimination migration of those observers is queued behind proposal 0057 (request-side fields extension).Test plan
uv run pytest tests/— 1180 pass (was 1142 pre-PR; +7 fixtures + 31 helper unit tests)uv run pytest tests/conformance/test_observability.py -k 'llm-completion-event'— all 7 fixtures passuv run pytest tests/conformance/test_typed_event_harness.py— all 31 helper tests passuv run pyright tests/conformance/test_observability.py tests/conformance/test_typed_event_harness.py— cleanuv run ruff check tests/conformance/test_observability.py tests/conformance/test_typed_event_harness.py— cleanuv run python scripts/check_conformance_manifest.py— 51 accepted proposals, 51 manifest entries, all consistentNotes
results: list<dict>in the outer state but the LLM-drivenscoreis a string; the builder overridesresultsto plainlistso the collect step type-checks. ATODOcomment notes the override should be revisited if a future fixture revision adds final-state assertions.isinstance(event, LlmCompletionEvent)early-returns in PR Add LlmCompletionEvent typed event variant #137; they currently ignore the typed event and continue driving their§5.5 / §8.4.4surface off the sentinelNodeEventpair. Type-discrimination migration of those observers waits on proposal 0057 (request-side fields extension currently under spec review indiscuss-llm-completion-event-request-side-fields).