openova/openova
1
0
Fork 0
Code Issues Pull Requests Actions 4 Packages Projects Releases Wiki Activity

fix(catalyst-ui): hard-clamp Flow node positions inside viewBox (Closes #481) (#521)

Browse Source 
Live failure on otech17/cluster-bootstrap (2026-05-01): the JobDetail
flow canvas rendered as yellow horizontal lines with zero visible
bubbles. Investigation showed nodes drifted to x=30,400+ in viewBox
coordinates because the dependency graph had longest-path depth ~190
(bp-* leaves chained through "applications"). At PER_DEPTH_X=160 that
placed nodes far outside the MAX_VBOX_W=1200 ceiling. The viewBox
captured only a 1200px slice of a 30,000px cluster, so 99% of bubbles
rendered off-canvas. The few yellow lines visible were edges from the
selection job (openJobId) that happened to cross the visible window.

Pre-existing bounded tests modelled depth=0/1 stars only (#486 #499) so
this pathology slipped through.

Operator's two explicit asks for this fix:

  1. "No single bubble could be outside of the canvas."
  2. "Max distance of a line cannot be longer than a percentage of canvas."

Implementation — Constraint A + Constraint B as a render-time projection:

* Compute the natural cluster bbox from livePos as before, clamp to
  MIN/MAX viewBox.
* When natural bbox exceeds the viewBox, anchor vbX/vbY at the
  left-most / top-most cluster point (instead of centring on the
  cluster centroid which placed depth 0 at x=-15,000).
* Linear-scale every render position so the cluster fits inside an
  inset rectangle (vbX+CLAMP_INSET .. vbX+vbW-CLAMP_INSET).
  Pathological depth=190 chains compress to fit; sparse graphs with
  scale=1 are unchanged.
* Hard-clamp every position into the inset rectangle as a final safety
  net (FP drift, partial-tick frames). No bubble can ever sit outside.
* Edges read renderPos so they're drawn between already-clamped
  endpoints — line length is bounded by the viewBox diagonal, no
  "kilometers of edges" possible regardless of what the simulation
  produces.

Test:

* New `keeps every bubble inside the viewBox for a deep dependency
  chain` — 50-node depth chain (each at depth=i, mirroring production
  shape). Asserts every centroid inside [vbX, vbX+vbW] × [vbY, vbY+vbH]
  AND every line length <= viewBox diagonal. Strict — no overshoot
  tolerance. Fails on main, passes after the fix.
* All 11 pre-existing bounded tests still pass; tsc clean.

Live verification + Playwright screenshot to follow on the deployed SHA.

Co-authored-by: alierenbaysal <alierenbaysal@noreply.github.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
e3mrah 2026-05-02 09:04:37 +04:00 committed by GitHub
parent 8ee647a21c
commit 7acd7d720d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 193 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

105
products/catalyst/bootstrap/ui/src/pages/sovereign/FlowCanvasOrganic.bounded.test.tsx
View File

@ -302,6 +302,111 @@ describe('FlowCanvasOrganic — Bug #481 bounded layout', () => {
},
)
/* ── Bug #481 (reopened 2026-05-01) — deep-chain production shape ──
*
* Live failure on otech17/cluster-bootstrap: nodes drifted to
* x≈30,560 because the dependency graph had longest-path depth ~190
* (bp-* leaves chained through "applications"). At PER_DEPTH_X=160,
* that placed nodes at depth*160=30,400 — far outside the
* MAX_VBOX_W=1200 ceiling. The viewBox showed only a 1200px slice
* of a 30,000px cluster, so 99% of bubbles rendered off-canvas.
* Operator saw yellow horizontal lines (the few edges that crossed
* the visible window) and zero bubbles.
*
* Pre-existing bounded tests modelled depth=0/1 stars only, so this
* pathology slipped through. Lock it in.
*/
it('keeps every bubble inside the viewBox for a deep dependency chain (production shape, #481 reopen)', () => {
const CHAIN = 50
const nodes = Array.from({ length: CHAIN }, (_, i) => ({
id: `n${i}`,
depth: i,
regionId: 'primary',
familyId: 'catalyst',
label: `Node ${i}`,
subLabel: '',
status: 'pending' as const,
isGroup: false,
isFolded: false,
childCount: 0,
job: {
id: `n${i}`,
jobName: `n${i}`,
type: 'install' as const,
appId: 'x',
parentId: '',
dependsOn: i > 0 ? [`n${i - 1}`] : [],
childIds: [],
status: 'pending' as const,
startedAt: null,
finishedAt: null,
durationMs: 0,
},
}))
const edges = nodes.slice(1).map((n, i) => ({
fromId: `n${i}`,
toId: n.id,
fromStatus: 'pending' as const,
crossRegion: false,
kind: 'depends-on' as const,
}))
const layout: OrganicLayoutResult = {
nodes,
edges,
maxDepth: CHAIN - 1,
regions: REGIONS,
families: FAMILIES,
}
const { container } = render(
<FlowCanvasOrganic
layout={layout}
openJobId={null}
hostJobId={null}
onJobClick={() => {}}
onJobDoubleClick={() => {}}
onCanvasBackgroundClick={() => {}}
/>,
)
const svg = container.querySelector<SVGSVGElement>(
'[data-testid="flow-canvas-svg"]',
)!
const vb = svg.getAttribute('viewBox') ?? ''
const [vbX, vbY, vbW, vbH] = vb.split(/\s+/).map(Number)
expect(vbW).toBeLessThanOrEqual(1200)
expect(vbH).toBeLessThanOrEqual(700)
const groups = container.querySelectorAll<SVGGElement>(
'[data-flow-draggable]',
)
expect(groups.length).toBe(CHAIN)
// Operator's exact request: "no single bubble could be outside of
// the canvas". Strict — every centroid inside [vbX, vbX+vbW] ×
// [vbY, vbY+vbH] regardless of how pathological the depth chain.
for (const g of Array.from(groups)) {
const t = g.getAttribute('transform') ?? ''
const m = t.match(/translate\(([-\d.]+),\s*([-\d.]+)\)/)
expect(m).not.toBeNull()
const x = Number(m![1])
const y = Number(m![2])
expect(x).toBeGreaterThanOrEqual(vbX)
expect(x).toBeLessThanOrEqual(vbX + vbW)
expect(y).toBeGreaterThanOrEqual(vbY)
expect(y).toBeLessThanOrEqual(vbY + vbH)
}
// Operator's second rule: "max distance of a line cannot be longer
// than a percentage of canvas". With render-time clamping into the
// viewBox, every line is at most the viewBox diagonal — structural,
// not flaky.
const diagonal = Math.hypot(vbW, vbH)
const lines = container.querySelectorAll<SVGLineElement>('line')
for (const ln of Array.from(lines)) {
const x1 = Number(ln.getAttribute('x1') ?? '0')
const y1 = Number(ln.getAttribute('y1') ?? '0')
const x2 = Number(ln.getAttribute('x2') ?? '0')
const y2 = Number(ln.getAttribute('y2') ?? '0')
expect(Math.hypot(x2 - x1, y2 - y1)).toBeLessThanOrEqual(diagonal)
}
})
it('keeps every bubble visible (radius ≥40) and viewBox bounded for a 60-node graph', () => {
const layout = makeLayout(60)
const { container } = render(

104
products/catalyst/bootstrap/ui/src/pages/sovereign/FlowCanvasOrganic.tsx
View File

@ -521,6 +521,32 @@ export function FlowCanvasOrganic(props: FlowCanvasOrganicProps) {
}
}
// Bug #481 (reopened 2026-05-01) — operator's exact words:
// "they could have put a simple rule saying the max distance of a
// line cannot be longer than a percentage of canvas, or they could
// say no single bubble could be outside of the canvas etc."
//
// Live failure on otech17: a single dependency chain reached depth
// 190 (e.g. bp-catalyst-platform → bp-langfuse → ... long chain). At
// PER_DEPTH_X=160 that placed leaves at x=30,400+. Per-tick clamps
// bound nodes around their own depth-anchor (which itself was at
// 30,400) so they never came back into view — and the viewBox
// ceiling MAX_VBOX_W=1200 captured only a 1200px slice of a 30,000px
// cluster. The yellow horizontal lines on screen were the few edges
// that happened to cross the visible 1200px window; every bubble was
// off-canvas.
//
// The bounded tests passed because they asserted positions inside
// the viewBox for 5-80 sibling stars, but never modelled a deep
// chain (the actual production shape).
//
// Structural fix per operator's ask: "no single bubble could be
// outside of the canvas". After the natural bbox is computed, clamp
// it to MAX so the viewBox stays bounded, then HARD-CLAMP every
// rendered position into the viewBox. Edges are then drawn between
// already-clamped positions, so the second rule ("max line length")
// is also bounded as a side effect (any link is at most the
// viewBox's diagonal).
let bbMinX = Infinity, bbMinY = Infinity, bbMaxX = -Infinity, bbMaxY = -Infinity
for (const p of livePos.values()) {
if (p.x < bbMinX) bbMinX = p.x
@ -533,21 +559,62 @@ export function FlowCanvasOrganic(props: FlowCanvasOrganicProps) {
const PAD_Y_BOTTOM = NODE_RADIUS + 40
const naturalW = (bbMaxX - bbMinX) + PAD_X * 2
const naturalH = (bbMaxY - bbMinY) + PAD_Y_TOP + PAD_Y_BOTTOM
// Bug #481 — clamp the viewBox between MIN and MAX so:
// • Tiny graphs (4-6 nodes) don't squeeze into a microscopic cluster
// (MIN floor keeps them readable at ≥120px wide each).
// • Pathological / runaway graphs don't force the SVG to scale
// down so far the nodes turn into specks (MAX ceiling).
// The MAX ceiling is the operative half of this clamp for #481 — it
// is the structural answer to "kilometers of edges between tiny
// nodes": even if the simulation drifts, the visible area never
// exceeds MAX, so nodes always render at a usable size.
const vbW = Math.min(MAX_VBOX_W, Math.max(MIN_VBOX_W, naturalW))
const vbH = Math.min(MAX_VBOX_H, Math.max(MIN_VBOX_H, naturalH))
const cx = Number.isFinite(bbMinX) ? (bbMinX + bbMaxX) / 2 : vbW / 2
const cy = Number.isFinite(bbMinY) ? (bbMinY + bbMaxY) / 2 : vbH / 2
const vbX = cx - vbW / 2
const vbY = cy - vbH / 2
// Bug #481 — when the natural bbox is wider than MAX_VBOX, anchor
// the viewBox at the LEFT-MOST cluster point (depth 0) instead of
// centring on the cluster centroid. Centring put depth 0 at
// x=-15,000 off-canvas; left-anchor keeps the visible 1200px slice
// starting at the actual left edge of the data, where the operator
// expects depth-0 root nodes to live.
const naturalCx = Number.isFinite(bbMinX) ? (bbMinX + bbMaxX) / 2 : vbW / 2
const naturalCy = Number.isFinite(bbMinY) ? (bbMinY + bbMaxY) / 2 : vbH / 2
const vbX = naturalW > MAX_VBOX_W && Number.isFinite(bbMinX)
? bbMinX - PAD_X
: naturalCx - vbW / 2
const vbY = naturalH > MAX_VBOX_H && Number.isFinite(bbMinY)
? bbMinY - PAD_Y_TOP
: naturalCy - vbH / 2
// Bug #481 — Constraint A: hard-clamp every render position into the
// viewBox so no bubble ever drifts off-canvas, regardless of what the
// simulation, depth-anchor, or grid-target produced. Operator's exact
// request: "no single bubble could be outside of the canvas".
//
// For pathological-width clusters (depth-190 chains with naturalW
// ≈ 30,000 vs MAX_VBOX_W=1200) plain clamping would pile every
// distant node on the right edge. Solve that by scaling the X axis
// proportionally to fit the viewBox, then clamping as a final
// safety net. Y gets the same treatment.
const CLAMP_INSET = NODE_RADIUS + 8
const usableW = vbW - CLAMP_INSET * 2
const usableH = vbH - CLAMP_INSET * 2
const xScale = naturalW > vbW && (bbMaxX - bbMinX) > 0
? usableW / (bbMaxX - bbMinX)
: 1
const yScale = naturalH > vbH && (bbMaxY - bbMinY) > 0
? usableH / (bbMaxY - bbMinY)
: 1
const project = (p: { x: number; y: number }) => {
let x = p.x
let y = p.y
if (xScale < 1 && Number.isFinite(bbMinX)) {
x = vbX + CLAMP_INSET + (p.x - bbMinX) * xScale
}
if (yScale < 1 && Number.isFinite(bbMinY)) {
y = vbY + CLAMP_INSET + (p.y - bbMinY) * yScale
}
// Final safety clamp — even when scaling, FP drift / partial-tick
// values can land a fraction outside; the inset clamp guarantees
// the bubble's full diameter stays visible.
x = Math.min(vbX + vbW - CLAMP_INSET, Math.max(vbX + CLAMP_INSET, x))
y = Math.min(vbY + vbH - CLAMP_INSET, Math.max(vbY + CLAMP_INSET, y))
return { x, y }
}
const renderPos = new Map<string, { x: number; y: number }>()
for (const [id, p] of livePos) {
renderPos.set(id, project(p))
}
return (
<svg
@ -605,8 +672,11 @@ export function FlowCanvasOrganic(props: FlowCanvasOrganicProps) {
</defs>
{layout.edges.map((e) => {
const s = livePos.get(e.fromId)
const t = livePos.get(e.toId)
// Bug #481 — use clamped renderPos, not raw livePos. Edges
// between clamped endpoints are bounded by the viewBox
// diagonal so "kilometers of edges" is structurally impossible.
const s = renderPos.get(e.fromId)
const t = renderPos.get(e.toId)
if (!s || !t) return null
const onSelectionPath =
openJobId !== null && (e.fromId === openJobId || e.toId === openJobId)
@ -625,7 +695,9 @@ export function FlowCanvasOrganic(props: FlowCanvasOrganicProps) {
})}
{layout.nodes.map((node) => {
const pos = livePos.get(node.id)
// Bug #481 — render at clamped position so no bubble ever sits
// outside the viewBox.
const pos = renderPos.get(node.id)
if (!pos) return null
const family = familyById.get(node.familyId) ?? null
const isNeighbor = neighborIds.has(node.id)