Skip to content

Activate proposal 0049 conformance fixtures 050-056#139

Merged
chris-colinsky merged 2 commits into
mainfrom
feature/0049-fixture-activation-typed-event-collector
Jun 8, 2026
Merged

Activate proposal 0049 conformance fixtures 050-056#139
chris-colinsky merged 2 commits into
mainfrom
feature/0049-fixture-activation-typed-event-collector

Conversation

@chris-colinsky

Copy link
Copy Markdown
Member

Summary

  • Adds the typed_event_collector harness directive support to tests/conformance/test_observability.py and 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, and 056-llm-completion-event-strict-serial-ordering.
  • New 8 assertion shape handlers cover: 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.
  • Three graph builders cover the fixture topologies: simple entry-ask-END, fan-out, parallel-branches. The failure-path runner walks the NodeException cause chain to reach the underlying ProviderUnavailable category.
  • New tests/conformance/test_typed_event_harness.py adds 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).
  • Flips conformance.toml's [proposals."0049"] from not-yet to implemented with since = "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).
  • Fourth PR of the v0.13.0 LLM hardening cycle (proposal 0049).

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 pass
  • uv run pytest tests/conformance/test_typed_event_harness.py — all 31 helper tests pass
  • uv run pyright tests/conformance/test_observability.py tests/conformance/test_typed_event_harness.py — clean
  • uv run ruff check tests/conformance/test_observability.py tests/conformance/test_typed_event_harness.py — clean
  • uv run python scripts/check_conformance_manifest.py — 51 accepted proposals, 51 manifest entries, all consistent

Notes

  • The fan-out fixture (054) declares results: list<dict> in the outer state but the LLM-driven score is a string; the builder overrides results to plain list so the collect step type-checks. A TODO comment notes the override should be revisited if a future fixture revision adds final-state assertions.
  • The OTel + Langfuse observers gained no-op 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.4 surface off the sentinel NodeEvent pair. Type-discrimination migration of those observers waits on proposal 0057 (request-side fields extension currently under spec review in discuss-llm-completion-event-request-side-fields).

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.
Copilot AI review requested due to automatic review settings June 8, 2026 00:42

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.py unit tests for typed-event harness helper behavior.
  • Mark proposal 0049 as implemented in conformance.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.

Comment thread tests/conformance/test_observability.py
Comment thread tests/conformance/test_observability.py
Comment thread tests/conformance/test_observability.py
Comment thread tests/conformance/test_observability.py Outdated
Comment thread tests/conformance/test_observability.py
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 chris-colinsky merged commit e8906da into main Jun 8, 2026
5 checks passed
@chris-colinsky chris-colinsky deleted the feature/0049-fixture-activation-typed-event-collector branch June 8, 2026 00:56
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.
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.

2 participants