Skip to content

Conversation

@HimethW
Copy link

@HimethW HimethW commented Feb 12, 2026

Goals

Fixed issues regarding the layout structure

Approach

avoid node collision
Bridges at line intersections
label styling fixed
portal creation logic fixed

@HimethW HimethW requested a review from hevayo as a code owner February 12, 2026 05:07
Copilot AI review requested due to automatic review settings February 12, 2026 05:07
@HimethW HimethW requested a review from gigara as a code owner February 12, 2026 05:07
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 12, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR aims to fix multiple layout/rendering issues in the Arazzo designer visualizer’s vertical layout: reducing node/edge collisions, adding bridge rendering at line intersections, and improving label placement/styling. It also includes a change to portal creation logic for backward jumps.

Changes:

  • Adjust vertical layout spacing constants and retry/condition branch positioning.
  • Improve edge routing/collision avoidance and add “bridge” rendering for orthogonal edge intersections.
  • Update portal creation rule for vertical-layout backward jumps and tweak label truncation/positioning.

Reviewed changes

Copilot reviewed 9 out of 15 changed files in this pull request and generated 13 comments.

Show a summary per file
File Description
workspaces/arazzo/arazzo-designer-visualizer/src/visitors/PositionVisitorVertical_v2.ts Adds special Y placement for RETRY branch heads in vertical positioning.
workspaces/arazzo/arazzo-designer-visualizer/src/visitors/PortalCreator_v2.ts Broadens portal creation for backward jumps in vertical layout.
workspaces/arazzo/arazzo-designer-visualizer/src/visitors/NodeFactoryVisitorVertical_v2.ts Updates handle selection and waypoint routing; introduces shared geometry utilities and shift logic for collision avoidance.
workspaces/arazzo/arazzo-designer-visualizer/src/constants/nodeConstants.ts Tweaks spacing constants and introduces label-related constants.
workspaces/arazzo/arazzo-designer-visualizer/src/components/edges/edgeUtils.ts New shared geometry helpers for segment/rectangle intersection checks.
workspaces/arazzo/arazzo-designer-visualizer/src/components/edges/edgeIntersectDetect.ts New helpers to detect intersections and generate SVG bridge arcs.
workspaces/arazzo/arazzo-designer-visualizer/src/components/edges/WaypointCreator.ts Adds optional shift amount to adjust skip routing column and updates waypoint offsets.
workspaces/arazzo/arazzo-designer-visualizer/src/components/edges/PlannedPathEdge.tsx Adds bridge-aware segment rendering and improves label positioning/truncation.
workspaces/arazzo/arazzo-designer-extension/src/extension.ts Changes language server binary naming logic.
common/config/rush/pnpm-lock.yaml Dependency lock updates.
Files not reviewed (1)
  • common/config/rush/pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

