diff --git a/CHANGELOG.md b/CHANGELOG.md index e07369b..74d64f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,7 +40,7 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). The - **A detached fan-out instance no longer opens a second root that abandons the first** (observability §4.4). A detached fan-out's per-instance root is stored under `prefix + (instance index)`, but the ancestor walk that decides whether to open one tested the bare `prefix`, so the test never matched and the arm fired once per inner **node event** rather than once per instance. Each additional inner node re-opened the root, and the second open replaced both the root and its detached invocation span in the observer's state, so the first pair was never ended and never exported. The result for a caller: three traces where there should be two, one instance's inner nodes split across two of them, and spans carrying a parent id that nothing emitted. It needs two or more nodes in the instance subgraph to see, which is why it went unnoticed: with a single node there is one event and one open, and every existing test used that shape. The guard now tests the key the root is actually stored under. The neighbouring bare-prefix test is kept for depth and is documented as redundant rather than load-bearing, since the detached-subgraph path is already guarded a line earlier. -- **A dispatch span synthesized from a wrapper-issued call no longer loses its subgraph identity or its caller metadata** (observability §5.4 / §5.5 / §5.6, graph-engine §6). When a fan-out's `instance_middleware` (or a parallel branch's) issues a provider or tool call and then returns without calling `next_call`, no inner node event is ever emitted, so the dispatch span the observer synthesizes from that call had nothing to repair it. Two attributes went missing and stayed missing. `openarmature.subgraph.name` was empty despite a declared `subgraph_identity`, because the identity reached an observer only through an inner node event; `fan_out_config` now carries an optional `subgraph_identity` alongside its four required keys, and the observer caches it from the fan-out node's own `started` event, which always precedes its instances. The cross-cutting `openarmature.user.*` set was absent whenever a `FailureIsolatedEvent` was the event that synthesized the span, which happens when a wrapper sets metadata and then raises without making a provider call first; that event now carries the same optional `caller_invocation_metadata` the provider events carry, populated in the engine task, because the metadata is per-async-context per §3.4 and an observer resolves on the serial delivery queue where a live read sees the wrong context. The same change puts the cross-cutting set on the `openarmature.failure_isolated` marker span, which §5.6 requires on every span in the invocation and which previously carried none. Both event-surface additions are additive: graph-engine §6 requires `fan_out_config` to present all four of its keys and does not close the set, and `caller_invocation_metadata` is already an optional field on the provider events. They ship ahead of any spec text, unit-tested, with standardization requested so a second implementation matches. +- **A dispatch span synthesized from a wrapper-issued call no longer loses its subgraph identity or its caller metadata** (observability §5.4 / §5.5 / §5.6, graph-engine §6). When a fan-out's `instance_middleware` (or a parallel branch's) issues a provider or tool call and then returns without calling `next_call`, no inner node event is ever emitted, so the dispatch span the observer synthesizes from that call had nothing to repair it. Two attributes went missing and stayed missing. `openarmature.subgraph.name` was empty despite a declared `subgraph_identity`, because the identity reached an observer only through an inner node event; `fan_out_config` now carries an optional `subgraph_identity` alongside its four required keys, and the observer caches it from the fan-out node's own `started` event, which always precedes its instances. The cross-cutting `openarmature.user.*` set was absent whenever a `FailureIsolatedEvent` was the event that synthesized the span, which happens when a wrapper sets metadata and then raises without making a provider call first; that event now carries the same optional `caller_invocation_metadata` the provider events carry, populated in the engine task, because the metadata is per-async-context per §3.4 and an observer resolves on the serial delivery queue where a live read sees the wrong context. The same change puts the cross-cutting set on the `openarmature.failure_isolated` marker span in both observers, which previously carried none. That is a consistency choice rather than a conformance one: `openarmature.failure_isolated` appears nowhere in the observability spec, and §5.6's scope sentence enumerates the spans it reaches without including it. The failure-isolation event is mandated by pipeline-utilities; the span emitted from it is ours and currently has no mapping, which spec is drafting. Both event-surface additions are additive: graph-engine §6 requires `fan_out_config` to present all four of its keys and does not close the set, and `caller_invocation_metadata` is already an optional field on the provider events. They ship ahead of any spec text, unit-tested, with standardization requested so a second implementation matches. - **A fan-out inside a parallel branch no longer returns a sibling branch's results** (pipeline-utilities §9 / §11). **Correctness fix; a graph could return data it never computed.** The engine keyed a fan-out's in-flight execution state by its namespace, node name, and enclosing fan-out-instance lineage. A parallel branch contributes to none of those: branch names never enter the namespace, and a branch descent adds only `null` entries to the fan-out-instance chain. So two sibling branches whose subgraphs each held a fan-out node of the same name built an identical key, and the second branch found the first's instances already `completed` and rolled its results forward. It executed none of its own item bodies and emitted no inner node events, so nothing in a trace showed the work had not happened. Where the two branches resolved different item counts the collision surfaced instead as a `CheckpointRecordInvalid` raised on a fresh run with no checkpointer attached, complaining about a saved record that did not exist. The state key now also carries the enclosing branch lineage, so sibling branches are distinct. The defect was schedule-dependent, and a single `await` in an item body was enough to hide it. Note for checkpointed graphs: a branch-nested fan-out now re-runs every instance on resume rather than skipping completed ones, because the checkpoint record has no field that distinguishes sibling branches and applying one branch's skips to another would be worse than re-running. Resume stays correct for this shape and saves no work; lifting that needs a record-format change, and is under discussion. diff --git a/src/openarmature/observability/langfuse/observer.py b/src/openarmature/observability/langfuse/observer.py index 5e93fef..21be9fd 100644 --- a/src/openarmature/observability/langfuse/observer.py +++ b/src/openarmature/observability/langfuse/observer.py @@ -964,11 +964,16 @@ def _handle_failure_isolated(self, event: FailureIsolatedEvent) -> None: correlation_id = current_correlation_id() if correlation_id is not None: metadata["correlation_id"] = correlation_id - # §5.6: the cross-cutting caller set goes on EVERY observation in the - # invocation, and this marker is one. It carried none until + # The cross-cutting caller set. It carried none until # `FailureIsolatedEvent` gained the field, because there was nothing to # read; omitting it now would leave the two observers disagreeing about # the same marker, which is worse than both lacking it. + # + # NOT a §5.6 obligation, though an earlier version of this comment said + # so. `openarmature.failure_isolated` appears nowhere in the + # observability spec: the EVENT is mandated by pipeline-utilities, the + # span is ours and unmapped. Cross-observer consistency is the reason + # this is here, not conformance. _apply_caller_metadata(metadata, event.caller_invocation_metadata) handle = self.client.span( trace_id=inv_state.trace_id, diff --git a/src/openarmature/observability/otel/observer.py b/src/openarmature/observability/otel/observer.py index fe1fdbd..5f8523f 100644 --- a/src/openarmature/observability/otel/observer.py +++ b/src/openarmature/observability/otel/observer.py @@ -2387,9 +2387,17 @@ def _handle_failure_isolated(self, event: FailureIsolatedEvent) -> None: cid = current_correlation_id() if cid is not None: attrs["openarmature.correlation_id"] = cid - # §5.6: the cross-cutting set goes on EVERY span emitted during the - # invocation, and this marker is one. It carried none until the event - # gained the field, because there was nothing to read. + # The cross-cutting caller set. It carried none until the event gained + # the field, because there was nothing to read. + # + # NOT a §5.6 obligation, though an earlier version of this comment said + # so. §5.6's scope sentence enumerates the spans it reaches -- invocation, + # node, subgraph, fan-out instance, LLM provider, retry attempt -- and + # `openarmature.failure_isolated` is not among them. It appears nowhere + # in the observability spec at all: the failure-isolation EVENT is + # mandated by pipeline-utilities, but the span we emit from it is ours + # and has no mapping. Spec is drafting one (coord release-v0.17.0/47); + # until it lands this is our own consistency choice, not conformance. _apply_caller_metadata(attrs, _event_caller_metadata(event)) span = self._tracer.start_span( name="openarmature.failure_isolated", diff --git a/tests/unit/test_failure_isolation_middleware.py b/tests/unit/test_failure_isolation_middleware.py index 257d010..f9ae9b2 100644 --- a/tests/unit/test_failure_isolation_middleware.py +++ b/tests/unit/test_failure_isolation_middleware.py @@ -730,10 +730,14 @@ async def test_langfuse_renders_failure_isolated_observation() -> None: async def test_langfuse_failure_isolated_marker_carries_caller_metadata() -> None: - # observability §5.6: the cross-cutting caller set goes on EVERY observation - # in the invocation, and this marker is one. It carried none until - # `FailureIsolatedEvent` gained `caller_invocation_metadata`, because there - # was nothing to read. + # The cross-cutting caller set on the failure-isolation marker. It carried + # none until `FailureIsolatedEvent` gained `caller_invocation_metadata`, + # because there was nothing to read. + # + # This pins a CONSISTENCY choice, not a §5.6 obligation. §5.6 enumerates the + # spans it reaches and this marker is not among them; the span appears + # nowhere in the observability spec, only the event does. Spec is drafting a + # mapping. If it lands differently, this assertion changes with it. # # Asserted on the LANGFUSE side specifically. The OTel half was covered # first, and covering only that leaves the two observers free to disagree