Skip to content

Commit 27e3782

Browse files
Tighten fan-out regression + fix CHANGELOG count
PR #102 review caught two issues: The fan-out regression test's inner subgraph contained only a raising node, so under the original shared-`latest_state_box` bug no inner step would have successfully written to the box — the test would have passed without exercising the leak it was meant to guard. The inner subgraph now has two nodes: `inner_succeeds` writes `inner_done=true` (so the descent's _invoke writes inner state to the box) followed by `inner_raises`. Confirmed by temp-reverting the descend-omit-`latest_state_box` change and observing the test fail with the typed-state-mismatch assertion. CHANGELOG said "three regression tests" but enumerated four (flat, subgraph, fan-out, parallel-branches). Bumped the count to four.
1 parent 4ceeec6 commit 27e3782

2 files changed

Lines changed: 27 additions & 5 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). The
2828
### Fixed
2929

3030
- **`InvocationCompletedEvent.final_state` on the failure path now surfaces the partial state at failure point.** Spec §8.4.1 *Resume semantics* requires the failure-path `trace.output` hook to receive "the partial final state captured at the failure point"; the original PR #99 implementation defaulted to `starting_state`, so the hook saw pre-execution state when it should have seen post-execution-up-to-failure state. The engine now tracks the latest post-merge state via a `latest_state_box` on `_InvocationContext`, updated after every successful step and read on the failure path. Success-path behavior unchanged.
31-
- **`latest_state_box` is per-context, not shared across subgraph descents.** Unlike the sibling `final_node_box` (which shares by reference because the spec wants the innermost failing node's name — the real culprit), `latest_state_box` must isolate per level so the outermost Langfuse trace receives outer-state-typed values. Without the isolation, a subgraph-internal step's inner-typed state would leak up to the outer trace.output hook, breaking the hook's typed contract. Each subgraph / fan-out instance / parallel-branches branch gets its own fresh box. Pinned by three regression tests covering flat, subgraph, fan-out, and parallel-branches failure paths.
31+
- **`latest_state_box` is per-context, not shared across subgraph descents.** Unlike the sibling `final_node_box` (which shares by reference because the spec wants the innermost failing node's name — the real culprit), `latest_state_box` must isolate per level so the outermost Langfuse trace receives outer-state-typed values. Without the isolation, a subgraph-internal step's inner-typed state would leak up to the outer trace.output hook, breaking the hook's typed contract. Each subgraph / fan-out instance / parallel-branches branch gets its own fresh box. Pinned by four regression tests covering flat, subgraph, fan-out, and parallel-branches failure paths.
3232

3333
### Notes
3434

tests/unit/test_observability_langfuse.py

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -926,10 +926,21 @@ class _FanOutOuterState(State):
926926
class _FanOutInnerState(State):
927927
item: int = 0
928928
out: int = 0
929+
inner_done: bool = False
930+
931+
932+
async def _fan_out_inner_succeeds(_s: _FanOutInnerState) -> dict[str, Any]:
933+
# Successful inner step — writes ``inner_done=true`` to the
934+
# instance's _invoke ``state`` local AND to the shared
935+
# ``latest_state_box`` (per-context, so it lands on the instance's
936+
# OWN box). Under the original shared-box bug this write would
937+
# leak into the outer box; under the per-context design it stays
938+
# isolated to the instance.
939+
return {"inner_done": True}
929940

930941

931942
async def _fan_out_inner_raises(_s: _FanOutInnerState) -> dict[str, Any]:
932-
raise RuntimeError("fan_out inner_node boom")
943+
raise RuntimeError("fan_out inner_raise boom")
933944

934945

935946
async def test_failure_path_final_state_is_outer_type_when_fan_out_inner_raises() -> None:
@@ -941,11 +952,22 @@ async def test_failure_path_final_state_is_outer_type_when_fan_out_inner_raises(
941952
# raises, the outermost ``invoke()``'s finally-block reads the
942953
# OUTER box — which holds outer state from ``outer_a``'s successful
943954
# completion, not the inner instance state.
955+
#
956+
# The inner subgraph has TWO inner nodes: ``inner_succeeds`` writes
957+
# inner state to the instance's box, then ``inner_raises``
958+
# propagates. Under the original shared-box bug, the box would
959+
# end with ``_FanOutInnerState(inner_done=true)`` and the outer
960+
# hook would receive that inner-typed value. The two-node shape
961+
# is load-bearing — a single-node "always raise" subgraph would
962+
# not exercise the leak because no successful inner step would
963+
# write to the box.
944964
inner_graph = (
945965
GraphBuilder(_FanOutInnerState)
946-
.add_node("inner_raise", _fan_out_inner_raises)
947-
.add_edge("inner_raise", END)
948-
.set_entry("inner_raise")
966+
.add_node("inner_succeeds", _fan_out_inner_succeeds)
967+
.add_node("inner_raises", _fan_out_inner_raises)
968+
.add_edge("inner_succeeds", "inner_raises")
969+
.add_edge("inner_raises", END)
970+
.set_entry("inner_succeeds")
949971
.compile()
950972
)
951973

0 commit comments

Comments
 (0)