nextY_RETRY = nodeY + node.viewState.h + C.RETRY_GAP_Y_ConditionBranch;
} else {
nextY = nodeY + node.viewState.h + C.NODE_GAP_Y_Vertical;
//nextY_RETRY = nextY;
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nextY_RETRY is declared as a number but is only assigned inside the node.type === 'CONDITION' branch. It’s later used when positioning branch heads, which will trigger a TS definite-assignment error (and could yield undefined at runtime if branches exist on a non-CONDITION node). Initialize nextY_RETRY to a safe default (e.g., same as nextY) in the non-CONDITION branch or immediately after computing nextY.

Suggested change
//nextY_RETRY = nextY;
nextY_RETRY = nextY;

Copilot uses AI. Check for mistakes.
Comment on lines 218 to 234
const findshifts = (sourcePt: {x:number,y:number}, targetPt: {x:number,y:number}): number => {
let shifts = 0;
while (true) {
for (const nodePos of this.allNodePositions) {
if (nodePos.id === source.id || nodePos.id === target.id) continue;
const rect = { x: nodePos.x, y: nodePos.y, w: nodePos.w, h: nodePos.h };
if (segmentIntersectsRect(sourcePt, targetPt, rect)) {
shifts++;
sourcePt.x += C.NODE_WIDTH;
targetPt.x += C.NODE_WIDTH;
break;
}
}
return shifts;
}

}
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

findshifts currently returns shifts unconditionally at the end of the first while(true) iteration, so it can never perform more than one shift even when the shifted segment still intersects another node. This makes the collision-avoidance logic ineffective for multiple blockers. The return should only happen once a full pass finds no intersections (or the loop should be restructured to break/continue appropriately).

Copilot uses AI. Check for mistakes.
Comment on lines +1 to +2
import type { Point as P } from './WaypointCreator';

Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused import: import type { Point as P } from './WaypointCreator'; is not referenced in this module. Please remove it to avoid confusion and keep the module minimal.

Suggested change
import type { Point as P } from './WaypointCreator';

Copilot uses AI. Check for mistakes.
Comment on lines 24 to 26
// Column: move right by approximately 1.5 * block.w from block.x
const columnX = block.x + block.w * C.WAYPOINT_SKIP_HORIZONTAL_OFFSET_MULTIPLIER;
const columnX = block.x + block.w + C.WAYPOINT_SKIP_HORIZONTAL_OFFSET + (C.NODE_WIDTH + C.NODE_GAP_X_Vertical) * (shiftAmount || 0);

Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment still says the routing column is "~1.5 * block.w", but the new columnX calculation is block.w + WAYPOINT_SKIP_HORIZONTAL_OFFSET + .... Update the comment to reflect the current formula so future adjustments don’t rely on outdated assumptions.

Copilot uses AI. Check for mistakes.
import { Node, Edge, MarkerType } from '@xyflow/react';
import * as C from '../constants/nodeConstants';
import WaypointCreator from '../components/edges/WaypointCreator';
import { pointInRect, segIntersectsSeg, segmentIntersectsRect } from '../components/edges/edgeUtils';;
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is an extra semicolon at the end of this import (...edgeUtils';;). This is harmless at runtime but will typically fail lint/style checks; remove the duplicate ;.

Suggested change
import { pointInRect, segIntersectsSeg, segmentIntersectsRect } from '../components/edges/edgeUtils';;
import { pointInRect, segIntersectsSeg, segmentIntersectsRect } from '../components/edges/edgeUtils';

Copilot uses AI. Check for mistakes.
Comment on lines +78 to +99
// Gather other edges & nodes once (exclude current).
// NOTE: React Flow may not populate sourceX/targetX on edges returned by
// `useEdges()` in some render passes — fall back to node positions.
const allEdges = useEdges();
const otherEdges = (allEdges || []).filter(e => e.id !== id);
// Get nodes from React Flow so we can reconstruct edge endpoints when
// `sourceX/sourceY` or `targetX/targetY` are not available on the edge object.
const nodes = useNodes() as any[] || [];
const nodeById = new Map((nodes || []).map((n: any) => [n.id, n]));

// Forwarding wrapper that uses the centralized helper (keeps behaviour)
const getPointsForEdge = (e: any) => getEdgePoints(e, nodeById);

// Emit a straight segment but inject bridge arcs when crossing
// orthogonal segments from other edges (supports vertical *and* horizontal).
let pen = { x: points[0].x, y: points[0].y };
const emitLineWithBridges = (from: { x: number; y: number }, to: { x: number; y: number }) => {
const intersections = detectBridgesForSegment(from, to, otherEdges as any[], nodeById, BRIDGE_RADIUS, DEFAULT_EPS);
const frag = buildSegmentPathWithBridges(from, to, intersections, BRIDGE_RADIUS);
pen = { x: to.x, y: to.y };
return frag;
};
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bridge detection recomputes intersections by scanning all other edges for every straight segment, on every render of each edge. This is O(E² × segments) work and can become a bottleneck on larger graphs. Consider memoizing otherEdges/nodeById and/or caching the detected intersections per edge/segment (e.g., via useMemo) so the path doesn’t fully recompute unless edges/nodes/waypoints actually change.

Copilot uses AI. Check for mistakes.
Comment on lines +88 to +90
// Forwarding wrapper that uses the centralized helper (keeps behaviour)
const getPointsForEdge = (e: any) => getEdgePoints(e, nodeById);

Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused variable getPointsForEdge.

Suggested change
// Forwarding wrapper that uses the centralized helper (keeps behaviour)
const getPointsForEdge = (e: any) => getEdgePoints(e, nodeById);

Copilot uses AI. Check for mistakes.
): DetectedIntersection[] {
const dx = to.x - from.x;
const dy = to.y - from.y;
const isVertical = Math.abs(dx) <= eps && Math.abs(dy) > eps;
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused variable isVertical.

Suggested change
const isVertical = Math.abs(dx) <= eps && Math.abs(dy) > eps;

Copilot uses AI. Check for mistakes.
import { Node, Edge, MarkerType } from '@xyflow/react';
import * as C from '../constants/nodeConstants';
import WaypointCreator from '../components/edges/WaypointCreator';
import { pointInRect, segIntersectsSeg, segmentIntersectsRect } from '../components/edges/edgeUtils';;
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused imports pointInRect, segIntersectsSeg.

Suggested change
import { pointInRect, segIntersectsSeg, segmentIntersectsRect } from '../components/edges/edgeUtils';;
import { segmentIntersectsRect } from '../components/edges/edgeUtils';;

Copilot uses AI. Check for mistakes.
return shifts;
}

}
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid automated semicolon insertion (97% of all statements in the enclosing function have an explicit semicolon).

