Fix nested fan-out lineage in the engine and observers#194
Merged
Conversation
A fan-out nested inside an outer fan-out instance keyed its per-fan-out tracking entry by namespace and node name only, so the shared progress dict collided across concurrent outer instances. The second instance found the first's entry already complete and rolled its result forward, so every outer instance returned the first instance's inner result (silently wrong output) and the inner subgraph ran only once. Carry the enclosing fan-out instance lineage on the tracking key, in the in-memory dict and through the checkpoint projection, lookup, cleanup, and restore. Top-level and subgraph- or branch-nested fan-outs have an empty lineage, so their behavior, including resume, is unchanged. A fan-out nested inside an outer instance re-runs rather than skipping on resume, since the record format carries no lineage; tracked as a follow-up.
A fan-out instance dispatch or a parallel-branches per-branch dispatch nested inside an outer fan-out instance was keyed by its local namespace only, so the same dispatch in different outer instances shared one key: the second instance's inner nodes reparented under the first's dispatch and an inner augmentation reached the wrong outer instance. Key dispatches in both the OTel and Langfuse observers by their full enclosing fan-out instance and branch lineage, and resolve a nested node's parent and find its dispatch node by that lineage too. The per-branch key reads the branch name from the event so callable branches still synthesize. Un-defer conformance fixture 039 cases 1 and 2 (nested-lineage augmentation) in both observer harnesses.
There was a problem hiding this comment.
Pull request overview
Fixes a correctness bug where fan-outs nested inside an outer fan-out instance (and observer dispatch spans nested inside outer instances) were keyed without enclosing lineage, causing cross-instance collisions under concurrency and incorrect parenting/augmentation in observability.
Changes:
- Engine: include enclosing fan-out instance lineage in the in-memory fan-out progress key, and thread that through lookup and cleanup paths.
- Observers (OTel + Langfuse): make per-instance/per-branch dispatch keys lineage-aware and resolve parent/dispatch spans using the full enclosing chain.
- Tests + conformance: add regression/resume tests and un-defer conformance fixture 039 cases 1–2; document behavior in CHANGELOG.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/openarmature/graph/observer.py |
Extends invocation context fan-out progress key to include enclosing instance lineage. |
src/openarmature/graph/fan_out.py |
Uses lineage-aware key when registering per-fan-out execution state. |
src/openarmature/graph/compiled.py |
Updates fan-out progress lookup/projection/restore and cleanup to match lineage-aware keying. |
src/openarmature/observability/otel/observer.py |
Makes dispatch span keying and parent resolution lineage-aware for nested fan-out / parallel-branches. |
src/openarmature/observability/langfuse/observer.py |
Mirrors lineage-aware dispatch keying and resolution logic in Langfuse observer. |
tests/unit/test_fan_out.py |
Adds engine regression test for nested fan-out correctness under concurrency. |
tests/unit/test_checkpoint.py |
Adds checkpoint save/restore test coverage for nested fan-out progress projection and resume behavior. |
tests/unit/test_observability_otel.py |
Adds OTel tests asserting correct lineage isolation for nested dispatch spans and augmentation propagation. |
tests/conformance/test_observability_langfuse.py |
Wires and un-defers conformance fixture 039 cases 1–2 with helper builders. |
CHANGELOG.md |
Documents the nested-dispatch collision fixes (observers) and nested fan-out concurrency fix (engine). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The PR review's type-tightening surfaced two latent flat-key lookups that silently always-missed against the now-composite dispatch maps: the Langfuse LLM parent resolver and the OTel callable-branch provider-nesting lookup both still built the old flat key. Both now use the lineage-aware key, with a regression test for the Langfuse resolver (an LLM call inside a top-level fan-out instance parents under its per-instance dispatch). The tightening itself: the lineage-key helpers returned tuple[Any, ...] instead of their concrete shape. Add _DispatchKey and _BranchDispatchKey aliases for the helper returns, the dispatch maps, and the close-helper key params (all still typed tuple[str, ...] from before the lineage keys), and drop the now-unnecessary key[0] / key[1] casts. Also reword the resume comment so the nested-fan-out re-run reads as conditional on the outer instance re-entering.
chris-colinsky
added a commit
that referenced
this pull request
Jun 29, 2026
From a PR #197 review: several deferral reasons and conformance.toml statuses overstated non-implementation for partially-shipped work. - conformance.toml: 0081 not-yet -> textual-only (the <any-string> non-empty matcher is enforced by the adapter; precedent 0055/0071); 0085 not-yet -> partial (the save-side enclosing_fan_out_lineage keying shipped in #194; only the resume consume-side is unshipped), and drop the false "fixture 076 defers with it" claim (076 is number-gated, not deferred); drop the stale "a new case on 008" from the 0084 comment (that case is 0061's and is now wired). - Reword the 132 / 023-027 / 119 / 022-023 deferral reasons to state what actually ships (the 0084 lineage keying, the 0059 embed wire, 0075 callable branches, the structured_output_invalid base mapping) versus what is genuinely deferred. - Extract _assert_detached_raise_both_spans from the two near-identical 008 raise-case blocks, and add the parent openarmature.invocation ERROR assertion the fixtures' span_trees expect.
chris-colinsky
added a commit
that referenced
this pull request
Jun 29, 2026
* Bump spec conformance pin to v0.84.0 Advance the spec submodule to v0.84.0 and defer every conformance fixture the jump pulls in for proposals not yet implemented (0062, 0077-0089), at both the parser and run layers, so the suite is green at the new baseline. Mechanical: no proposal implemented, no src behavior changed. - spec submodule + the three pin constants (pyproject, __init__, test_smoke) move 0.70.1 -> 0.84.0. - conformance.toml: spec_pin -> v0.84.0 + 14 not-yet proposal rows. - The deferral dicts in the parser-level harness and the four per-capability runners gain the new fixtures, each reason naming its proposal; AGENTS.md regenerated. - 008's detached fan-out-instance error-status case (proposal 0061, implemented) is deferred pending a harness-wiring fix tracked separately; the mechanism stays verified by the subgraph case. * Wire 008 fan-out-instance error-status case Per a PR #197 review: _run_fixture_008_case silently skipped the detached fan-out-instance error-status case via a bare return, so the run reported it as passing without exercising it. Proposal 0061 is implemented, so this was a harness-wiring gap, not a not-yet deferral. Set expect_raise for the case and assert ERROR status + the exception event + the error category on both the parent fan-out node span and the raising instance's detached invocation span, mirroring the detached-subgraph error-status case. The case passes. * Tighten v0.84.0 deferral accuracy + dedup 008 From a PR #197 review: several deferral reasons and conformance.toml statuses overstated non-implementation for partially-shipped work. - conformance.toml: 0081 not-yet -> textual-only (the <any-string> non-empty matcher is enforced by the adapter; precedent 0055/0071); 0085 not-yet -> partial (the save-side enclosing_fan_out_lineage keying shipped in #194; only the resume consume-side is unshipped), and drop the false "fixture 076 defers with it" claim (076 is number-gated, not deferred); drop the stale "a new case on 008" from the 0084 comment (that case is 0061's and is now wired). - Reword the 132 / 023-027 / 119 / 022-023 deferral reasons to state what actually ships (the 0084 lineage keying, the 0059 embed wire, 0075 callable branches, the structured_output_invalid base mapping) versus what is genuinely deferred. - Extract _assert_detached_raise_both_spans from the two near-identical 008 raise-case blocks, and add the parent openarmature.invocation ERROR assertion the fixtures' span_trees expect.
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
A fan-out nested inside an outer fan-out instance was identified by a key that
omitted the enclosing instance lineage. The same defect showed up in two places.
Engine (correctness). The per-fan-out progress-tracking dict was keyed by
namespace and node name only, so concurrent outer instances collided. The second
instance found the first's entry already complete and rolled its result forward,
so every outer instance returned the first instance's inner result (silently
wrong output) and the inner subgraph ran only once. The tracking key now carries
the enclosing fan-out instance lineage, in the in-memory dict and through the
checkpoint projection, lookup, cleanup, and restore. Top-level and
subgraph- or branch-nested fan-outs have an empty lineage, so their behavior
(including resume) is unchanged.
Observers. The per-instance and per-branch dispatch spans were keyed by local
namespace only, so a dispatch in a different outer instance shared one key. Inner
nodes reparented under the wrong instance's dispatch and an inner augmentation
reached the wrong outer instance. Both the OTel and Langfuse observers now key
dispatches by the full enclosing fan-out instance and branch lineage, and resolve
a nested node's parent and find its dispatch node by that lineage too. The
per-branch key reads the branch name from the event so callable branches still
synthesize.
This un-defers conformance fixture 039 cases 1 and 2 (nested-lineage
augmentation) in both observer harnesses.
Tests
default concurrency returns the correct per-instance results.
fan-out's progress is projected onto the record and that resume rolls the
outer instances forward to the correct results).
dispatch lineage.
clean.
Follow-ups (tracked, out of scope here)
so inner nodes of different outer instances dedup under concurrency and drop
spans. The two observer nested tests use concurrency=1 around it. Engine results
are correct regardless of concurrency.
because the checkpoint record format carries no lineage. Fixing that is a
record-format change and is deferred for separate coordination.