diff --git a/frontend/packages/shared/src/jobq-graph/hive-jobq-graph.js b/frontend/packages/shared/src/jobq-graph/hive-jobq-graph.js index 1d68c3bc..0a9bcbb5 100644 --- a/frontend/packages/shared/src/jobq-graph/hive-jobq-graph.js +++ b/frontend/packages/shared/src/jobq-graph/hive-jobq-graph.js @@ -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]))); }