Suggested change
}
};

Copilot uses AI. Check for mistakes.
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 12 out of 18 changed files in this pull request and generated 4 comments.

Files not reviewed (1)
  • common/config/rush/pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

const isTargetRight = target.viewState.x > source.viewState.x + source.viewState.w;
const isTargetRight = target.viewState.x + target.viewState.w/2 > source.viewState.x + source.viewState.w/2;
const isTargetAbove = target.viewState.y < source.viewState.y;
const isTargetLeft = target.viewState.x + target.viewState.w/2 < source.viewState.x + source.viewState.w/2; //add margeins
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo in comment: //add margeins should be // add margins.

Suggested change
const isTargetLeft = target.viewState.x + target.viewState.w/2 < source.viewState.x + source.viewState.w/2; //add margeins
const isTargetLeft = target.viewState.x + target.viewState.w/2 < source.viewState.x + source.viewState.w/2; // add margins

Copilot uses AI. Check for mistakes.
Comment on lines 505 to 510
specifier: 5.8.3
version: 5.8.3
webpack:
specifier: ^5.94.0
specifier: ^5.88.2
version: 5.104.1([email protected])
webpack-cli:
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This lockfile hunk shows tooling dependency specifier/version changes (e.g., webpack specifier changes, and multiple version downgrades). These look unrelated to the PR’s stated layout goals; if they’re from an accidental lockfile regen, consider reverting/minimizing them or documenting why the dependency changes are needed, since they can impact builds across the repo.

Copilot uses AI. Check for mistakes.
}else if(scenario === 'branch') {
// For branches, add a slight horizontal offset to the label position to avoid overlap with the node
computedWaypoints = WaypointCreator(sourcePt, targetPt, foundBlockingRect, 'branch');
computedWaypoints = WaypointCreator(sourcePt, targetPt, foundBlockingRect, 'branch',);
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The WaypointCreator(...) call has an extra trailing comma with no argument: WaypointCreator(sourcePt, targetPt, foundBlockingRect, 'branch',);. This is a syntax error and will fail to compile. Remove the extra comma or pass the intended 5th argument.

Suggested change
computedWaypoints = WaypointCreator(sourcePt, targetPt, foundBlockingRect, 'branch',);
computedWaypoints = WaypointCreator(sourcePt, targetPt, foundBlockingRect, 'branch');

Copilot uses AI. Check for mistakes.
}
}

}
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid automated semicolon insertion (97% of all statements in the enclosing function have an explicit semicolon).

Suggested change
}
};

Copilot uses AI. Check for mistakes.
@axewilledge axewilledge merged commit d450d3a into wso2:arazzo-extension Feb 12, 2026
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants