fix(web): don't pin actor to FIRST when another actor feeds it#129
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
WalkthroughThis PR contains two independent changes: README landing page marketing updates including a refreshed hero tagline, colored skill badges, and quickstart timing adjustment, alongside a refinement to the ELK graph layout algorithm that conditionally pins actor nodes to the leftmost layer only when they lack incoming edges from other actors. ChangesREADME Marketing Updates
Conditional Actor Node Pinning
Sequence Diagram(s)No sequence diagram is generated for this PR, as the changes consist of documentation updates and a single conditional logic refinement without multi-component interaction flows or complex control flow sequences. Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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/src/lib/flow-layout.ts`:
- Around line 868-871: The loop over topology.displayEdges incorrectly treats
actor self-loops as incoming-from-another-actor; update the conditional inside
the for (const e of topology.displayEdges) block to also check that e.source !==
e.target before adding to incomingFromActor (i.e., only add e.target when
topology.nodeSnapshots.get(e.source)?.nodeType === 'actor' AND e.source !==
e.target), so standalone/self-loop actors are not considered incoming from
another actor.
🪄 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: d5a26904-8ed9-44ad-86c3-68f3ae121015
📒 Files selected for processing (2)
README.mdpackages/web/src/lib/flow-layout.ts
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
force-pushed
the
fix/actor-pin-collision
branch
from
f932f30 to
98376ef
Compare
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>
1 participant
You spotted this — flow `NdFjXxlxv717` (claude-sandbox step 5) had all nodes crushed together because ELK was crashing and the renderer fell back to `computeFallbackPositions` (tighter `FALLBACK_COLUMN_GAP=220` vs ELK's `200+80`).
Root cause
ELK threw:
```
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.
```
The actor-pin from #99 was unconditional. The sandbox flow has two `type: actor` nodes (`user` and `agent-cfg`) with `user → agent-cfg` between them. Both got pinned to layer 0; ELK refuses intra-layer FIRST→FIRST edges → crash → silent fallback to the tight grid layout.
Fix
Tighten the pin predicate: skip `layerConstraint=FIRST` 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.
Verified
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
Improvements