Skip to content

fix!: Make AgentGraph traversal topological - #214

Open
mattrmc1 wants to merge 2 commits into
mainfrom
mmccarthy/AIC-3042/py-sdk-fix-agent-graph
Open

fix!: Make AgentGraph traversal topological#214
mattrmc1 wants to merge 2 commits into
mainfrom
mmccarthy/AIC-3042/py-sdk-fix-agent-graph

Conversation

@mattrmc1

@mattrmc1 mattrmc1 commented Jul 28, 2026

Copy link
Copy Markdown

Summary

Makes AgentGraphDefinition traversal topological and gives each visitor a dependency-scoped
execution context
, aligning the traversal behavior across the LaunchDarkly AI SDKs.

Previously traverse / reverse_traverse used depth-layering / level-by-level BFS over a single
shared, accumulating context dict. That had two problems:

  1. Ordering: where branches of unequal length converge, the convergence node could run before all
    of its predecessors on a longer path had run.
  2. Context leakage & mutation: every callback saw the results of all previously-visited nodes
    (including unrelated parallel branches), and results were written back into the caller's
    execution_context.

What changed

packages/sdk/server-aildai/agent_graph/__init__.py only (no public API/signature changes):

  • traverse now visits a node only after all of its reachable predecessors have been visited
    (Kahn over in-degree; the root is always released first). On cycles, the unvisited node with the
    lowest remaining in-degree is chosen next, ties broken by discovery order.
  • reverse_traverse now visits a node only after all of its reachable descendants have been
    visited, so the root is visited last (Kahn over out-degree, root excluded from cycle-break
    selection). Pure cycles now visit every node instead of being a no-op.
  • Scoped context: each callback receives a fresh dict = execution_context plus only that node's
    true dependency results (transitive ancestors forward / descendants reverse). Results are kept in a
    private map keyed by node; unrelated branches and self-loops are excluded, and the caller's dict is
    never mutated. Cross-node data flows only through callback return values.
  • Determinism: discovery order is the graph's BFS encounter order (root first, then declared edge
    order), used only for tie-breaks.
fn: Callable[[AgentGraphNode, Dict[str, Any]], Any]

Behavior change (breaking)

Titled fix! because visit order and callback-context contents change for graphs with convergent
paths, cycles, or parallel branches. Simple linear graphs are unaffected. Cross-SDK parity with
JS/Python/.NET/Java.

Tests

  • Canonical cross-SDK vectors G1G6 (+G2b) assert exact visit order and exact context
    keys in both directions.
  • Convergence runs the shared node last; pure cycle visits all nodes (root last); caller context not
    mutated; unrelated branches excluded; self-loop excluded from its own context; deterministic across
    runs.

Notes

  • No manual CHANGELOG.md edit — release-please generates it from the conventional-commit title.
  • Graph tracking / wire parsing unchanged; this PR is scoped to traversal + context.

Note

High Risk
Breaking change to traversal order and callback context for non-linear agent graphs; any production code depending on old depth ordering or full accumulated context will behave differently.

Overview
Replaces depth-layer BFS in AgentGraphDefinition.traverse / reverse_traverse with Kahn-style topological ordering (predecessors-first forward, descendants-first reverse, BFS discovery order for ties, explicit cycle breaks). Convergence nodes now run only after all reachable predecessors; pure cycles still visit every node with the root last on reverse.

Each visitor callback gets a fresh, dependency-scoped context (initial execution_context plus only transitive ancestor/descendant results), stored in a private results map—no mutation of the caller’s dict and no leakage from unrelated parallel branches.

Tests drop the old “every prior node in context” assertions, add G1–G6 / G2b parity vectors for exact visit order and context keys, plus determinism checks—aligning behavior with other LaunchDarkly AI SDKs. Breaking for convergent paths, cycles, and parallel graphs; linear graphs largely unchanged.

Reviewed by Cursor Bugbot for commit 3f801da. Bugbot is set up for automated code reviews on this repo. Configure here.

@mattrmc1 mattrmc1 changed the title fix!: use topological order and scoped context for agent graph traversal fix!: Make AgentGraph traversal topological Jul 28, 2026
@mattrmc1
mattrmc1 marked this pull request as ready for review July 29, 2026 21:54
@mattrmc1
mattrmc1 requested a review from a team as a code owner July 29, 2026 21:54

@cursor cursor Bot 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.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 3f801da. Configure here.

ancestors[nxt] = anc
node = self.get_node(nxt)
assert node is not None
results[nxt] = fn(node, scoped(anc))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Self-loop crashes scoped context

High Severity

traverse and reverse_traverse mark a node visited before building its dependency set, then treat a self-edge as a ready dependency. scoped then reads that node from results before it has been written, so any graph with a self-loop raises KeyError instead of excluding the node from its own context.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 3f801da. Configure here.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Seems legit

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