FollowWiring Bug
This commit is contained in:
@@ -4,4 +4,4 @@
|
||||
server.url=http://localhost:8787
|
||||
# Stamped by manage-ac.sh (stamp_cli_version) from ac-code-server's agenticcode.version
|
||||
# at build time. "dev" means this jar wasn't built via manage-ac.sh.
|
||||
version=106
|
||||
version=108
|
||||
|
||||
@@ -3,7 +3,7 @@ quarkus.http.port=8787
|
||||
# AgenticCode's own release counter (not the Maven project version) — bump this by hand for each
|
||||
# release. Single source of truth for the startup log line, GET /api/version, and the MCP
|
||||
# 'version' tool/server-info (referenced below via property expression, not duplicated).
|
||||
agenticcode.version=106
|
||||
agenticcode.version=108
|
||||
|
||||
# MCP server (HTTP/SSE transport) — tools exposed at http://<host>:8787/mcp/sse
|
||||
quarkus.mcp.server.server-info.name=agenticcode
|
||||
|
||||
@@ -33,8 +33,15 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
|
||||
*
|
||||
* <p>The budget is driven down to 2 rather than nesting the fixture absurdly deep, because real Natural
|
||||
* nesting does not reach the default of 20 (measured on {@code upms}: at most 9 internal hops for a
|
||||
* single module hop). At {@code depth=1} the raw bound is therefore {@code 1*(1+2)=3}, and
|
||||
* {@code DEPTHLEAF} sits 5 raw edges behind {@code DEEPCHAIN}'s {@code PERFORM} chain.
|
||||
* single module hop). {@code DEPTHLEAF} sits 5 raw edges behind {@code DEEPCHAIN}'s {@code PERFORM} chain.
|
||||
*
|
||||
* <p><b>Item 94 changed what the budget can cut.</b> The budget used to bound the whole traversal, so an
|
||||
* internal {@code PERFORM} chain longer than it also hid the <em>module</em> at the end of that chain —
|
||||
* {@code DEPTHLEAF} was dropped although it is one module hop away, and {@code db-accesses?depth=1},
|
||||
* which has always taken its module set from the same BFS, reported it. The two endpoints contradicted
|
||||
* each other. Module rows now come from that BFS and are exact, so the budget bounds only the
|
||||
* intra-module subroutine walk. What gets cut here is therefore {@code S3}/{@code S4}, not
|
||||
* {@code DEPTHLEAF} — and that cut still has to announce itself, which is the point the test protects.
|
||||
*/
|
||||
@QuarkusTest
|
||||
@TestProfile(CallTreeTruncationIT.TinyBudgetProfile.class)
|
||||
@@ -84,11 +91,28 @@ class CallTreeTruncationIT {
|
||||
"a subroutine of the root module crosses no module boundary");
|
||||
body.setRootPath("");
|
||||
assertTrue(body.getBoolean("truncated"),
|
||||
"the traversal stopped at its raw-hop budget, so the caller must be told the list may be "
|
||||
"the intra-module walk stopped at its budget, so the caller must be told the list may be "
|
||||
+ "incomplete. Full response was: " + body.getList("items.name"));
|
||||
org.hamcrest.MatcherAssert.assertThat(
|
||||
"DEPTHLEAF sits 5 raw hops away, beyond the bound of 3 — it is the thing that got cut",
|
||||
body.getList("items.name"), not(hasItem("DEPTHLEAF")));
|
||||
"S3/S4 sit past the budget of 2 — the subroutine chain is what got cut",
|
||||
body.getList("items.name"), not(hasItem("S3")));
|
||||
org.hamcrest.MatcherAssert.assertThat(
|
||||
body.getList("items.name"), not(hasItem("S4")));
|
||||
}
|
||||
|
||||
/**
|
||||
* Item 94: the budget bounds the intra-module walk only. A module one hop away stays in the tree
|
||||
* however deep inside the caller its call site sits — otherwise {@code call-tree} would keep
|
||||
* disagreeing with {@code db-accesses}/{@code sql-statements}, which derive their module set from the
|
||||
* same BFS and have always reported it.
|
||||
*/
|
||||
@Test
|
||||
void aModuleOneHopAwayIsReportedEvenWhenItsCallSiteSitsPastTheBudget() {
|
||||
given().pathParam("name", "DEEPCHAIN")
|
||||
.queryParam("depth", 1)
|
||||
.when().get("/api/projects/" + PROJECT + "/modules/{name}/call-tree")
|
||||
.then().statusCode(200)
|
||||
.body("items.find { it.name == 'DEPTHLEAF' }.depth", org.hamcrest.Matchers.is(1));
|
||||
}
|
||||
|
||||
public static class TinyBudgetProfile implements QuarkusTestProfile {
|
||||
|
||||
@@ -2046,23 +2046,56 @@ public final class CypherQueries {
|
||||
* transitively instead of stopping at direct calls. {@code false} keeps the
|
||||
* exact default {@code CALLS}-only query.
|
||||
*/
|
||||
public static String callTree(int maxDepth, boolean resolveInterfaces, boolean followWiring, int internalBudget) {
|
||||
/**
|
||||
* Item 94: the {@code MODULE} rows of a call tree. Their depth is the minimum module-hop distance,
|
||||
* which is exactly what the {@code GraphRepository.moduleDepths} BFS already records, so it is passed
|
||||
* in rather than recomputed by enumerating paths. This query only attaches {@code sourceFile} to the
|
||||
* names the BFS found.
|
||||
*
|
||||
* @param resolveInterfaces item J3 - drop interface nodes that have a known implementation.
|
||||
*/
|
||||
public static String callTreeModuleRows(boolean resolveInterfaces) {
|
||||
String interfaceFilter = resolveInterfaces ? "WHERE NOT (target)-[:IMPLEMENTED_BY]->()" : "";
|
||||
String relTypes = followWiring ? "CALLS|INJECTS|REFERENCES" : "CALLS";
|
||||
int rawBound = maxDepth * (1 + internalBudget);
|
||||
// maxRaw is returned unfiltered so the caller can tell whether the traversal hit rawBound â i.e.
|
||||
return """
|
||||
UNWIND $modules AS modName
|
||||
MATCH (target:AstNode {type: 'MODULE', name: modName, project: $project})
|
||||
%s
|
||||
RETURN DISTINCT target.name AS name, target.type AS type, target.sourceFile AS sourceFile
|
||||
""".formatted(interfaceFilter);
|
||||
}
|
||||
|
||||
/**
|
||||
* Item 94: the {@code FUNCTION} rows of a call tree - the subroutines/methods reachable from a module
|
||||
* in the tree <em>without leaving it</em>, i.e. through non-{@code MODULE} nodes only. Each function
|
||||
* inherits the module-hop depth of the module it was reached from (the minimum, when several reach
|
||||
* it), which reproduces the old {@code min(#MODULE nodes on path) - 1} exactly.
|
||||
*
|
||||
* <p>Deliberately {@code CALLS}-only: {@code INJECTS}/{@code REFERENCES} are emitted class-to-class
|
||||
* ({@code JavaParser.addWiringEdges}) and materialized class-to-class by the CHA steps, so they can
|
||||
* never reach a {@code FUNCTION} and are not needed here. Keeping them out is the point: as dense
|
||||
* MODULE-to-MODULE edges they made the previous single-pattern traversal enumerate paths
|
||||
* combinatorially, so {@code followWiring} took >120s at depth 2 although the result had already
|
||||
* converged at a raw bound of 3.
|
||||
*
|
||||
* @param internalBudget raw hops allowed <em>within</em> one module (measured maximum: 9). Bounds this
|
||||
* traversal alone, no longer multiplied by the module depth. Natural needs the
|
||||
* full budget - its result only converges around 21 raw hops - and shows no
|
||||
* blow-up here, because intra-module {@code PERFORM} chains do not fan out.
|
||||
*/
|
||||
public static String callTreeFunctionRows(int internalBudget) {
|
||||
// maxRaw is returned unfiltered so the caller can tell whether the traversal hit its bound, i.e.
|
||||
// whether anything may have been cut off. It is reported rather than swallowed: an unannounced
|
||||
// truncation is exactly what made items 65 and 68 bugs instead of documented limits.
|
||||
return """
|
||||
MATCH (m:AstNode {type: 'MODULE', name: $name, project: $project})
|
||||
MATCH p = (m) (()-[:%s]->(x) WHERE x.type <> 'MODULE' OR x.name IN $modules){1,%d} (target:AstNode)
|
||||
%s
|
||||
WITH target, min(size([n IN nodes(p) WHERE n.type = 'MODULE']) - 1) AS depth,
|
||||
max(length(p)) AS maxRaw
|
||||
UNWIND $modules AS modName
|
||||
MATCH (m:AstNode {type: 'MODULE', name: modName, project: $project})
|
||||
MATCH p = (m) (()-[:CALLS]->(x) WHERE x.type <> 'MODULE'){1,%d} (target:AstNode)
|
||||
WHERE target.type = 'FUNCTION'
|
||||
WITH target, min($depths[modName]) AS depth, max(length(p)) AS maxRaw
|
||||
RETURN DISTINCT target.name AS name, target.type AS type, target.sourceFile AS sourceFile,
|
||||
depth AS depth, maxRaw AS maxRaw
|
||||
ORDER BY depth, name
|
||||
""".formatted(relTypes, rawBound, interfaceFilter);
|
||||
""".formatted(internalBudget);
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -430,16 +430,44 @@ public class GraphRepository {
|
||||
*/
|
||||
private static List<String> moduleTree(TransactionContext tx, String project, String root, int maxDepth,
|
||||
boolean followWiring) {
|
||||
Set<String> reached = new LinkedHashSet<>();
|
||||
reached.add(root);
|
||||
reached.addAll(moduleDepths(tx, project, root, maxDepth, followWiring).keySet());
|
||||
return List.copyOf(reached);
|
||||
}
|
||||
|
||||
/**
|
||||
* Item 94: the same BFS as {@link #moduleTree}, but keeping the hop at which each module was first
|
||||
* reached. That hop <em>is</em> the module-hop depth {@code call-tree} reports, so the depths no
|
||||
* longer have to be recovered by enumerating every path and taking a {@code min} over them.
|
||||
*
|
||||
* <p>The root is never expanded twice, but it is still recorded if some module calls back into it:
|
||||
* the path-enumerating predecessor reported such a cycle, so dropping it would be a silent behaviour
|
||||
* change.
|
||||
*
|
||||
* @return module name to minimum module hops from {@code root}, excluding {@code root} itself unless
|
||||
* it is genuinely reachable again.
|
||||
*/
|
||||
private static LinkedHashMap<String, Integer> moduleDepths(TransactionContext tx, String project, String root,
|
||||
int maxDepth, boolean followWiring) {
|
||||
String hopQuery = followWiring ? CypherQueries.MODULE_HOP_OUT_WIRING : CypherQueries.MODULE_HOP_OUT;
|
||||
Set<String> seen = new LinkedHashSet<>();
|
||||
seen.add(root);
|
||||
LinkedHashMap<String, Integer> depths = new LinkedHashMap<>();
|
||||
Set<String> expanded = new LinkedHashSet<>();
|
||||
expanded.add(root);
|
||||
List<String> frontier = List.of(root);
|
||||
for (int hop = 0; hop < maxDepth && !frontier.isEmpty(); hop++) {
|
||||
for (int hop = 1; hop <= maxDepth && !frontier.isEmpty(); hop++) {
|
||||
List<String> found = tx.run(hopQuery, Map.of("project", project, "names", frontier))
|
||||
.list(record -> record.get("name").asString());
|
||||
frontier = found.stream().filter(seen::add).toList();
|
||||
List<String> next = new ArrayList<>();
|
||||
for (String name : found) {
|
||||
depths.putIfAbsent(name, hop);
|
||||
if (expanded.add(name)) {
|
||||
next.add(name);
|
||||
}
|
||||
}
|
||||
frontier = List.copyOf(next);
|
||||
}
|
||||
return List.copyOf(seen);
|
||||
return depths;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -1741,27 +1769,45 @@ public class GraphRepository {
|
||||
* @param followWiring item J9 — also traverse {@code INJECTS}/{@code REFERENCES} edges (Java
|
||||
* DI/class-literal wiring), not just {@code CALLS}, so a job reaches its
|
||||
* steps/collaborators transitively.
|
||||
* @param internalBudget item 67 — raw hops allowed per module hop, a safety cap on the traversal
|
||||
* (see {@link CypherQueries#callTree}). Exhausting it sets
|
||||
* {@link CallTreeResponse#truncated()}.
|
||||
* @param internalBudget item 67/94 — raw hops allowed <em>within</em> one module, a safety cap on
|
||||
* the intra-module traversal (see
|
||||
* {@link CypherQueries#callTreeFunctionRows}). No longer multiplied by the
|
||||
* module depth: the module rows come from the BFS and are exact. Exhausting
|
||||
* it sets {@link CallTreeResponse#truncated()}, which therefore now means
|
||||
* "some module's internal subroutine chain may be cut off", not "the whole
|
||||
* traversal may be".
|
||||
*/
|
||||
public Uni<CallTreeResponse> callTree(String project, String moduleName, int maxDepth, boolean resolveInterfaces,
|
||||
boolean followWiring, int internalBudget) {
|
||||
int clampedDepth = Math.min(Math.max(maxDepth, 1), CALL_TREE_MAX_DEPTH_LIMIT);
|
||||
int rawBound = clampedDepth * (1 + internalBudget);
|
||||
return Uni.createFrom().item(() -> {
|
||||
try (Session session = driver.session()) {
|
||||
return session.executeRead((TransactionContext tx) -> {
|
||||
// Item 67: the module-hop bound. moduleTree (item 65) is the BFS that answers "which
|
||||
// modules are within N module hops"; the query below prunes its traversal to exactly
|
||||
// those, which is the only way to express the bound in module hops rather than raw
|
||||
// CALLS edges.
|
||||
List<String> modules = moduleTree(tx, project, moduleName, clampedDepth, followWiring);
|
||||
List<CallTreeRow> rows = tx.run(
|
||||
CypherQueries.callTree(clampedDepth, resolveInterfaces, followWiring, internalBudget),
|
||||
Map.of("project", project, "name", moduleName, "modules", modules))
|
||||
.list(GraphRepository::toCallTreeRow);
|
||||
return buildCallTreeResponse(rows, clampedDepth, rawBound);
|
||||
// Item 94: the BFS answers "which modules are within N module hops, and at which hop"
|
||||
// directly, so the MODULE rows are read off it instead of being recovered from a path
|
||||
// enumeration. Only the FUNCTION rows still need a traversal, and that one stays
|
||||
// inside a single module (CALLS-only), which is where the budget applies.
|
||||
LinkedHashMap<String, Integer> depths =
|
||||
moduleDepths(tx, project, moduleName, clampedDepth, followWiring);
|
||||
List<CallTreeRow> rows = new ArrayList<>();
|
||||
tx.run(CypherQueries.callTreeModuleRows(resolveInterfaces),
|
||||
Map.of("project", project, "modules", List.copyOf(depths.keySet())))
|
||||
.forEachRemaining(record -> {
|
||||
String targetName = record.get("name").asString();
|
||||
rows.add(new CallTreeRow(targetName,
|
||||
NodeType.valueOf(record.get("type").asString()),
|
||||
record.get("sourceFile").asString(""),
|
||||
depths.getOrDefault(targetName, clampedDepth), 0));
|
||||
});
|
||||
// The intra-module traversal starts at the root too: its own subroutines are depth 0.
|
||||
Map<String, Object> functionDepths = new LinkedHashMap<>(depths);
|
||||
functionDepths.put(moduleName, 0);
|
||||
rows.addAll(tx.run(CypherQueries.callTreeFunctionRows(internalBudget),
|
||||
Map.of("project", project,
|
||||
"modules", List.copyOf(functionDepths.keySet()),
|
||||
"depths", functionDepths))
|
||||
.list(GraphRepository::toCallTreeRow));
|
||||
return buildCallTreeResponse(rows, clampedDepth, internalBudget);
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
@@ -91,11 +91,12 @@ Endpoints that matter for this work:
|
||||
- **`db-accesses` at depth 0 is empty for orchestrators.** A Natural batch program usually has no direct SQL;
|
||||
the access happens in an access-layer subprogram (`Y****MN0`) it `CALLNAT`s. **Always query with
|
||||
`depth=3` or more** and read the `via` field to see which module actually touches the table.
|
||||
- **Transitive `db-accesses` drops `DECLARES` rows.** `db-accesses` on a Java `@Entity` reports its table;
|
||||
`db-accesses?depth=1` on the *same* module reports nothing. To get a Java class's table, query the
|
||||
entity module **directly, without `depth`**.
|
||||
- **`call-tree?followWiring=true` times out on `pur`** at `depth ≥ 2` (CHA fan-out explosion). Use it only
|
||||
at `depth=1`, or build the closure yourself by iterating `callees` breadth-first.
|
||||
- **`db-accesses?depth=N` is a superset of `db-accesses`.** Besides `READS`/`WRITES` it returns
|
||||
`mode: "DECLARES"` rows — the table a Java `@Entity`/repository maps to — with `via` naming the
|
||||
declaring module. This is how you get the Java side of a DB comparison: query the *caller* with
|
||||
`depth`, and read the entity's table off the `via` chain.
|
||||
- **`call-tree?followWiring=true` is the right tool for a Java job's closure** and works at full depth.
|
||||
A `callees`-only closure misses the steps/collaborators a batch job reaches through DI.
|
||||
- **The graph can be stale.** After code changes, refresh (`POST /api/projects/{p}/refresh`, or
|
||||
`?deep=true` for a full field-level pass) before trusting results. For Java work refresh **`pur`**, for
|
||||
Natural work refresh **`upms`** — refreshing the wrong project wastes several minutes.
|
||||
|
||||
@@ -342,11 +342,17 @@ or `CALLNAT` that only exists in a `.cpy` shows up in the host's `db-accesses`/`
|
||||
now inside the bound, because it crosses no further boundary. Previously `call-tree?depth=1` could hide
|
||||
a `DEFINE SUBROUTINE` of *the very module you asked about*, just because it was `PERFORM`ed from
|
||||
another subroutine (raw depth 2) — that is the same bug seen from the inside.
|
||||
- `call-tree` also returns **`truncated`**. `true` means the traversal stopped at its internal raw-hop
|
||||
budget, so `items` may be incomplete — **not** that your `depth` was exceeded (that is a normal,
|
||||
complete answer). It is conservative and can be `true` for a complete result. If you see it, query the
|
||||
deep branch directly instead of concluding the module calls nothing further. Tune via
|
||||
`agenticcode.call-tree.internal-budget` (default 20; the deepest internal chain observed in `upms` is 9).
|
||||
- `call-tree` also returns **`truncated`**. `true` means the *intra-module* subroutine walk stopped at
|
||||
its raw-hop budget, so some `FUNCTION` items may be missing — **not** that your `depth` was exceeded
|
||||
(that is a normal, complete answer). It is conservative and can be `true` for a complete result. Tune
|
||||
via `agenticcode.call-tree.internal-budget` (default 20; the deepest internal chain observed in `upms`
|
||||
is 9).
|
||||
- **Since item 94 the budget cannot hide a module.** `MODULE` rows come from the same module-hop BFS
|
||||
that `db-accesses`/`sql-statements` use, so a callee one hop away is always listed even when its call
|
||||
site sits behind a long internal `PERFORM` chain (before item 94 it was dropped, and `call-tree` then
|
||||
contradicted `db-accesses`). This also removed the path enumeration that made
|
||||
`call-tree?followWiring=true` time out on Java projects at `depth ≥ 2`; `followWiring` is now usable
|
||||
at full depth.
|
||||
- **`field-flow`'s `depth` counts module hops (item 68).** Like `db-accesses`/`sql-statements` (item 65),
|
||||
`variables/{name}/field-flow?depth=N` now means "up to N **module** calls apart", not N raw `CALLS`
|
||||
edges. Before item 68 a consumer called from inside a subroutine sat several raw hops away and was
|
||||
|
||||
@@ -184,6 +184,43 @@ wrong answer, found by the 2026-07-17 `VMULTMN4` audit.)*
|
||||
external subroutines are a language feature this parser does not resolve — worth its own item if the
|
||||
corpus ever needs it.)*
|
||||
|
||||
- [x] **94. `call-tree` no longer enumerates paths; `followWiring` usable again** (2026-07-20, JX0034N0 ↔
|
||||
MultiTableImportJob functional comparison). `call-tree?followWiring=true` timed out on `pur` at
|
||||
`depth ≥ 2` (>120s; depth 1 already took 5.3s), which made the Java wiring closure unobtainable.
|
||||
Measured cause — a single quantified path pattern over `CALLS|INJECTS|REFERENCES`, bounded by
|
||||
`maxDepth × (1 + internalBudget)` (= 42 at depth 2), recovering each target's depth as
|
||||
`min(#MODULE nodes on path) - 1`, i.e. by enumerating *every* path. With the CHA-materialized wiring
|
||||
edges (items 31/92) that is combinatorial:
|
||||
|
||||
| rawBound | Java, `followWiring`, depth 2 | Natural `JX0034N0`, depth 5 |
|
||||
|---|---|---|
|
||||
| 3 | 1.6s, 71 targets | — |
|
||||
| 4 | 1.7s, 71 targets | 2.6s, truncated (42) |
|
||||
| 6 | 18.9s, 71 targets | 3.2s, truncated (79) |
|
||||
| 8 / 12 | >120s | 2.2s / 2.4s, truncated (120/172) |
|
||||
| 21 | >120s | 3.9s, **converged (182)** |
|
||||
| 42 (production) | >120s | 3.2s, 182 |
|
||||
|
||||
So the budget is *necessary* for Natural (whose result converges only near 21) and *useless* for Java
|
||||
(converged at 3) — lowering it globally would silently truncate Natural, the exact failure its javadoc
|
||||
warns about. The blow-up comes from the wiring edges, which `JavaParser.addWiringEdges` and the CHA
|
||||
steps only ever emit **class-to-class**, so they can never reach a `FUNCTION`. Fix: split the query.
|
||||
`MODULE` rows now come straight from the `moduleDepths` BFS (its hop index *is* the module-hop depth —
|
||||
verified equal to the old query's module set: 71/71 for Java, 46/46 for Natural), and only `FUNCTION`
|
||||
rows still traverse, `CALLS`-only and bounded *within* one module. **Behaviour change:** the budget can
|
||||
no longer hide a module whose call site sits behind a long internal `PERFORM` chain — `DEPTHLEAF` is now
|
||||
reported at depth 1, which also removes a standing contradiction with `db-accesses`/`sql-statements`,
|
||||
whose module set always came from the same BFS. `truncated` accordingly now means "some module's
|
||||
internal subroutine chain may be cut off". Covered by `CallTreeTruncationIT` (both tests).
|
||||
|
||||
Measured live after deploy — `call-tree?followWiring=true` on `MultiTableImportJob`: depth 2 **1.9s**
|
||||
(was >120s) with the **same 71 modules**, depth 6 3.0s, depth 10 2.3s converging at 852 modules. On
|
||||
`upms/JX0034N0` at depth 5 the module set grew 46 → 53 with **nothing lost**; the seven that had been
|
||||
hidden are `NDBERR`, `NDBNOERR`, `USIX009N`, `USIX052N`, `USIX053N`, `YELEMGN0`, `YLITEMN0`. They are
|
||||
real: `USIX052N` is `CALLNAT`ed by `ISI173N0` (line 474), itself a direct callee of `JX0034N0`, so it
|
||||
sits at module depth 2; and `YLITEMN0` was already reported by `db-accesses?depth=5` as a `via`, which
|
||||
is the contradiction this item removes.
|
||||
|
||||
- [x] **93. Transitive `db-accesses` lost the `DECLARES` rows** (2026-07-20, JX0034N0 ↔ MultiTableImportJob
|
||||
functional comparison). `DB_ACCESSES` resolves a table from three sources — `READS`/`WRITES`, an entity's
|
||||
own `MAPS_TO`, and a repository's `repositoryEntity` (item 32) — but its transitive counterpart
|
||||
|
||||
Reference in New Issue
Block a user