jobq-graph: scope dep-edge reorder per connected component
Fixes a real bug argus caught: the flat batch-round topo sort could interleave two fully independent dep pairs in the same sibling list (e.g. W, X after_ok(W), Y, Z after_ok(Y) reordered to W, Y, X, Z), and the single-column rail then drew one continuous line across rows that have no relationship at all. Reorder is now scoped per connected component of the local dependency graph -- each component renders as a contiguous block (first-seen order, so an already-correct list doesn't reorder needlessly), so two unrelated pairs can never weave together. Within one component, overlapping ranges are still correct: they mean the nodes really are related (a diamond, for instance). Also: a pass-through row's tooltip now names what's passing through it (not just the edge it's itself an endpoint of) -- addresses the same disambiguation gap argus flagged as a secondary note. Re-verified against a wider fixture set including the exact interleaving case from the review (17 checks: prior 6 unaffected + argus's regression case, a shuffled-order variant, three simultaneous independent pairs, and a genuine diamond that's expected to overlap).
This commit is contained in:
parent
86a7a62519
commit
6f87821110
1 changed files with 62 additions and 20 deletions
|
|
@ -57,16 +57,26 @@ function buildTree(nodes) {
|
|||
|
||||
// A `Node`-kind dep only ever names a sibling under the same parent (product
|
||||
// decision on the dep-edge-visibility issue — nothing crosses a group
|
||||
// boundary), so a
|
||||
// dependency edge is always local to one sibling list. This reorders that
|
||||
// list so a dependency always renders before what depends on it (stable:
|
||||
// ties keep original array order, which is already root-then-subtree order),
|
||||
// and returns the edges as index ranges into the *new* order for the
|
||||
// boundary), so a dependency edge is always local to one sibling list. This
|
||||
// reorders that list so a dependency always renders before what depends on
|
||||
// it, and returns the edges as index ranges into the *new* order for the
|
||||
// gutter-rail renderer below.
|
||||
//
|
||||
// O(n^2) worst case (each pass rescans the remaining nodes) — fine here,
|
||||
// sibling-list sizes are small (tens, not thousands) and this only runs once
|
||||
// per render, not per frame.
|
||||
// Ordering is scoped **per connected component** of the local dependency
|
||||
// graph, not one flat topo sort over the whole list — two independent dep
|
||||
// pairs (no edge relates them, directly or transitively) must never end up
|
||||
// interleaved, or the single-column rail below would draw one continuous
|
||||
// line across both and imply a relationship that doesn't exist. Emitting
|
||||
// each component as a contiguous block (in first-seen order, so an already-
|
||||
// correct list doesn't reorder unnecessarily) keeps every edge's [lo, hi]
|
||||
// span either fully inside its own component's block or, within a
|
||||
// component, genuinely overlapping because the nodes really are related
|
||||
// (e.g. a diamond: two siblings both depending on the same third one).
|
||||
//
|
||||
// O(n^2) worst case (component discovery + each component's own topo sort
|
||||
// rescans its remaining members per pass) — fine here, sibling-list sizes
|
||||
// are small (tens, not thousands) and this runs once per render, not per
|
||||
// frame.
|
||||
function orderSiblings(list) {
|
||||
if (list.length < 2) return { order: list, ranges: [] };
|
||||
const idx = new Map(list.map((n, i) => [n.id, i]));
|
||||
|
|
@ -78,20 +88,45 @@ function orderSiblings(list) {
|
|||
.filter((d) => d.kind === 'Node' && idx.has(d.id))
|
||||
.map((d) => idx.get(d.id)),
|
||||
);
|
||||
const placed = new Array(list.length).fill(false);
|
||||
const orderIdx = [];
|
||||
let remaining = list.map((_, i) => i);
|
||||
while (remaining.length) {
|
||||
const ready = remaining.filter((i) => localDeps[i].every((d) => placed[d]));
|
||||
// A cycle can't happen from a well-formed graph, but if it ever does,
|
||||
// dump whatever's left in original order rather than looping forever.
|
||||
const take = ready.length ? ready : remaining;
|
||||
for (const i of take) {
|
||||
orderIdx.push(i);
|
||||
placed[i] = true;
|
||||
|
||||
const adjacency = list.map(() => []);
|
||||
localDeps.forEach((deps, i) => {
|
||||
for (const d of deps) { adjacency[i].push(d); adjacency[d].push(i); }
|
||||
});
|
||||
const componentOf = new Array(list.length).fill(-1);
|
||||
let numComponents = 0;
|
||||
for (let start = 0; start < list.length; start++) {
|
||||
if (componentOf[start] !== -1) continue;
|
||||
const stack = [start];
|
||||
componentOf[start] = numComponents;
|
||||
while (stack.length) {
|
||||
const i = stack.pop();
|
||||
for (const j of adjacency[i]) {
|
||||
if (componentOf[j] === -1) { componentOf[j] = numComponents; stack.push(j); }
|
||||
}
|
||||
}
|
||||
remaining = remaining.filter((i) => !placed[i]);
|
||||
numComponents++;
|
||||
}
|
||||
|
||||
const orderIdx = [];
|
||||
const emitted = new Array(list.length).fill(false);
|
||||
for (let start = 0; start < list.length; start++) {
|
||||
if (emitted[start]) continue;
|
||||
const members = [];
|
||||
for (let i = 0; i < list.length; i++) if (componentOf[i] === componentOf[start]) members.push(i);
|
||||
const placed = new Set();
|
||||
let remaining = members;
|
||||
while (remaining.length) {
|
||||
const ready = remaining.filter((i) => localDeps[i].every((d) => placed.has(d)));
|
||||
// A cycle can't happen from a well-formed graph, but if it ever
|
||||
// does, dump whatever's left of this component in original order
|
||||
// rather than looping forever.
|
||||
const take = ready.length ? ready : remaining;
|
||||
for (const i of take) { orderIdx.push(i); placed.add(i); emitted[i] = true; }
|
||||
remaining = remaining.filter((i) => !placed.has(i));
|
||||
}
|
||||
}
|
||||
|
||||
const posOf = new Array(list.length);
|
||||
orderIdx.forEach((origIdx, pos) => { posOf[origIdx] = pos; });
|
||||
const ranges = [];
|
||||
|
|
@ -171,9 +206,16 @@ function renderGroup(list) {
|
|||
bottom[lo] = true;
|
||||
top[hi] = true;
|
||||
dot[hi] = true;
|
||||
for (let k = lo + 1; k < hi; k++) { top[k] = true; bottom[k] = true; }
|
||||
addTitle(hi, 'waits on: ' + order[lo].payload.label);
|
||||
addTitle(lo, 'blocks: ' + order[hi].payload.label);
|
||||
for (let k = lo + 1; k < hi; k++) {
|
||||
top[k] = true;
|
||||
bottom[k] = true;
|
||||
// This row isn't itself either end of the edge, just sitting between
|
||||
// them in render order — say what's passing through so the rail
|
||||
// doesn't read as an unexplained line.
|
||||
addTitle(k, order[lo].payload.label + ' → ' + order[hi].payload.label + ' passes through here');
|
||||
}
|
||||
}
|
||||
return order.map((n, i) => renderNode(n, renderRail(top[i], bottom[i], dot[i], titles[i])));
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue