mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-08 03:08:13 +00:00
fix(processes): explore siblings in source order, log the exhausted budget
`slice(0, maxBranching)` selected the FIRST N callees while `pop()` explored
them LAST-first, so the trace budget went to the last-declared branch. For
`main() { init(); loadConfig(); run(); shutdown(); }` the walk spends itself on
`shutdown` and can drop `init` — the earliest steps of a flow, which is the
opposite of what a process describes. Selecting first-N and exploring last-first
was simply inconsistent; pushing in reverse makes the stack pop in source order.
Measured on the reporting repo, this costs depth: 6-8 step processes go 168 ->
146 of 816. Still roughly three times the pre-PR baseline of 50, and the right
trade — a deep branch is no longer reached by accident of being declared last.
The remaining limit is the BUDGET, not the traversal: with a fixed quota a deep
branch declared after enough shallow ones is not reached at all. That is now
asserted in both directions rather than left implicit, and the walk logs when it
stops with branches unexplored — a silently truncating cap reads as "this is
everything", the same confident-empty answer this work is about, and the repo
already sets that precedent for `dispatchFanoutSkipped`.
Removes the second depth test, which was vacuous: the note twelve lines above
it already said a `processProcesses`-level depth assertion passes under BOTH
traversals, and measured it does — breadth-first yields the same deepest
stepCount of 8, so it passed with the production change reverted. Traversal
order is asserted against `traceFromEntryPoint` directly; what is observable at
the pipeline level is which traces survive selection, which the diversity tests
cover.
Also renames `queue` to `stack` and corrects the BFS references in the module
docstring and the function's own JSDoc, which is what an IDE hover shows.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
3c5eadc705
commit
56f34561fe
2 changed files with 63 additions and 70 deletions
|
|
@ -3,7 +3,7 @@
|
|||
*
|
||||
* Detects execution flows (Processes) in the code graph by:
|
||||
* 1. Finding entry points (functions with no internal callers)
|
||||
* 2. Tracing forward via CALLS edges (BFS)
|
||||
* 2. Tracing forward via CALLS edges (DFS)
|
||||
* 3. Grouping and deduplicating similar paths
|
||||
* 4. Labeling with heuristic names
|
||||
*
|
||||
|
|
@ -386,11 +386,11 @@ const findEntryPoints = (
|
|||
};
|
||||
|
||||
// ============================================================================
|
||||
// HELPER: Trace from entry point (BFS)
|
||||
// HELPER: Trace from entry point (DFS)
|
||||
// ============================================================================
|
||||
|
||||
/**
|
||||
* Trace forward from an entry point using BFS.
|
||||
* Trace forward from an entry point using DEPTH-first search.
|
||||
* Returns all distinct paths up to maxDepth.
|
||||
*/
|
||||
// Exported for tests ONLY: traversal order is the whole behaviour here, and it
|
||||
|
|
@ -421,10 +421,13 @@ export const traceFromEntryPoint = (
|
|||
// same `maxTraceDepth` ceiling — only the ORDER of exploration differs, and
|
||||
// the caller already sorts by length and dedupes by endpoint, so it was
|
||||
// always asking for the deepest traces this walk could give it.
|
||||
const queue: [string, string[]][] = [[entryId, [entryId]]];
|
||||
// A LIFO stack, not a queue — the name followed the traversal when this was
|
||||
// breadth-first and was left behind by the change to depth-first.
|
||||
const stack: [string, string[]][] = [[entryId, [entryId]]];
|
||||
|
||||
while (queue.length > 0 && traces.length < config.maxBranching * 3) {
|
||||
const [currentId, path] = queue.pop()!;
|
||||
const traceBudget = config.maxBranching * 3;
|
||||
while (stack.length > 0 && traces.length < traceBudget) {
|
||||
const [currentId, path] = stack.pop()!;
|
||||
|
||||
// Get outgoing calls
|
||||
const callees = callsEdges.get(currentId) || [];
|
||||
|
|
@ -444,10 +447,18 @@ export const traceFromEntryPoint = (
|
|||
const limitedCallees = callees.slice(0, config.maxBranching);
|
||||
let addedBranch = false;
|
||||
|
||||
for (const calleeId of limitedCallees) {
|
||||
// PUSHED IN REVERSE so the stack POPS them in source order. `slice`
|
||||
// selects the first N callees while `pop()` takes the last pushed, so
|
||||
// without this the walk spends its trace budget on the LAST-declared
|
||||
// branch first: for `main() { init(); loadConfig(); run(); shutdown(); }`
|
||||
// it explores `shutdown` first and can exhaust the quota before reaching
|
||||
// `init` — dropping the earliest steps of a flow, which is the opposite
|
||||
// of what a process is meant to describe. Selecting the first N and then
|
||||
// exploring them last-first was simply inconsistent.
|
||||
for (const calleeId of [...limitedCallees].reverse()) {
|
||||
// Avoid cycles
|
||||
if (!path.includes(calleeId)) {
|
||||
queue.push([calleeId, [...path, calleeId]]);
|
||||
stack.push([calleeId, [...path, calleeId]]);
|
||||
addedBranch = true;
|
||||
}
|
||||
}
|
||||
|
|
@ -459,6 +470,17 @@ export const traceFromEntryPoint = (
|
|||
}
|
||||
}
|
||||
|
||||
// A silently truncating cap reads as "this is everything", which is the same
|
||||
// class of confident-empty answer this work is about. The repo already sets
|
||||
// this precedent for `dispatchFanoutSkipped` and
|
||||
// `propertyDispatch.skippedKeys`.
|
||||
if (stack.length > 0) {
|
||||
logger.debug(
|
||||
{ entryId, traceBudget, unexploredBranches: stack.length },
|
||||
'process-processor: trace budget exhausted; unexplored branches remain for this entry point',
|
||||
);
|
||||
}
|
||||
|
||||
return traces;
|
||||
};
|
||||
|
||||
|
|
|
|||
|
|
@ -585,83 +585,54 @@ describe('process depth (D1/D2)', () => {
|
|||
// passes under BOTH traversals and guards nothing.
|
||||
const cfg = { maxTraceDepth: 10, maxBranching: 4, maxProcesses: 75, minSteps: 3 };
|
||||
|
||||
it('descends a deep chain instead of spending the budget on shallow branches', () => {
|
||||
const deepAndShallow = (order: readonly string[]): Map<string, string[]> => {
|
||||
// Fan-out is capped at maxBranching (4), so the budget is exhausted BELOW
|
||||
// the entry: three shallow branches carrying four immediate terminals each
|
||||
// = 12 traces, exactly the walk budget (maxBranching * 3). Breadth-first
|
||||
// records all twelve and stops before descending the fourth branch.
|
||||
// records all twelve and stops before descending the deep branch at all.
|
||||
const calls = new Map<string, string[]>();
|
||||
calls.set('entry', ['s1', 's2', 's3', 'd1']);
|
||||
calls.set('entry', [...order]);
|
||||
for (const b of ['s1', 's2', 's3']) {
|
||||
calls.set(b, [`${b}_l1`, `${b}_l2`, `${b}_l3`, `${b}_l4`]);
|
||||
}
|
||||
for (let i = 1; i <= 7; i++) calls.set(`d${i}`, [`d${i + 1}`]);
|
||||
return calls;
|
||||
};
|
||||
|
||||
const traces = traceFromEntryPoint('entry', calls, cfg);
|
||||
it('descends a deep chain instead of spending the budget on shallow branches', () => {
|
||||
const traces = traceFromEntryPoint('entry', deepAndShallow(['d1', 's1', 's2', 's3']), cfg);
|
||||
const deepest = Math.max(0, ...traces.map((t) => t.length));
|
||||
// Shallow terminals are 3 nodes. Anything longer proves it descended.
|
||||
expect(deepest).toBeGreaterThan(3);
|
||||
});
|
||||
const addFn = (graph: ReturnType<typeof createKnowledgeGraph>, id: string): void => {
|
||||
graph.addNode({
|
||||
id,
|
||||
label: 'Function',
|
||||
properties: { name: id.split(':')[1], filePath: 'src/a.ts', startLine: 1, endLine: 2 },
|
||||
});
|
||||
};
|
||||
const addCall = (
|
||||
graph: ReturnType<typeof createKnowledgeGraph>,
|
||||
from: string,
|
||||
to: string,
|
||||
): void => {
|
||||
graph.addRelationship({
|
||||
id: `rel:${from}->${to}`,
|
||||
sourceId: from,
|
||||
targetId: to,
|
||||
type: 'CALLS',
|
||||
confidence: 1,
|
||||
reason: 'test',
|
||||
});
|
||||
};
|
||||
|
||||
it('finds a deep chain that shallow branches would otherwise crowd out', async () => {
|
||||
const graph = createKnowledgeGraph();
|
||||
addFn(graph, 'func:entry');
|
||||
|
||||
// Fan-out is capped at `maxBranching` (4), so the budget can only be
|
||||
// exhausted BELOW the entry, not beside it: three shallow branches each
|
||||
// carrying four immediate terminals = 12 traces, exactly the walk's trace
|
||||
// budget (maxBranching * 3). Breadth-first records all twelve before it
|
||||
// ever descends the fourth branch, and stops.
|
||||
for (let b = 1; b <= 3; b++) {
|
||||
const mid = `func:s${b}`;
|
||||
addFn(graph, mid);
|
||||
addCall(graph, 'func:entry', mid);
|
||||
for (let l = 1; l <= 4; l++) {
|
||||
const leaf = `func:l${b}_${l}`;
|
||||
addFn(graph, leaf);
|
||||
addCall(graph, mid, leaf);
|
||||
}
|
||||
}
|
||||
|
||||
// The fourth branch is a deep chain: entry → d1 → … → d8 (terminal).
|
||||
let prev = 'func:entry';
|
||||
addFn(graph, 'func:d1');
|
||||
addCall(graph, 'func:entry', 'func:d1');
|
||||
prev = 'func:d1';
|
||||
for (let i = 2; i <= 8; i++) {
|
||||
const id = `func:d${i}`;
|
||||
addFn(graph, id);
|
||||
addCall(graph, prev, id);
|
||||
prev = id;
|
||||
}
|
||||
|
||||
const result = await processProcesses(graph, []);
|
||||
const deepest = Math.max(0, ...result.processes.map((p) => p.stepCount));
|
||||
// Shallow terminals are 3 steps. Anything deeper proves the walk descended
|
||||
// instead of spending its whole budget fanning out.
|
||||
expect(deepest).toBeGreaterThan(3);
|
||||
// Sibling ORDER, which the walk previously got backwards: `slice` selected
|
||||
// the first N callees while `pop()` explored them last-first, so the budget
|
||||
// went to the LAST-declared branch. For `main() { init(); …; shutdown(); }`
|
||||
// that spends the walk on `shutdown` and can drop `init` — the earliest steps
|
||||
// of a flow, which is the opposite of what a process describes.
|
||||
//
|
||||
// The consequence is honest and worth pinning: with a fixed trace budget, a
|
||||
// deep branch declared AFTER enough shallow ones is not reached. That is a
|
||||
// budget limitation, not a traversal one, and it must not be silent.
|
||||
it('follows source order, so an early deep branch wins and a late one may not', () => {
|
||||
const early = traceFromEntryPoint('entry', deepAndShallow(['d1', 's1', 's2', 's3']), cfg);
|
||||
const late = traceFromEntryPoint('entry', deepAndShallow(['s1', 's2', 's3', 'd1']), cfg);
|
||||
expect(Math.max(0, ...early.map((t) => t.length))).toBeGreaterThan(3);
|
||||
// Not asserted as a desirable outcome — asserted so a change to the budget
|
||||
// shows up here rather than silently altering which flows exist.
|
||||
expect(Math.max(0, ...late.map((t) => t.length))).toBe(3);
|
||||
});
|
||||
// The `processProcesses`-level depth test that used to sit here was VACUOUS,
|
||||
// and the note at the top of this describe says exactly why: `findEntryPoints`
|
||||
// returns several starting points, so the deep chain gets traced from inside
|
||||
// it whatever the traversal does. Measured: under breadth-first the same
|
||||
// fixture still yielded a deepest stepCount of 8, so the assertion passed
|
||||
// with the production change reverted and guarded nothing.
|
||||
//
|
||||
// What IS observable at this level is which traces survive SELECTION, and
|
||||
// that is asserted in the diversity describe below. Traversal order is
|
||||
// asserted against `traceFromEntryPoint` directly, above.
|
||||
});
|
||||
|
||||
describe('process selection diversity (R2-3)', () => {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue