Skip to content

Commit a100712

Browse files
authored
fix!: Make AgentGraph traversal topological (#199)
## 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 map. 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 context map. ## What changed `lib/sdk/server-ai` — `AgentGraphDefinition.java` 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 map = the initial context plus only that node's true dependency results (transitive ancestors forward / descendants reverse). Dependencies are accumulated **before** the node is marked visited in both directions, so a self-loop node never includes itself; unrelated branches are excluded and the caller's map 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. ```java void traverse(BiFunction<AgentGraphNode, Map<String, Object>, Object> fn, Map<String, Object> ctx); ``` ## 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 `G1`–`G6` (+`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; a dedicated self-loop test confirms a node is not in its own context; deterministic across runs. Redundant per-vector order-only tests were dropped in favor of the data-driven vector test. ## 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. - Minor follow-up (not required): the class-level Javadoc still says traversal is "BFS-based" — worth updating to "topological / dependency-order" so the docs match the new behavior. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Overview** > Replaces **BFS** `traverse` / `reverseTraverse` on `AgentGraphDefinition` with **dependency-ordered** walks (Kahn-style, cycle-safe, BFS discovery for tie-breaks). Convergence nodes run only after all predecessors (or descendants in reverse); pure cycles now visit every reachable node instead of effectively no-oping on reverse. > > **Breaking behavior:** each visitor gets a **fresh** map—initial `ctx` plus only that node’s true dependency results (ancestors forward, descendants reverse). The caller’s `ctx` is **not** mutated with node outputs, and parallel-branch results no longer leak into unrelated callbacks. Self-loops exclude the node from its own context. > > Tests add cross-SDK **G1–G8** vectors for exact visit order and context keys, plus determinism, diamond graphs, and caller-context immutability checks. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 3c24de6. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 58f3c25 commit a100712

2 files changed

Lines changed: 491 additions & 113 deletions

File tree

‎lib/sdk/server-ai/src/main/java/com/launchdarkly/sdk/server/ai/AgentGraphDefinition.java‎

Lines changed: 226 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import com.launchdarkly.sdk.server.ai.internal.AgentGraphFlagValue;
44

5+
import java.util.AbstractMap;
56
import java.util.ArrayList;
67
import java.util.Collections;
78
import java.util.HashMap;
@@ -23,8 +24,9 @@
2324
* {@link #getConfig()} and {@link #createTracker()} remain meaningful, so callers can still inspect
2425
* the raw flag value and fire graph-level usage events for a disabled graph.
2526
* <p>
26-
* Traversal methods ({@link #traverse} and {@link #reverseTraverse}) are BFS-based and
27-
* cycle-safe: each node is visited at most once.
27+
* Traversal ({@link #traverse}, {@link #reverseTraverse}) visits each reachable node once in
28+
* topological order (predecessors-first or descendants-first), deterministically and cycle-safe.
29+
* Each visitor sees only the initial context plus that node's dependency results.
2830
* <p>
2931
* This class is thread-safe. All returned collections are unmodifiable.
3032
*/
@@ -158,16 +160,16 @@ public AIGraphTracker createTracker() {
158160
}
159161

160162
/**
161-
* Performs a BFS traversal of the graph starting from the root node.
163+
* Topological traversal from the root (predecessors-first; root first).
162164
* <p>
163-
* For each node visited, {@code fn} is called with the node and the mutable context map. The
164-
* return value of {@code fn} is stored in the context map under the node's key, making it
165-
* available to subsequently visited nodes. Each node is visited exactly once (cycle-safe).
165+
* A node runs only after all reachable predecessors. Ties break by discovery order (BFS from
166+
* root, declared edge order). Cycle-safe: each reachable node is visited once.
166167
* <p>
167-
* This is a no-op when the graph is disabled or the root node is absent.
168+
* {@code fn} receives a fresh map of the initial {@code ctx} plus that node's predecessor
169+
* results only. {@code ctx} itself is not mutated. No-op if disabled or root is absent.
168170
*
169-
* @param fn the visitor function; receives the current node and the context map
170-
* @param ctx the mutable context map; values from earlier nodes are available to later ones
171+
* @param fn visitor; node and dependency-scoped context
172+
* @param ctx initial context template (global scratch); not written with node results
171173
*/
172174
public void traverse(BiFunction<AgentGraphNode, Map<String, Object>, Object> fn,
173175
Map<String, Object> ctx) {
@@ -176,68 +178,251 @@ public void traverse(BiFunction<AgentGraphNode, Map<String, Object>, Object> fn,
176178
return;
177179
}
178180

181+
Map.Entry<Set<String>, List<String>> rd = reachableAndDiscovery(root.getKey());
182+
Set<String> reachable = rd.getKey();
183+
List<String> order = rd.getValue();
184+
185+
Map<String, Integer> indeg = new HashMap<>();
186+
for (String k : reachable) {
187+
indeg.put(k, 0);
188+
}
189+
for (String k : reachable) {
190+
AgentGraphNode node = getNode(k);
191+
if (node == null) {
192+
continue;
193+
}
194+
for (GraphEdge e : node.getEdges()) {
195+
if (reachable.contains(e.getKey())) {
196+
indeg.merge(e.getKey(), 1, Integer::sum);
197+
}
198+
}
199+
}
200+
indeg.put(root.getKey(), 0);
201+
179202
Set<String> visited = new HashSet<>();
180-
Queue<AgentGraphNode> queue = new LinkedList<>();
181-
visited.add(root.getKey());
182-
queue.add(root);
203+
Map<String, Object> results = new HashMap<>();
204+
Map<String, Set<String>> ancestors = new HashMap<>();
205+
while (visited.size() < reachable.size()) {
206+
String next = firstReady(order, visited, indeg);
207+
if (next == null) {
208+
next = lowestDegree(order, visited, indeg);
209+
}
183210

184-
while (!queue.isEmpty()) {
185-
AgentGraphNode node = queue.poll();
186-
Object result = fn.apply(node, ctx);
187-
ctx.put(node.getKey(), result);
211+
// Accumulate deps before marking visited so a self-loop does not count as its own ancestor.
212+
Set<String> anc = new HashSet<>();
213+
for (AgentGraphNode parent : getParentNodes(next)) {
214+
String pk = parent.getKey();
215+
if (!visited.contains(pk) || !reachable.contains(pk)) {
216+
continue;
217+
}
218+
anc.add(pk);
219+
Set<String> parentAnc = ancestors.get(pk);
220+
if (parentAnc != null) {
221+
anc.addAll(parentAnc);
222+
}
223+
}
224+
ancestors.put(next, anc);
225+
visited.add(next);
226+
227+
AgentGraphNode nextNode = getNode(next);
228+
results.put(next, fn.apply(nextNode, scopedCtx(ctx, results, anc)));
188229

189-
for (AgentGraphNode child : getChildNodes(node.getKey())) {
190-
if (visited.add(child.getKey())) {
191-
queue.add(child);
230+
if (nextNode != null) {
231+
for (GraphEdge e : nextNode.getEdges()) {
232+
if (reachable.contains(e.getKey())) {
233+
indeg.merge(e.getKey(), -1, Integer::sum);
234+
}
192235
}
193236
}
194237
}
195238
}
196239

197240
/**
198-
* Performs a reverse BFS traversal of the graph, starting from terminal nodes and working
199-
* upward toward the root.
241+
* Reverse topological traversal (descendants-first; root last).
242+
* <p>
243+
* A node runs only after all reachable descendants. Ties break by discovery order. Cycle-safe,
244+
* including graphs with no terminals. Each reachable node is visited once.
200245
* <p>
201-
* The root node is always processed last. Each node is visited exactly once (cycle-safe). This
202-
* is a no-op when the graph is disabled or there are no terminal nodes.
246+
* {@code fn} receives a fresh map of the initial {@code ctx} plus that node's descendant
247+
* results only. {@code ctx} itself is not mutated. No-op if disabled or root is absent.
203248
*
204-
* @param fn the visitor function; receives the current node and the context map
205-
* @param ctx the mutable context map; values from earlier nodes are available to later ones
249+
* @param fn visitor; node and dependency-scoped context
250+
* @param ctx initial context template (global scratch); not written with node results
206251
*/
207252
public void reverseTraverse(BiFunction<AgentGraphNode, Map<String, Object>, Object> fn,
208253
Map<String, Object> ctx) {
209254
AgentGraphNode root = rootNode();
210255
if (root == null) {
211256
return;
212257
}
258+
String rootKey = root.getKey();
259+
260+
Map.Entry<Set<String>, List<String>> rd = reachableAndDiscovery(rootKey);
261+
Set<String> reachable = rd.getKey();
262+
List<String> order = rd.getValue();
263+
264+
Map<String, Integer> outdeg = new HashMap<>();
265+
for (String k : reachable) {
266+
int d = 0;
267+
AgentGraphNode node = getNode(k);
268+
if (node != null) {
269+
for (GraphEdge e : node.getEdges()) {
270+
// The root is visited last, outside this loop, so no node waits on it.
271+
if (!e.getKey().equals(rootKey) && reachable.contains(e.getKey())) {
272+
d++;
273+
}
274+
}
275+
}
276+
outdeg.put(k, d);
277+
}
213278

214279
Set<String> visited = new HashSet<>();
215-
Queue<AgentGraphNode> queue = new LinkedList<>();
280+
Map<String, Object> results = new HashMap<>();
281+
Map<String, Set<String>> descendants = new HashMap<>();
282+
while (hasNonRootRemaining(reachable, visited, rootKey)) {
283+
String next = firstReadyNonRoot(order, visited, outdeg, rootKey);
284+
if (next == null) {
285+
next = lowestDegreeNonRoot(order, visited, outdeg, rootKey);
286+
}
216287

217-
// Seed from terminals, excluding root (it will be processed last).
218-
for (AgentGraphNode terminal : terminalNodes()) {
219-
if (!terminal.getKey().equals(root.getKey()) && visited.add(terminal.getKey())) {
220-
queue.add(terminal);
288+
// Accumulate deps before marking visited so a self-loop does not count as its own descendant.
289+
Set<String> desc = new HashSet<>();
290+
AgentGraphNode nextNode = getNode(next);
291+
if (nextNode != null) {
292+
for (GraphEdge e : nextNode.getEdges()) {
293+
String ck = e.getKey();
294+
if (!reachable.contains(ck) || !visited.contains(ck)) {
295+
continue;
296+
}
297+
desc.add(ck);
298+
Set<String> childDesc = descendants.get(ck);
299+
if (childDesc != null) {
300+
desc.addAll(childDesc);
301+
}
302+
}
303+
}
304+
descendants.put(next, desc);
305+
visited.add(next);
306+
results.put(next, fn.apply(nextNode, scopedCtx(ctx, results, desc)));
307+
308+
for (AgentGraphNode parent : getParentNodes(next)) {
309+
String pk = parent.getKey();
310+
if (!pk.equals(rootKey) && reachable.contains(pk)) {
311+
outdeg.merge(pk, -1, Integer::sum);
312+
}
221313
}
222314
}
223315

224-
while (!queue.isEmpty()) {
225-
AgentGraphNode node = queue.poll();
226-
Object result = fn.apply(node, ctx);
227-
ctx.put(node.getKey(), result);
316+
Set<String> rootDeps = new HashSet<>();
317+
for (String k : reachable) {
318+
if (!k.equals(rootKey)) {
319+
rootDeps.add(k);
320+
}
321+
}
322+
results.put(rootKey, fn.apply(root, scopedCtx(ctx, results, rootDeps)));
323+
}
228324

229-
for (AgentGraphNode parent : getParentNodes(node.getKey())) {
230-
if (!parent.getKey().equals(root.getKey()) && visited.add(parent.getKey())) {
231-
queue.add(parent);
325+
/** Reachable set and discovery order (BFS from root, declared edge order). */
326+
private Map.Entry<Set<String>, List<String>> reachableAndDiscovery(String rootKey) {
327+
Set<String> reachable = new HashSet<>();
328+
List<String> order = new ArrayList<>();
329+
Queue<String> queue = new LinkedList<>();
330+
reachable.add(rootKey);
331+
order.add(rootKey);
332+
queue.add(rootKey);
333+
while (!queue.isEmpty()) {
334+
String key = queue.poll();
335+
AgentGraphNode node = getNode(key);
336+
if (node == null) {
337+
continue;
338+
}
339+
for (GraphEdge edge : node.getEdges()) {
340+
if (getNode(edge.getKey()) != null && reachable.add(edge.getKey())) {
341+
order.add(edge.getKey());
342+
queue.add(edge.getKey());
232343
}
233344
}
234345
}
346+
return new AbstractMap.SimpleEntry<>(reachable, order);
347+
}
348+
349+
/** Copy of {@code initial} with {@code results} entries for {@code deps} overlaid. */
350+
private static Map<String, Object> scopedCtx(
351+
Map<String, Object> initial, Map<String, Object> results, Set<String> deps) {
352+
Map<String, Object> out = new HashMap<>(initial);
353+
for (String k : deps) {
354+
out.put(k, results.get(k));
355+
}
356+
return out;
357+
}
358+
359+
private static String firstReady(
360+
List<String> order, Set<String> visited, Map<String, Integer> degree) {
361+
for (String k : order) {
362+
if (!visited.contains(k) && degree.get(k) != null && degree.get(k) == 0) {
363+
return k;
364+
}
365+
}
366+
return null;
367+
}
368+
369+
private static String lowestDegree(
370+
List<String> order, Set<String> visited, Map<String, Integer> degree) {
371+
String best = null;
372+
int bestDeg = Integer.MAX_VALUE;
373+
for (String k : order) {
374+
if (visited.contains(k)) {
375+
continue;
376+
}
377+
Integer d = degree.get(k);
378+
int deg = d == null ? 0 : d;
379+
if (best == null || deg < bestDeg) {
380+
best = k;
381+
bestDeg = deg;
382+
}
383+
}
384+
return best;
385+
}
386+
387+
private static String firstReadyNonRoot(
388+
List<String> order, Set<String> visited, Map<String, Integer> degree, String rootKey) {
389+
for (String k : order) {
390+
if (k.equals(rootKey) || visited.contains(k)) {
391+
continue;
392+
}
393+
if (degree.get(k) != null && degree.get(k) == 0) {
394+
return k;
395+
}
396+
}
397+
return null;
398+
}
235399

236-
// Process root last (whether or not it was encountered as a parent above).
237-
if (visited.add(root.getKey())) {
238-
Object result = fn.apply(root, ctx);
239-
ctx.put(root.getKey(), result);
400+
private static String lowestDegreeNonRoot(
401+
List<String> order, Set<String> visited, Map<String, Integer> degree, String rootKey) {
402+
String best = null;
403+
int bestDeg = Integer.MAX_VALUE;
404+
for (String k : order) {
405+
if (k.equals(rootKey) || visited.contains(k)) {
406+
continue;
407+
}
408+
Integer d = degree.get(k);
409+
int deg = d == null ? 0 : d;
410+
if (best == null || deg < bestDeg) {
411+
best = k;
412+
bestDeg = deg;
413+
}
414+
}
415+
return best;
416+
}
417+
418+
private static boolean hasNonRootRemaining(
419+
Set<String> reachable, Set<String> visited, String rootKey) {
420+
for (String k : reachable) {
421+
if (!k.equals(rootKey) && !visited.contains(k)) {
422+
return true;
423+
}
240424
}
425+
return false;
241426
}
242427

243428
/**

0 commit comments

Comments
 (0)