Conversation
railway-app
bot
temporarily deployed
to
rivet-frontend / rivet-pr-4170
|
🚅 Deployed to the rivet-pr-4170 environment in rivet-frontend
|
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
How to use the Graphite Merge QueueAdd the label merge-queue to this PR to add it to the merge queue. You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
PR #4170 Review: feat: add xyflowThis PR replaces a custom SVG-based workflow visualizer (~1,882 lines) with an XYFlow (React Flow) implementation. The refactor is a solid improvement in interactivity and maintainability. Here are issues and observations worth addressing before merge. Bugs1. Branch name matching can produce false positives ( Using // Current (fragile)
const branchItems = nested.filter((ni) => ni.key.includes(`/${name}/`));
// Better
const branchItems = nested.filter((ni) => ni.key.startsWith(`${item.key}/${name}/`));2. The last location element can be a 3. Dead regex in
4. Passing an empty string is semantically different from Code Quality5. Type assertion workaround in (node as XYNode & { parentId: string }).parentId = parentId;
(node as XYNode & { extent: string }).extent = "parent";Define a proper extended type or construct the full object literal at creation time rather than mutating through double type assertions. 6. This type is already defined (privately) in 7. Hard-coded hex colors mixed with CSS variables
Architecture / Design8. XYFlow requires an active Pro license to suppress attribution. Using this prop without a license violates XYFlow's terms of service. If the project does not have a Pro subscription, remove this prop. 9. This 1,269-line file contains only fixture data for Ladle stories. It should live alongside the stories or in a 10. No error handling around If the layout function throws on malformed history data, the entire CSS / Styling11. XYFlow dark-theme CSS is duplicated The 12.
Minor Notes
Overall the XYFlow rewrite is a substantial improvement over the previous SVG renderer. The main concerns worth resolving before merge are the branch-name matching bug (#1), the unexplained |
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| import { useState } from "react"; | ||
| import { | ||
| Handle, | ||
| NodeToolbar, | ||
| Position, | ||
| type NodeProps, | ||
| type Node, | ||
| } from "@xyflow/react"; | ||
| import { | ||
| faArrowDown, | ||
| faArrowUp, | ||
| faBolt, | ||
| faCircleCheck, | ||
| faCircleExclamation, | ||
| faCircleXmark, | ||
| faClock, | ||
| faCodeMerge, | ||
| faEnvelope, | ||
| faFlag, | ||
| faPlay, | ||
| faRefresh, | ||
| faSpinnerThird, | ||
| faTrash, | ||
| Icon, | ||
| } from "@rivet-gg/icons"; | ||
| import { cn } from "@/components"; |
There was a problem hiding this comment.
Imports need to be sorted according to the project's conventions. Consider organizing them in groups: React imports first, then third-party libraries, then local imports, each group sorted alphabetically.
Spotted by Graphite Agent (based on CI logs)
Is this helpful? React 👍 or 👎 to let us know.
| import type { Edge, Node } from "@xyflow/react"; | ||
| import { | ||
| NODE_HEIGHT, | ||
| NODE_WIDTH, | ||
| LOOP_HEADER_HEIGHT, | ||
| LOOP_PADDING_X, | ||
| LOOP_PADDING_BOTTOM, | ||
| type WorkflowNodeData, | ||
| type LoopGroupNodeData, | ||
| type BranchGroupNodeData, | ||
| formatDuration, | ||
| } from "./xyflow-nodes"; | ||
| import type { | ||
| EntryKindType, | ||
| EntryStatus, | ||
| ExtendedEntryType, | ||
| HistoryItem, | ||
| JoinEntry, | ||
| LoopEntry, | ||
| LoopIterationMarker, | ||
| MessageEntry, | ||
| RaceEntry, | ||
| RemovedEntry, | ||
| SleepEntry, | ||
| StepEntry, | ||
| WorkflowHistory, | ||
| } from "./workflow-types"; |
There was a problem hiding this comment.
Imports need to be sorted according to the project's conventions. Consider organizing them in groups: third-party libraries first, then local imports, each group sorted alphabetically.
Spotted by Graphite Agent (based on CI logs)
Is this helpful? React 👍 or 👎 to let us know.
| {/* Icon box */} | ||
| <div | ||
| className="ml-2.5 flex h-8 w-8 shrink-0 items-center justify-center rounded-lg" | ||
| style={{ | ||
| backgroundColor: colors.iconBg, | ||
| border: `1px solid ${colors.icon}4d`, | ||
| }} | ||
| > | ||
| <TypeIcon type={data.entryType} size={14} /> | ||
| </div> |
There was a problem hiding this comment.
This icon container should have an aria-hidden="true" attribute if it's decorative, or an appropriate aria-label if it conveys meaning that's not available to screen readers.
Spotted by Graphite Agent (based on CI logs)
Is this helpful? React 👍 or 👎 to let us know.
jog1t
force-pushed
the
02-11-feat_add_xyflow
branch
from
0716e7c to
9b4d46a
Compare
railway-app
bot
temporarily deployed
to
rivet-frontend / rivet-pr-4170
jog1t
force-pushed
the
02-11-feat_add_xyflow
branch
from
9b4d46a to
8bf01e3
Compare
railway-app
bot
temporarily deployed
to
rivet-frontend / rivet-pr-4170
jog1t
force-pushed
the
02-10-feat_add_ladle
branch
from
aa5df64 to
995ac00
Compare
jog1t
force-pushed
the
02-11-feat_add_xyflow
branch
from
8bf01e3 to
95291ff
Compare
jog1t
force-pushed
the
02-10-feat_add_ladle
branch
from
995ac00 to
e03420d
Compare
jog1t
force-pushed
the
02-11-feat_add_xyflow
branch
from
95291ff to
7b2929d
Compare
NathanFlurry
force-pushed
the
02-11-feat_add_xyflow
branch
from
7b2929d to
b13ac06
Compare
NathanFlurry
force-pushed
the
02-10-feat_add_ladle
branch
from
e03420d to
21d269f
Compare
Merge activity
|
# Description Added a workflow visualization component using XYFlow to render workflow history data as interactive node graphs. This implementation includes: - Custom node types for different workflow entry types (step, loop, sleep, message, etc.) - Support for nested structures like loops and parallel branches - Visual indicators for node status (completed, running, failed, retrying) - Duration display and timing gaps between nodes - Dark theme styling for all XYFlow components The implementation includes example data for various workflow patterns (linear, loops, joins, races) and Ladle stories to showcase each node type and complete workflow examples. ## Type of change - [x] New feature (non-breaking change which adds functionality) ## How Has This Been Tested? Tested through Ladle stories that demonstrate: - Individual node types with different statuses - Simple linear workflows - Workflows with loops - Workflows with parallel branches (join) - Workflows with racing branches - Complex workflows with multiple patterns - In-progress, retrying, and failed workflows ## Checklist: - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my code - [x] I have commented my code, particularly in hard-to-understand areas - [x] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes
1 participant
Description
Added a workflow visualization component using XYFlow to render workflow history data as interactive node graphs. This implementation includes:
The implementation includes example data for various workflow patterns (linear, loops, joins, races) and Ladle stories to showcase each node type and complete workflow examples.
Type of change
How Has This Been Tested?
Tested through Ladle stories that demonstrate:
Checklist: