Skip to content

Commit 835bfb9

Browse files
Fix failure-isolation event attempt_index
FailureIsolatedEvent.attempt_index now reports the wrapped node's final / exhausting attempt (spec ruling), not the post-reset baseline. An isolation-scoped terminal-attempt ContextVar carries it: RetryMiddleware records the final attempt on give-up, and FailureIsolationMiddleware reads it (falling back to the live attempt index when no retry exhausted). Un-defers conformance fixture 061.
1 parent 154434e commit 835bfb9

6 files changed

Lines changed: 147 additions & 62 deletions

File tree

conformance.toml

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -386,12 +386,13 @@ since = "0.13.0"
386386
# (LlmRetryAttemptEvent) scoped to a future cycle. Call-level retry
387387
# ships terminal-only: exactly one LlmCompletionEvent / LlmFailedEvent
388388
# per ``complete()`` call. Failure-isolation conformance fixtures
389-
# (058-063) are wired + passing this cycle EXCEPT 061 (three-piece
390-
# composition): the FailureIsolatedEvent's attempt_index there is the
391-
# node-level baseline (0) while the fixture asserts the final retry
392-
# attempt (1) per §6.3's lineage-correlation rule — RetryMiddleware
393-
# resets the attempt ContextVar before the outer isolation middleware
394-
# catches. That attempt_index reconciliation is pending.
389+
# (058-063) are all wired + passing: the FailureIsolatedEvent's
390+
# attempt_index reports the final / exhausting attempt per §6.3's
391+
# lineage-correlation rule (spec ruled this in the attempt-index coord
392+
# thread; RetryMiddleware now records the final attempt in a
393+
# terminal-attempt scope the outer isolation reads, rather than the
394+
# post-reset baseline). ``partial`` is now solely about the
395+
# call-level-retry per-attempt span surface above.
395396
[proposals."0050"]
396397
status = "partial"
397398
since = "0.14.0"

src/openarmature/graph/middleware/failure_isolation.py

Lines changed: 60 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -40,11 +40,14 @@
4040
from typing import Any
4141

4242
from openarmature.observability.correlation import (
43+
_reset_terminal_attempt_index,
44+
_set_terminal_attempt_index,
4345
current_attempt_index,
4446
current_branch_name,
4547
current_dispatch,
4648
current_fan_out_index,
4749
current_namespace_prefix,
50+
current_terminal_attempt_index,
4851
)
4952

