Skip to content

fix(web): orthogonal float epsilon — flat edges no longer dive past target#100

Merged
naorsabag merged 1 commit into
masterfrom
fix/orthogonal-float-epsilon
May 10, 2026
Merged

fix(web): orthogonal float epsilon — flat edges no longer dive past target#100
naorsabag merged 1 commit into
masterfrom
fix/orthogonal-float-epsilon

Conversation

@naorsabag
Copy link
Copy Markdown
Owner

@naorsabag naorsabag commented May 10, 2026

Summary

Reproduced on the orion-main flow at `/flow/eIxRYAM39kwT`: the `claude-router → mongodb` road entered the database from the top instead of the left, then dove out into empty space (the user-visible "dead end").

Root cause is sub-pixel float drift after `shiftRoutesAfterSnap`: two endpoints that should share `y` differed by ~1 ULP (3e-15). `orthogonalizeRoutePoints` used strict `!==`, declared the segment diagonal, and inserted a phantom vertical corner. `inferPortAssignmentsFromRoutes` then ran `targetHandleFromSegment(0, -3e-15)` and picked `bottom` (because `|dx|=0` is not `≥ |dy|=3e-15`), so `extendRouteToNodeCenters` shifted the endpoint upward away from the node — that's the dead-end.

Fix: 0.5px epsilon on the axis-equality test. Below that we treat the segment as already orthogonal, so float noise can't fabricate a corner.

Test plan

  • `npm test` in `packages/web` — 41/41 pass, including the new regression in `flow-layout.test.ts` for the sub-pixel-y case
  • Verified locally against the live `flow/eIxRYAM39kwT`: route now goes `claude-router:right → mongodb:left` as a single horizontal segment instead of detouring past mongodb's top edge
  • Visually confirm in `npx openhop@beta demo` after merge

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Fixed edge routing precision in flow layouts by handling sub-pixel floating-point variations to prevent phantom orthogonal corners and incorrect port assignments.
  • Tests
    • Added a regression test covering orthogonal route handling with sub-pixel mismatches to prevent future regressions.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 10, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 58bd8334-2282-4cee-a2eb-2378ee78b8de

📥 Commits

Reviewing files that changed from the base of the PR and between eb62f50 and cb9296f.

📒 Files selected for processing (2)
  • packages/web/__tests__/flow-layout.test.ts
  • packages/web/src/lib/flow-layout.ts
✅ Files skipped from review due to trivial changes (1)
  • packages/web/tests/flow-layout.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/web/src/lib/flow-layout.ts

Walkthrough

This PR addresses floating-point precision issues in orthogonal route layout by introducing an epsilon-based tolerance check. A new ORTHOGONAL_EPSILON constant prevents tiny coordinate drift from creating phantom orthogonal corners, which could otherwise mis-infer port assignments. A regression test validates the fix.

Changes

Sub-pixel Epsilon Tolerance for Orthogonal Routes

Layer / File(s) Summary
Epsilon Constant Definition
packages/web/src/lib/flow-layout.ts
Introduces ORTHOGONAL_EPSILON constant to define tolerance threshold for orthogonality checks after snap/shift operations.
Orthogonalization Logic
packages/web/src/lib/flow-layout.ts
Corner-insertion condition in orthogonalizeRoutePoints updated to use tolerance-based comparison, treating points as aligned when both x and y differences are within epsilon.
Regression Test
packages/web/__tests__/flow-layout.test.ts
New test case validates inferPortAssignmentsFromRoutes correctly handles route points with sub-pixel y-coordinate noise, asserting flat alignment and expected port assignments.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • naorsabag/OpenHop#57: Both PRs modify orthogonalization logic—main PR adds ORTHOGONAL_EPSILON to prevent sub-pixel noise from creating phantom corners, while the retrieved PR also orthogonalizes routes before port inference.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main fix: addressing floating-point epsilon tolerance in orthogonal route handling to prevent edges from extending past their target nodes.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/orthogonal-float-epsilon

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

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/web/__tests__/flow-layout.test.ts`:
- Around line 288-324: The test currently calls inferPortAssignmentsFromRoutes
with a 2-point straight route which already biases horizontal dominance and
won't reproduce the pre-fix failure; update the test to run the route through
the orthogonalization step that used to insert the phantom corner (e.g., call
shiftRoutesAfterSnap or orthogonalizeRoutes on the noisyRoute for the
topology/displayEdges before calling inferPortAssignmentsFromRoutes) so the tiny
dy drift can create the intermediate corner and the regression is exercised;
locate the test's topology, edge (topology.displayEdges[0]), noisyRoute, and
replace the direct call to inferPortAssignmentsFromRoutes with the sequence that
orthogonalizes the route first and then infers port assignments.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 352aa593-c66d-4812-84a9-3904099ab069

📥 Commits

Reviewing files that changed from the base of the PR and between 2cb0644 and eb62f50.

📒 Files selected for processing (2)
  • packages/web/__tests__/flow-layout.test.ts
  • packages/web/src/lib/flow-layout.ts

Comment thread packages/web/__tests__/flow-layout.test.ts
…hantom port

After shiftRoutesAfterSnap, two route endpoints that should share a y
coordinate can drift by ~1 ULP (~3e-15) due to float arithmetic.
orthogonalizeRoutePoints used strict \!== for axis comparison, treated
the segment as diagonal, and inserted a phantom vertical corner.
inferPortAssignmentsFromRoutes then saw a tiny non-zero dy at the end
and ran targetHandleFromSegment(0, -3e-15) → 'bottom' instead of the
true 'left' entry. The road then dove past the target node into empty
space (visible on claude-router → mongodb in the orion-main flow at
flow/eIxRYAM39kwT).

Tighten the orthogonality test with a 0.5px epsilon — anything below
that is treated as already axis-aligned, so float noise no longer
creates phantom corners or wrong port assignments.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@naorsabag naorsabag force-pushed the fix/orthogonal-float-epsilon branch from eb62f50 to cb9296f Compare May 10, 2026 06:33
@naorsabag naorsabag merged commit a149871 into master May 10, 2026
7 checks passed
naorsabag added a commit that referenced this pull request May 10, 2026
Bundles the actor-leftmost layer pin (#99), orthogonal float epsilon
(#100), and chrome z-index + monochrome SVG playback icons (#101).

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@naorsabag naorsabag deleted the fix/orthogonal-float-epsilon branch May 10, 2026 17:38
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.

1 participant