Skip to content

fix!: Make AgentGraph traversal topological - #325

Open
mattrmc1 wants to merge 9 commits into
mainfrom
mmccarthy/AIC-3043/agent-graph-traversal
Open

fix!: Make AgentGraph traversal topological#325
mattrmc1 wants to merge 9 commits into
mainfrom
mmccarthy/AIC-3043/agent-graph-traversal

Conversation

@mattrmc1

@mattrmc1 mattrmc1 commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

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 / ReverseTraverse were plain BFS over a single shared, accumulating context
dictionary. That had two problems:

  1. Ordering: where branches of unequal length converge, the convergence node ran on first
    discovery — before all of its predecessors 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
    initialContext.

What changed

pkgs/sdk/server-aiGraph/AgentGraphDefinition.cs 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.
  • ReverseTraverse 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 dictionary = initialContext 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 dictionary 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. Ready-check uses == 0 to match the reference algorithm and the
    other SDKs.
void Traverse(Func<AgentGraphNode, Dictionary<string, object>, object> fn,
              Dictionary<string, object> initialContext = null);

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

  • A data-driven test over the canonical cross-SDK vectors G1G6 (+G2b) asserts 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

Overview
Breaking behavior change (signatures unchanged): AgentGraphDefinition.Traverse and ReverseTraverse no longer use plain BFS with one shared, mutating context dictionary.

Forward traversal now walks only nodes reachable from the root in topological order (Kahn on in-degree; ties broken by BFS discovery / declared edge order). Each callback gets initialContext plus only transitive predecessor results—not parallel-branch siblings—and return values are stored privately instead of writing into the caller’s dictionary.

Reverse traversal uses the same reachability set in reverse topological order (descendants first, root last). Pure cycles now visit every node once instead of being a no-op; context is scoped to reachable descendants the same way.

A private ReachableAndDiscovery helper supplies reachability and tie-break order. Tests are expanded with cross-SDK G1–G6 / G2b vectors for exact visit order and context keys, plus updates for cycles, scoped context, self-loops, and determinism.

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

@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

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.

There are a lot of new dictionary calls, toDictionary, as well as several contains and order by. Do you have any performance requirements wrt throughput or memory allocation delays? May be worth a quick benchmark to see if your expected cases are well behaved.

I'm guessing this would apply to the other PRs for the other SDKs if they are following similar implementations.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No hard requirements here — traversal runs once per graph invocation and every callback makes an LLM call, so we're spending microseconds against seconds. Benchmarked to confirm: a 20-node graph is ~65 µs / 60 KB and a 50-node fan-out ~150 µs, with degradation only past a few hundred nodes, well beyond expected graph sizes.

@mattrmc1 mattrmc1 Aug 26, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Took a look at the space concerns too. The old approach was actually worse since it carried context level to level. Now, if you have a fan out graph, each node gets initialContext plus ONLY its transitive ancestors. The old way gave each node all context from levels above as well as sibling context depending on order of BFS traversal. Assuming the the graph is a diamond shape, the bloated context on the exit node is the same either way.

        a
       / \
      b   c
      |   |
      d   e
       \ /
        f
node old context new context
a {} {}
b {a} {a}
c {a, b} {a}
d {a, b, c} {a, b}
e {a, b, c, d} {a, c}
f {a, b, c, d, e} {a, b, c, d, e}

@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 6ba4a7f. Configure here.

Comment thread pkgs/sdk/server-ai/src/Graph/AgentGraphDefinition.cs
@mattrmc1
mattrmc1 requested a review from jsonbailey August 14, 2026 17:33
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