5053
from ._core import NextCall
@@ -131,38 +134,48 @@ def __init__(
131134
self.on_caught = on_caught
132135

133136
async def __call__(self, state: Any, next_: NextCall) -> Mapping[str, Any]:
137+
# Establish a clean terminal-attempt scope: an inner
138+
# RetryMiddleware records its final / exhausting attempt here on
139+
# give-up, and _emit_event reports it (proposal 0050 §6.3). The
140+
# ``None`` on entry shadows any stale value from a prior node; the
141+
# finally cleans up so it never leaks to a sibling / next node.
142+
terminal_token = _set_terminal_attempt_index(None)
134143
try:
135-
return await next_(state)
136-
except Exception as exc:
137-
# BaseException (cancellation) never enters here — it
138-
# extends BaseException, not Exception. Same rule as
139-
# RetryMiddleware: cancellation MUST propagate.
140-
if self.predicate is not None and not self.predicate(exc):
141-
raise
142-
# Resolve the degraded update once, at catch time, and reuse
143-
# it for the event's post_state and the node return so a
144-
# callable degraded_update is invoked exactly once. The
145-
# observable order the spec prescribes — emit the event, then
146-
# on_caught, then return the update — is preserved below;
147-
# resolving here first only populates post_state.
148-
degraded = self._resolve_degraded(state)
149-
self._emit_event(state, exc, degraded)
150-
if self.on_caught is not None:
151-
try:
152-
await self.on_caught(exc)
153-
except Exception as hook_error: # noqa: BLE001
154-
# on_caught is caller telemetry; a bug in it MUST NOT
155-
# turn a recovered node back into a crash. Isolate it
156-
# the way the observer-delivery contract isolates
157-
# observer exceptions (warn, don't propagate).
158-
# BaseException (cancellation) still propagates by not
159-
# being caught here.
160-
warnings.warn(
161-
f"FailureIsolationMiddleware on_caught raised "
162-
f"{type(hook_error).__name__}: {hook_error}",
163-
stacklevel=2,
164-
)
165-
return degraded
144+
try:
145+
return await next_(state)
146+
except Exception as exc:
147+
# BaseException (cancellation) never enters here — it
148+
# extends BaseException, not Exception. Same rule as
149+
# RetryMiddleware: cancellation MUST propagate.
150+
if self.predicate is not None and not self.predicate(exc):
151+
raise
152+
# Resolve the degraded update once, at catch time, and
153+
# reuse it for the event's post_state and the node return
154+
# so a callable degraded_update is invoked exactly once.
155+
# The observable order the spec prescribes — emit the
156+
# event, then on_caught, then return the update — is
157+
# preserved below; resolving here first only populates
158+
# post_state.
159+
degraded = self._resolve_degraded(state)
160+
self._emit_event(state, exc, degraded)
161+
if self.on_caught is not None:
162+
try:
163+
await self.on_caught(exc)
164+
except Exception as hook_error: # noqa: BLE001
165+
# on_caught is caller telemetry; a bug in it MUST
166+
# NOT turn a recovered node back into a crash.
167+
# Isolate it the way the observer-delivery contract
168+
# isolates observer exceptions (warn, don't
169+
# propagate). BaseException (cancellation) still
170+
# propagates by not being caught here.
171+
warnings.warn(
172+
f"FailureIsolationMiddleware on_caught raised "
173+
f"{type(hook_error).__name__}: {hook_error}",
174+
stacklevel=2,
175+
)
176+
return degraded
177+
finally:
178+
_reset_terminal_attempt_index(terminal_token)
166179

167180
def _resolve_degraded(self, state: Any) -> Mapping[str, Any]:
168181
if callable(self.degraded_update):
@@ -192,22 +205,26 @@ def _emit_event(self, state: Any, exc: Exception, degraded: Mapping[str, Any]) -
192205
cause = _resolve_cause(exc)
193206
cause_category = getattr(cause, "category", None)
194207
category = cause_category if isinstance(cause_category, str) else None
195-
# ``attempt_index`` here is deliberately the NODE-level baseline,
196-
# not a per-attempt wire index: failure isolation is a node-level
197-
# concern ("the node, across its retries, was isolated"). When
198-
# this middleware is OUTER of RetryMiddleware, retry has already
199-
# reset the attempt ContextVar to that baseline (0) in its
200-
# ``finally`` by the time the terminal exception reaches this
201-
# catch, which is the frame we want (spec-confirmed). Parenting is
202-
# unaffected: the node's attempt spans are already closed by
203-
# delivery time (their completed event precedes this one on the
204-
# serial queue), so observers parent the marker under the
205-
# invocation span and correlate by ``namespace`` + node name.
208+
# ``attempt_index`` is the wrapped node's final / exhausting
209+
# attempt (proposal 0050 §6.3: "the same lineage tuple NodeEvent
210+
# carries, for correlation with the wrapped node's other events").
211+
# When this middleware is OUTER of RetryMiddleware, retry records
212+
# that index in the terminal-attempt scope on give-up — its own
213+
# ``finally`` has reset the live attempt-index var to the baseline
214+
# by the time the exception reaches this catch, so we read the
215+
# recorded terminal index instead. With no retry, nothing is
216+
# recorded and we fall back to the live attempt index (0 at a node
217+
# body). Parenting is unaffected: the node's attempt spans are
218+
# already closed by delivery time (their completed event precedes
219+
# this one on the serial queue), so observers parent the marker
220+
# under the invocation span and correlate by ``namespace`` + name.
221+
terminal_attempt = current_terminal_attempt_index()
222+
attempt_index = terminal_attempt if terminal_attempt is not None else current_attempt_index()
206223
dispatch(
207224
FailureIsolatedEvent(
208225
event_name=self.event_name,
209226
namespace=current_namespace_prefix(),
210-
attempt_index=current_attempt_index(),
227+
attempt_index=attempt_index,
211228
fan_out_index=current_fan_out_index(),
212229
branch_name=current_branch_name(),
213230
pre_state=state,

src/openarmature/graph/middleware/retry.py

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,11 @@
2424
from typing import Any
2525

2626
from openarmature.llm.errors import TRANSIENT_CATEGORIES
27-
from openarmature.observability.correlation import _reset_attempt_index, _set_attempt_index
27+
from openarmature.observability.correlation import (
28+
_record_terminal_attempt_index,
29+
_reset_attempt_index,
30+
_set_attempt_index,
31+
)
2832
from openarmature.observability.metadata import (
2933
_invocation_metadata_var,
3034
_reset_invocation_metadata,
@@ -202,6 +206,11 @@ async def __call__(self, state: Any, next_: NextCall) -> Mapping[str, Any]:
202206
# not the failed attempt's transient state.
203207
_reset_invocation_metadata(metadata_token)
204208
if attempt + 1 >= self.config.max_attempts or not classifier(exc, state):
209+
# Record the final / exhausting attempt so an OUTER
210+
# FailureIsolationMiddleware reports it rather than
211+
# the post-reset baseline (proposal 0050 §6.3). The
212+
# enclosing isolation scope owns the cleanup.
213+
_record_terminal_attempt_index(attempt)
205214
raise
206215
if self.config.on_retry is not None:
207216
await self.config.on_retry(exc, attempt)

src/openarmature/observability/correlation.py

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -483,6 +483,60 @@ def _reset_attempt_index(token: Token[int]) -> None:
483483
_attempt_index_var.reset(token)
484484

485485

486+
# ---------------------------------------------------------------------------
487+
# Terminal attempt index — for FailureIsolationMiddleware (proposal 0050
488+
# §6.3). RetryMiddleware resets ``attempt_index`` in its per-iteration
489+
# ``finally`` as the exhausted exception unwinds, so an OUTER
490+
# FailureIsolationMiddleware would otherwise read the post-reset baseline
491+
# rather than the final / exhausting attempt the §6.3 lineage-correlation
492+
# rule mandates. On give-up, retry records the final attempt here; the
493+
# enclosing FailureIsolationMiddleware establishes the scope (None on
494+
# entry, reset on exit) and reads it, falling back to ``attempt_index``
495+
# when no retry exhausted. Scoped by the isolation middleware so it never
496+
# leaks across nodes; the sole reader is FailureIsolationMiddleware, which
497+
# always shadows any stale value with its own ``None`` on entry.
498+
#
499+
# Two setters by design: ``_set`` / ``_reset`` bracket the isolation SCOPE
500+
# (token-based, mirroring ``_attempt_index``); ``_record`` is retry's
501+
# fire-and-forget write WITHIN that scope (no token, since the scope owner
502+
# does the reset), keeping retry off ContextVar bookkeeping it doesn't own.
503+
#
504+
# Known limitation: the INNERMOST isolation consumes the record (its reset
505+
# discards it), so a nested OUTER isolation catching an inner-isolation-
506+
# rejected exception after a retry exhaustion reads the live baseline. That
507+
# nesting is contrived and intentionally unguarded.
508+
# ---------------------------------------------------------------------------
509+
510+
511+
_terminal_attempt_index_var: ContextVar[int | None] = ContextVar(
512+
"openarmature.terminal_attempt_index", default=None
513+
)
514+
515+
516+
def current_terminal_attempt_index() -> int | None:
517+
"""Return the final / exhausting attempt index recorded by a retry
518+
that gave up within the current FailureIsolationMiddleware scope, or
519+
``None`` when no retry exhausted. Internal."""
520+
return _terminal_attempt_index_var.get()
521+
522+
523+
def _set_terminal_attempt_index(value: int | None) -> Token[int | None]:
524+
"""Establish a terminal-attempt scope (FailureIsolationMiddleware sets
525+
``None`` on entry). Internal."""
526+
return _terminal_attempt_index_var.set(value)
527+
528+
529+
def _reset_terminal_attempt_index(token: Token[int | None]) -> None:
530+
_terminal_attempt_index_var.reset(token)
531+
532+
533+
def _record_terminal_attempt_index(value: int) -> None:
534+
"""Record the final / exhausting attempt within the current scope
535+
(RetryMiddleware on give-up). The enclosing FailureIsolationMiddleware
536+
scope owns the cleanup, so no token is returned. Internal."""
537+
_terminal_attempt_index_var.set(value)
538+
539+
486540
# ---------------------------------------------------------------------------
487541
# Active observer span — for engine-side OTel context attach inside
488542
# ``innermost``. Populated synchronously by an observer's ``prepare_sync``
@@ -562,11 +616,13 @@ def _reset_active_observer_span(token: Token[object | None]) -> None:
562616
"current_fan_out_index_chain",
563617
"current_invocation_id",
564618
"current_namespace_prefix",
619+
"current_terminal_attempt_index",
565620
"validate_invocation_id",
566621
# Engine-internal lifecycle helpers — exported so the engine in
567622
# ``openarmature.graph.compiled`` can drive set/reset without
568623
# pyright's strict ``reportUnusedFunction`` flagging them as
569624
# dead. Underscore-prefixed; not part of the user-facing API.
625+
"_record_terminal_attempt_index",
570626
"_reset_active_dispatch",
571627
"_reset_active_observer_span",
572628
"_reset_active_observers",
@@ -578,6 +634,7 @@ def _reset_active_observer_span(token: Token[object | None]) -> None:
578634
"_reset_fan_out_index_chain",
579635
"_reset_invocation_id",
580636
"_reset_namespace_prefix",
637+
"_reset_terminal_attempt_index",
581638
"_set_active_dispatch",
582639
"_set_active_observer_span",
583640
"_set_active_observers",
@@ -589,4 +646,5 @@ def _reset_active_observer_span(token: Token[object | None]) -> None:
589646
"_set_fan_out_index_chain",
590647
"_set_invocation_id",
591648
"_set_namespace_prefix",
649+
"_set_terminal_attempt_index",
592650
]

tests/conformance/test_pipeline_utilities.py

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -133,18 +133,6 @@ def _fixture_id(path: Path) -> str:
133133
"029-checkpoint-subgraph-resume": "checkpointing (test_checkpoint.py)",
134134
"030-checkpoint-not-found": "checkpointing (test_checkpoint.py)",
135135
"031-checkpoint-correlation-id-preserved-across-resume": "checkpointing (test_checkpoint.py)",
136-
# Failure-isolation three-piece composition (proposal 0050 §6.3). The
137-
# FailureIsolatedEvent's attempt_index should reflect the final retry
138-
# attempt (1) per §6.3's "same lineage tuple NodeEvent carries"
139-
# correlation rule, but python emits the node-level baseline (0):
140-
# RetryMiddleware resets the attempt ContextVar in its finally before
141-
# the outer isolation middleware catches. The 0050 impl claimed 0 was
142-
# spec-confirmed via the design thread; the fixture asserts 1.
143-
# Reconcile with spec and fix attempt_index, then un-defer.
144-
"061-failure-isolation-retry-three-piece-composition": (
145-
"FailureIsolatedEvent.attempt_index baseline (0) vs final-attempt (1) "
146-
"discrepancy; reconcile with spec + fix"
147-
),
148136
}
149137

150138

tests/unit/test_failure_isolation_middleware.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -437,13 +437,25 @@ async def _flaky(_s: _DocState) -> Mapping[str, Any]:
437437
.compile()
438438
)
439439

440+
isolated: list[FailureIsolatedEvent] = []
441+
442+
async def _capture(event: ObserverEvent) -> None:
443+
if isinstance(event, FailureIsolatedEvent):
444+
isolated.append(event)
445+
446+
graph.attach_observer(_capture)
440447
final = await graph.invoke(_DocState())
441448
await graph.drain()
442449

443450
# Retry exhausted its 3 attempts; failure isolation then caught the
444451
# propagated exhaustion exception and substituted the degraded value.
445452
assert attempts["n"] == 3
446453
assert final.note == "gave_up"
454+
# The event's attempt_index is the final / exhausting attempt (2 after
455+
# attempts 0/1/2), not the post-reset baseline (proposal 0050 §6.3
456+
# lineage correlation).
457+
assert len(isolated) == 1
458+
assert isolated[0].attempt_index == 2
447459

448460

449461
# ---------------------------------------------------------------------------

0 commit comments

Comments
 (0)