Skip to content

Commit 9f61c02

Browse files
ualtinokalfonso-aft
andcommitted
mason: collapse unresolved callgraph edges
Co-authored-by: Alfonso <289616620+alfonso-aft@users.noreply.github.com>
1 parent f357a7d commit 9f61c02

8 files changed

Lines changed: 286 additions & 15 deletions

File tree

packages/aft-bridge/src/__tests__/callgraph-format.test.ts

Lines changed: 78 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,24 +21,96 @@ describe("formatCallgraphSections", () => {
2121
expect(text).toContain("2 truncated");
2222
});
2323

24-
test("call_tree marks unresolved callees and leaves resolved ones unmarked", () => {
24+
test("call_tree collapses unresolved leaf children by default", () => {
2525
const text = formatCallgraphSections("call_tree", {
2626
name: "entry",
2727
file: "/repo/src/a.ts",
2828
line: 1,
2929
resolved: true,
3030
children: [
31-
{ name: "realCallee", file: "/repo/src/b.ts", line: 10, resolved: true, children: [] },
32-
{ name: "missing", file: "/repo/src/a.ts", line: 3, resolved: false, children: [] },
31+
{ name: "len", file: "/repo/src/a.ts", line: 2, resolved: false, children: [] },
32+
{ name: "Some", file: "/repo/src/a.ts", line: 3, resolved: false, children: [] },
33+
{ name: "len", file: "/repo/src/a.ts", line: 4, resolved: false, children: [] },
34+
{ name: "assert", file: "/repo/src/a.ts", line: 5, resolved: false, children: [] },
35+
{
36+
name: "realCallee",
37+
file: "/repo/src/b.ts",
38+
line: 10,
39+
resolved: true,
40+
children: [],
41+
},
42+
{
43+
name: "wrapping_add",
44+
file: "/repo/src/a.ts",
45+
line: 6,
46+
resolved: false,
47+
children: [],
48+
},
49+
{ name: "as_ref", file: "/repo/src/a.ts", line: 7, resolved: false, children: [] },
50+
{ name: "as_ptr", file: "/repo/src/a.ts", line: 8, resolved: false, children: [] },
51+
{ name: "load", file: "/repo/src/a.ts", line: 9, resolved: false, children: [] },
52+
{ name: "lock", file: "/repo/src/a.ts", line: 11, resolved: false, children: [] },
53+
{ name: "Err", file: "/repo/src/a.ts", line: 12, resolved: false, children: [] },
54+
{ name: "cfg", file: "/repo/src/a.ts", line: 13, resolved: false, children: [] },
55+
{ name: "panic", file: "/repo/src/a.ts", line: 14, resolved: false, children: [] },
3356
],
3457
}).join("\n");
35-
// Unresolved callee: file/line is the callsite, not a definition — must be flagged.
58+
59+
expect(text).toContain(
60+
"↳ + 12 unresolved external calls: len, Some, assert, wrapping_add, as_ref, as_ptr, load, lock, Err, cfg, … (+1 more)",
61+
);
62+
expect(text.match(/unresolved external calls/g) ?? []).toHaveLength(1);
63+
expect(text).toContain("realCallee [/repo/src/b.ts:10]");
64+
expect(text).not.toContain("len [/repo/src/a.ts:2] [unresolved]");
65+
expect(text).not.toContain("panic [/repo/src/a.ts:14] [unresolved]");
66+
});
67+
68+
test("call_tree includeUnresolved renders every unresolved callee individually", () => {
69+
const text = formatCallgraphSections(
70+
"call_tree",
71+
{
72+
name: "entry",
73+
file: "/repo/src/a.ts",
74+
line: 1,
75+
resolved: true,
76+
children: [
77+
{ name: "realCallee", file: "/repo/src/b.ts", line: 10, resolved: true, children: [] },
78+
{ name: "missing", file: "/repo/src/a.ts", line: 3, resolved: false, children: [] },
79+
{ name: "len", file: "/repo/src/a.ts", line: 4, resolved: false, children: [] },
80+
],
81+
},
82+
undefined,
83+
{ includeUnresolved: true },
84+
).join("\n");
85+
3686
expect(text).toContain("missing [/repo/src/a.ts:3] [unresolved]");
87+
expect(text).toContain("len [/repo/src/a.ts:4] [unresolved]");
88+
expect(text).not.toContain("unresolved external calls");
3789
// Resolved callee carries no marker.
3890
expect(text).toContain("realCallee [/repo/src/b.ts:10]");
3991
expect(text).not.toContain("realCallee [/repo/src/b.ts:10] [unresolved]");
4092
});
4193

94+
test("call_tree resolved-only output is unchanged by unresolved collapse option", () => {
95+
const payload = {
96+
name: "entry",
97+
file: "/repo/src/a.ts",
98+
line: 1,
99+
resolved: true,
100+
children: [
101+
{ name: "realCallee", file: "/repo/src/b.ts", line: 10, resolved: true, children: [] },
102+
],
103+
};
104+
const collapsed = formatCallgraphSections("call_tree", payload).join("\n");
105+
const expanded = formatCallgraphSections("call_tree", payload, undefined, {
106+
includeUnresolved: true,
107+
}).join("\n");
108+
109+
expect(collapsed).toBe(expanded);
110+
expect(collapsed).toContain("realCallee [/repo/src/b.ts:10]");
111+
expect(collapsed).not.toContain("unresolved external calls");
112+
});
113+
42114
test("callers collapses repeated symbols and keeps true total in summary", () => {
43115
const sections = formatCallgraphSections("callers", {
44116
total_callers: 16,
@@ -217,7 +289,7 @@ describe("formatCallgraphSections", () => {
217289
expect(roles).toContain("warning:~");
218290
});
219291

220-
test("call_tree keeps [unresolved] independent of name_match marker", () => {
292+
test("call_tree preserves name_match markers on resolved edges when siblings collapse", () => {
221293
const text = formatCallgraphSections("call_tree", {
222294
name: "entry",
223295
file: "/repo/a.ts",
@@ -242,7 +314,7 @@ describe("formatCallgraphSections", () => {
242314
],
243315
}).join("\n");
244316
expect(text).toContain("nameOnly [/repo/b.ts:9] ~");
245-
expect(text).toContain("missing [/repo/a.ts:3] [unresolved]");
317+
expect(text).toContain("+ 1 unresolved external call: missing");
246318
expect(text).not.toContain("[unresolved] ~");
247319
});
248320

packages/aft-bridge/src/callgraph-format.ts

Lines changed: 60 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,12 @@ export interface CallgraphTheme {
88
fg(role: string, text: string): string;
99
}
1010

11+
export interface CallgraphFormatOptions {
12+
includeUnresolved?: boolean;
13+
}
14+
15+
const UNRESOLVED_SUMMARY_NAME_LIMIT = 10;
16+
1117
export const PLAIN_CALLGRAPH_THEME: CallgraphTheme = {
1218
fg: (_role, text) => text,
1319
};
@@ -54,11 +60,33 @@ function nameMatchEdgeMarker(record: Record<string, unknown>, theme: CallgraphTh
5460
return asString(record.resolved_by) === "name_match" ? ` ${theme.fg("warning", "~")}` : "";
5561
}
5662

63+
function isUnresolvedLeaf(node: Record<string, unknown>): boolean {
64+
return node.resolved === false && asRecords(node.children).length === 0;
65+
}
66+
67+
function unresolvedSummaryText(nodes: Record<string, unknown>[]): string {
68+
const distinctNames: string[] = [];
69+
const seen = new Set<string>();
70+
for (const node of nodes) {
71+
const name = asString(node.name) ?? "(unknown)";
72+
if (seen.has(name)) continue;
73+
seen.add(name);
74+
distinctNames.push(name);
75+
}
76+
77+
const displayed = distinctNames.slice(0, UNRESOLVED_SUMMARY_NAME_LIMIT);
78+
const hidden = distinctNames.length - displayed.length;
79+
const names = hidden > 0 ? `${displayed.join(", ")}, … (+${hidden} more)` : displayed.join(", ");
80+
const noun = nodes.length === 1 ? "call" : "calls";
81+
return `+ ${nodes.length} unresolved external ${noun}: ${names}`;
82+
}
83+
5784
function renderCallTreeNode(
5885
node: Record<string, unknown>,
5986
depth: number,
6087
lines: string[],
6188
theme: CallgraphTheme,
89+
options: CallgraphFormatOptions,
6290
): void {
6391
const name = asString(node.name) ?? "(unknown)";
6492
const file = shortenPath(asString(node.file) ?? "(unknown file)");
@@ -71,8 +99,36 @@ function renderCallTreeNode(
7199
const nameMatch = nameMatchEdgeMarker(node, theme);
72100
const location = line !== undefined ? `[${file}:${line}]` : `[${file}]`;
73101
lines.push(treeLine(depth, `${name} ${location}${unresolved}${nameMatch}`));
74-
asRecords(node.children).forEach((child) => {
75-
renderCallTreeNode(child, depth + 1, lines, theme);
102+
103+
const children = asRecords(node.children);
104+
if (options.includeUnresolved) {
105+
children.forEach((child) => {
106+
renderCallTreeNode(child, depth + 1, lines, theme, options);
107+
});
108+
return;
109+
}
110+
111+
const unresolvedLeaves = children.filter(isUnresolvedLeaf);
112+
if (unresolvedLeaves.length === 0) {
113+
children.forEach((child) => {
114+
renderCallTreeNode(child, depth + 1, lines, theme, options);
115+
});
116+
return;
117+
}
118+
119+
const unresolvedLeafSet = new Set(unresolvedLeaves);
120+
let summaryInserted = false;
121+
children.forEach((child) => {
122+
if (unresolvedLeafSet.has(child)) {
123+
if (!summaryInserted) {
124+
lines.push(
125+
treeLine(depth + 1, theme.fg("warning", unresolvedSummaryText(unresolvedLeaves))),
126+
);
127+
summaryInserted = true;
128+
}
129+
return;
130+
}
131+
renderCallTreeNode(child, depth + 1, lines, theme, options);
76132
});
77133
}
78134

@@ -153,13 +209,14 @@ export function formatCallgraphSections(
153209
op: string,
154210
response: unknown,
155211
theme: CallgraphTheme = PLAIN_CALLGRAPH_THEME,
212+
options: CallgraphFormatOptions = {},
156213
): string[] {
157214
const record = asRecord(response);
158215
if (!record) return [theme.fg("muted", "No navigation result.")];
159216

160217
if (op === "call_tree") {
161218
const lines: string[] = [];
162-
renderCallTreeNode(record, 0, lines, theme);
219+
renderCallTreeNode(record, 0, lines, theme, options);
163220
const warning = depthWarning(record, theme);
164221
if (warning) lines.push(warning);
165222
return lines.length > 0 ? lines : [theme.fg("muted", "No call tree available.")];

packages/aft-bridge/src/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ export {
4141
tagStderrLine,
4242
} from "./bridge.js";
4343
// --- aft_callgraph flat formatter (shared by both plugin hosts) ---
44-
export type { CallgraphTheme } from "./callgraph-format.js";
44+
export type { CallgraphFormatOptions, CallgraphTheme } from "./callgraph-format.js";
4545
export { formatCallgraphSections, PLAIN_CALLGRAPH_THEME } from "./callgraph-format.js";
4646
export {
4747
coerceBoolean,

packages/opencode-plugin/src/__tests__/navigation.test.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,44 @@ describe("aft_callgraph OpenCode adapter", () => {
9393
});
9494
});
9595

96+
test("includeUnresolved is exposed and controls call_tree formatting only", async () => {
97+
const payload = {
98+
success: true,
99+
name: "run",
100+
file: "/repo/src/app.ts",
101+
line: 1,
102+
children: [
103+
{ name: "len", file: "/repo/src/app.ts", line: 2, resolved: false, children: [] },
104+
{ name: "Some", file: "/repo/src/app.ts", line: 3, resolved: false, children: [] },
105+
{ name: "project", file: "/repo/src/project.ts", line: 4, resolved: true, children: [] },
106+
],
107+
};
108+
const { bridge, calls } = makeMockBridge(() => payload);
109+
const tools = navigationTools(makePluginContext(bridge));
110+
111+
expect(Object.hasOwn(tools.aft_callgraph.args, "includeUnresolved")).toBe(true);
112+
expect(tools.aft_callgraph.description).toContain("includeUnresolved=true");
113+
114+
const collapsed = (await tools.aft_callgraph.execute(
115+
{ op: "call_tree", filePath: "src/app.ts", symbol: "run" },
116+
makeToolContext(),
117+
)) as string;
118+
const expanded = (await tools.aft_callgraph.execute(
119+
{ op: "call_tree", filePath: "src/app.ts", symbol: "run", includeUnresolved: true },
120+
makeToolContext(),
121+
)) as string;
122+
123+
expect(collapsed).toContain("+ 2 unresolved external calls: len, Some");
124+
expect(collapsed).toContain("project [/repo/src/project.ts:4]");
125+
expect(collapsed).not.toContain("len [/repo/src/app.ts:2] [unresolved]");
126+
expect(expanded).toContain("len [/repo/src/app.ts:2] [unresolved]");
127+
expect(expanded).toContain("Some [/repo/src/app.ts:3] [unresolved]");
128+
expect(expanded).not.toContain("unresolved external calls");
129+
expect(calls).toHaveLength(2);
130+
expect(calls[0].params).not.toHaveProperty("includeUnresolved");
131+
expect(calls[1].params).not.toHaveProperty("includeUnresolved");
132+
});
133+
96134
test("trace_to_symbol ambiguous_target errors include candidates (Rust top-level shape)", async () => {
97135
// Rust's error_with_data() merges extras into the top-level response,
98136
// so production traffic has `candidates` next to `code`/`message`, NOT

packages/opencode-plugin/src/tools/navigation.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ export function navigationTools(ctx: PluginContext): Record<string, ToolDefiniti
4040
"- 'trace_to_symbol': Shortest call path from one symbol to another. Requires 'toSymbol'. If multiple targets match, the error returns candidate files; retry with 'toFile' to disambiguate.\n" +
4141
"- 'trace_data': Follow a value through variable assignments and function parameters across files. Requires 'symbol' (scope to trace from) and 'expression'.\n\n" +
4242
"All ops require both 'filePath' and 'symbol'. 'expression' is additionally required for trace_data; 'toSymbol' for trace_to_symbol.\n\n" +
43-
"Markers: ~ = edge resolved by name only (may point at the wrong same-named symbol); [unresolved] = callee not resolved to a definition, so the location shown is the call site. Unmarked edges are resolved exactly.\n",
43+
"Markers: ~ = edge resolved by name only (may point at the wrong same-named symbol); [unresolved] = callee not resolved to a definition, so the location shown is the call site. Unmarked edges are resolved exactly. By default, unresolved external/stdlib leaf calls in call_tree are collapsed into one summary per parent; pass includeUnresolved=true to show every unresolved edge individually.\n",
4444
// Parameters are Zod-optional because different ops need different subsets.
4545
// Runtime guards below validate per-op requirements and give clear errors.
4646
args: {
@@ -76,6 +76,12 @@ export function navigationTools(ctx: PluginContext): Record<string, ToolDefiniti
7676
.boolean()
7777
.optional()
7878
.describe("Include test files in callers/paths. Defaults to false; tests are hidden."),
79+
includeUnresolved: z
80+
.boolean()
81+
.optional()
82+
.describe(
83+
"Show every unresolved external/stdlib call individually. Defaults to false; unresolved leaf calls are collapsed into one summary per parent.",
84+
),
7985
},
8086
execute: async (args, context): Promise<string> => {
8187
if (isEmptyParam(args.filePath)) {
@@ -129,7 +135,9 @@ export function navigationTools(ctx: PluginContext): Record<string, ToolDefiniti
129135
}
130136
throw new Error(message);
131137
}
132-
return formatCallgraphSections(args.op as string, response).join("\n");
138+
return formatCallgraphSections(args.op as string, response, undefined, {
139+
includeUnresolved: coerceBoolean(args.includeUnresolved),
140+
}).join("\n");
133141
},
134142
},
135143
};

packages/pi-plugin/src/__tests__/navigate-renderers.test.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,45 @@ describe("navigate renderer", () => {
127127
expect(traceData).toContain("depth limited");
128128
});
129129

130+
test("renderNavigateResult honors includeUnresolved", () => {
131+
const payload = {
132+
name: "run",
133+
file: "/repo/src/a.ts",
134+
line: 1,
135+
children: [
136+
{ name: "len", file: "/repo/src/a.ts", line: 2, resolved: false, children: [] },
137+
{ name: "Some", file: "/repo/src/a.ts", line: 3, resolved: false, children: [] },
138+
],
139+
};
140+
141+
const collapsed = renderToString(
142+
renderNavigateResult(
143+
makeResult("", payload),
144+
{ op: "call_tree", filePath: "src/a.ts", symbol: "run" },
145+
mockTheme,
146+
makeContext({ op: "call_tree", filePath: "src/a.ts", symbol: "run" }),
147+
),
148+
);
149+
const expanded = renderToString(
150+
renderNavigateResult(
151+
makeResult("", payload),
152+
{ op: "call_tree", filePath: "src/a.ts", symbol: "run", includeUnresolved: true },
153+
mockTheme,
154+
makeContext({
155+
op: "call_tree",
156+
filePath: "src/a.ts",
157+
symbol: "run",
158+
includeUnresolved: true,
159+
}),
160+
),
161+
);
162+
163+
expect(collapsed).toContain("+ 2 unresolved external calls: len, Some");
164+
expect(collapsed).not.toContain("len [/repo/src/a.ts:2] [unresolved]");
165+
expect(expanded).toContain("len [/repo/src/a.ts:2] [unresolved]");
166+
expect(expanded).not.toContain("unresolved external calls");
167+
});
168+
130169
test("renderNavigateResult handles error and empty payloads", () => {
131170
const error = renderToString(
132171
renderNavigateResult(

0 commit comments

Comments
 (0)