Skip to content

fix(web): pin actor nodes to leftmost layer#99

Merged
naorsabag merged 1 commit into
masterfrom
fix/actor-leftmost-layer
May 10, 2026
Merged

fix(web): pin actor nodes to leftmost layer#99
naorsabag merged 1 commit into
masterfrom
fix/actor-leftmost-layer

Conversation

@naorsabag
Copy link
Copy Markdown
Owner

@naorsabag naorsabag commented May 10, 2026

Summary

  • ELK could place actor nodes (the human/external initiator) at any layer that minimized total edge length, sometimes pushing user / browser inward when it also appeared as a final response target.
  • Added elk.layered.layering.layerConstraint=FIRST for type: actor nodes so the actor always anchors the leftmost column.
  • New test in __tests__/flow-layout.test.ts asserts that user lands at min(x) across the order-flow example layout.

Test plan

  • npm test in packages/web (41/41 passing — new test included)
  • Visually verify npx openhop@beta demo after merge: browser on far left of the auth flow

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Fixed flow layout behavior to ensure actor-typed nodes are correctly positioned at the leftmost layer in flow diagrams, even when they appear as edge targets. Improved per-node layout options computation for more accurate positioning.
  • Tests

    • Added test validation for flow layout behavior to ensure consistent positioning of actor nodes.

Review Change Stack

ELK's layered algorithm placed the actor (e.g. `user` / `browser`) at
whichever layer minimized total edge length, which could push it inward
when it had both outgoing and incoming traffic. Set
`elk.layered.layering.layerConstraint=FIRST` for actor-typed nodes so
the human/external initiator always reads as the leftmost column.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@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: 7c742db4-97df-42ea-8f1e-0c0cbec2e60f

📥 Commits

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

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

Walkthrough

This PR modifies ELK graph layout computation to pin actor-typed nodes to the leftmost layer by conditionally adding layer constraints. It refactors per-node layout options configuration and adds a test verifying actor nodes remain at the minimum x-coordinate.

Changes

Actor Node Layer Pinning

Layer / File(s) Summary
ELK Layout Configuration
packages/web/src/lib/flow-layout.ts
buildElkGraph now conditionally applies elk.layered.layering.layerConstraint: 'FIRST' to actor-typed nodes. portConstraints: 'FIXED_SIDE' is added only when portAssignments exist. The returned node structure conditionally spreads both layoutOptions and ports.
Layout Verification Test
packages/web/__tests__/flow-layout.test.ts
New test case verifies computeElkLayout pins the user actor node to the leftmost layer by asserting its x position equals the global minimum x coordinate.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 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 change: pinning actor nodes to the leftmost layer in ELK layout, which matches the core implementation in flow-layout.ts and is validated by the new test.
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/actor-leftmost-layer

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

@naorsabag naorsabag merged commit 863e3e3 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/actor-leftmost-layer branch May 10, 2026 17:38
naorsabag added a commit that referenced this pull request May 11, 2026
Flows with two actors connected to each other (e.g. user → agent-cfg,
both type=actor) crashed ELK with:

  org.eclipse.elk.core.UnsupportedConfigurationException: Node
  'openhop-flow.user' has its layer constraint set to FIRST, but has
  at least one incoming edge that does not come from a FIRST_SEPARATE
  node.

ELK rejects intra-layer FIRST→FIRST edges. The actor-pin from #99 was
unconditional: every type=actor node got layerConstraint=FIRST. When
two actors connect, both land in layer 0 and ELK refuses to lay it
out. useFlowGraphLayout silently catches and falls back to
computeFallbackPositions, which uses tighter FALLBACK_COLUMN_GAP=220
vs ELK's 200+80 — the visible result was "all the nodes are too close
together" on the affected flows.

Tighten the pin predicate: skip the FIRST constraint only when the
actor is fed by ANOTHER actor. The common case where an actor is the
final response target (e.g. user ← api in orion-main) still pins
correctly because the response edge's source is an endpoint/service,
not an actor — so user stays leftmost.

Tested against:
- examples/order-flow (1 actor, response loop) — user is leftmost ✓
- claude-sandbox flow NdFjXxlxv717 (2 actors connected) — ELK
  no longer crashes, gaps are 200/80 px as expected ✓

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
naorsabag added a commit that referenced this pull request May 11, 2026
* fix(web): don't pin actor to FIRST when another actor feeds it

Flows with two actors connected to each other (e.g. user → agent-cfg,
both type=actor) crashed ELK with:

  org.eclipse.elk.core.UnsupportedConfigurationException: Node
  'openhop-flow.user' has its layer constraint set to FIRST, but has
  at least one incoming edge that does not come from a FIRST_SEPARATE
  node.

ELK rejects intra-layer FIRST→FIRST edges. The actor-pin from #99 was
unconditional: every type=actor node got layerConstraint=FIRST. When
two actors connect, both land in layer 0 and ELK refuses to lay it
out. useFlowGraphLayout silently catches and falls back to
computeFallbackPositions, which uses tighter FALLBACK_COLUMN_GAP=220
vs ELK's 200+80 — the visible result was "all the nodes are too close
together" on the affected flows.

Tighten the pin predicate: skip the FIRST constraint only when the
actor is fed by ANOTHER actor. The common case where an actor is the
final response target (e.g. user ← api in orion-main) still pins
correctly because the response edge's source is an endpoint/service,
not an actor — so user stays leftmost.

Tested against:
- examples/order-flow (1 actor, response loop) — user is leftmost ✓
- claude-sandbox flow NdFjXxlxv717 (2 actors connected) — ELK
  no longer crashes, gaps are 200/80 px as expected ✓

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(web): exclude actor self-loops from the incoming-from-actor guard

CodeRabbit on #129: an actor self-loop (source === target) was
incorrectly treated as 'fed by another actor', which would unpin a
standalone actor that happens to have a self-loop step. Self-loops
don't introduce a second FIRST-pinned node, so they can't trigger
the ELK FIRST-FIRST intra-layer error this guard exists to prevent.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* ci: prettier-format flow-layout.ts

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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