feat(pipeline-ui): add execution logs and formatting utilities#491
feat(pipeline-ui): add execution logs and formatting utilities#491
Conversation
|
|
Warning Rate limit exceeded
⌛ 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. ✨ 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 |
- Introduced `execution-logs.ts` for handling execution spans and formatting timestamps, durations, and bytes. - Added `format-time.ts` for high precision time formatting. - Created `pipeline-utils.ts` for converting pipeline definitions to structured info and details. - Implemented layout logic in `layout.ts` for positioning nodes in the pipeline graph. - Updated `index.ts` to export new utilities and types. - Added global styles in `globals.css` and defined types in `types.ts`. - Configured TypeScript and build settings with `tsconfig.json` and `tsdown.config.ts`. - Updated dependencies in `pnpm-lock.yaml` and `pnpm-workspace.yaml` for new packages.
1a355da to
d04127b
Compare
🌏 Preview Deployments
Built from commit: 🤖 This comment will be updated automatically when you push new commits to this PR. |
There was a problem hiding this comment.
Pull request overview
This pull request introduces a new package @ucdjs/pipelines-ui that provides React UI components and utilities for visualizing and managing UCD pipelines. The package includes execution logs viewing, pipeline graph visualization using React Flow, status indicators, and various React hooks for data fetching and state management.
Changes:
- Added new
@ucdjs/pipelines-uipackage with comprehensive React components for pipeline visualization - Integrated dependencies including
@xyflow/reactfor graph visualization and@icons-pack/react-simple-iconsfor icons - Implemented execution logging components with waterfall charts, log tables, and span details
- Created React hooks for pipeline data fetching, execution management, and version selection
Reviewed changes
Copilot reviewed 43 out of 44 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-workspace.yaml | Added catalog entries for new dependencies (@icons-pack/react-simple-icons, @xyflow/react) |
| pnpm-lock.yaml | Updated lockfile with new package dependencies and their transitive dependencies |
| packages/pipelines/pipeline-ui/package.json | New package configuration with exports, dependencies, and build scripts |
| packages/pipelines/pipeline-ui/turbo.json | Turborepo configuration for build caching |
| packages/pipelines/pipeline-ui/tsconfig.json | TypeScript configuration extending base config |
| packages/pipelines/pipeline-ui/tsdown.config.ts | Build configuration using tsdown with React compiler plugin |
| packages/pipelines/pipeline-ui/src/types.ts | TypeScript type definitions for pipeline data structures and API responses |
| packages/pipelines/pipeline-ui/src/lib/*.ts | Utility functions for formatting, graph layout, colors, and pipeline transformations |
| packages/pipelines/pipeline-ui/src/hooks/*.ts | React hooks for data fetching, execution, and UI state management |
| packages/pipelines/pipeline-ui/src/components/**/*.tsx | React components for pipeline visualization, logs, graphs, and UI elements |
| packages/pipelines/pipeline-ui/README.md | Package documentation with installation instructions |
| packages/pipelines/pipeline-ui/eslint.config.js | ESLint configuration for the package |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "email": "lucasnrgaard@gmail.com", | ||
| "url": "https://luxass.dev" | ||
| }, | ||
| "packageManager": "pnpm@10.27.0", |
There was a problem hiding this comment.
The packageManager field is set to pnpm@10.27.0, but the repository root and most other packages use pnpm@10.29.1. This inconsistency can lead to lock file conflicts and confusion. Consider updating to match the repository standard.
| import type { ClassValue } from "clsx"; | ||
|
|
||
| import { clsx } from "clsx"; | ||
|
|
||
| import { twMerge } from "tailwind-merge"; | ||
|
|
||
| export function cn(...inputs: ClassValue[]) { | ||
| return twMerge(clsx(inputs)); | ||
| } |
There was a problem hiding this comment.
A local cn utility function is defined here, but the codebase convention is to import cn from @ucdjs-internal/shared-ui instead of maintaining local copies. This duplication should be avoided to ensure consistency across the codebase. Remove this file and import cn from @ucdjs-internal/shared-ui where needed.
| export type { PipelineFlowEdge, PipelineFlowNode } from "./adapter"; | ||
| export { getNodeColor, nodeTypeColors } from "./colors"; | ||
| export { applyLayout, NODE_HEIGHT, NODE_WIDTH } from "./layout"; | ||
| export { cn } from "./utils"; |
There was a problem hiding this comment.
The export of cn from ./utils should be removed since this function should be imported from @ucdjs-internal/shared-ui instead, following the codebase convention established in apps/web.
| toPipelineInfo, | ||
| toRouteDetails, | ||
| } from "./lib/pipeline-utils"; | ||
| export { cn } from "./lib/utils"; |
There was a problem hiding this comment.
The export of cn from the main index should be removed. Following codebase conventions, consumers should import cn directly from @ucdjs-internal/shared-ui rather than from this package.
| "typecheck": "tsc --noEmit -p tsconfig.build.json" | ||
| }, | ||
| "peerDependencies": { | ||
| "@tanstack/react-router": "catalog:web", |
There was a problem hiding this comment.
The peerDependency @tanstack/react-router is declared with catalog:web, but this is likely incorrect. Based on the devDependencies, it should probably be catalog:frontend to match the catalog used elsewhere. The catalog:web reference doesn't appear to be a standard catalog in the workspace configuration.
| "@tanstack/react-router": "catalog:web", | |
| "@tanstack/react-router": "catalog:frontend", |
| export function formatHighPrecisionTime(ms: number): string { | ||
| const seconds = Math.floor(ms / 1000); | ||
| const fractionalMs = ms % 1000; | ||
| return `${seconds}.${fractionalMs.toFixed(3).padStart(6, "0")}s`; |
There was a problem hiding this comment.
The formatting logic is incorrect. fractionalMs is already a number (the remainder), so calling .toFixed(3) on it creates a string with 3 decimal places, then .padStart(6, "0") pads it to 6 characters. This produces incorrect output. For example, if ms = 1234.567, then fractionalMs = 234.567, toFixed(3) gives "234.567" (7 chars), which won't be padded. The correct approach is to format milliseconds as a zero-padded integer, e.g., fractionalMs.toString().padStart(3, "0").
| "./lib/format-time": "./dist/lib/format-time.mjs", | ||
| "./lib/layout": "./dist/lib/layout.mjs", | ||
| "./lib/pipeline-utils": "./dist/lib/pipeline-utils.mjs", | ||
| "./lib/utils": "./dist/lib/utils.mjs", |
There was a problem hiding this comment.
The export path ./lib/utils should be removed from the package exports since the cn utility is being duplicated and should instead be imported from @ucdjs-internal/shared-ui by consumers.
| > [!IMPORTANT] | ||
| > This is an internal package. It may change without warning and is not subject to semantic versioning. Use at your own risk. | ||
|
|
||
| A collection of core pipeline functionalities for the UCD project. |
There was a problem hiding this comment.
The description "A collection of core pipeline functionalities for the UCD project" is incorrect and appears to be copied from another package. This package provides UI components for pipelines, not core pipeline functionalities. The description should accurately reflect the package's purpose, e.g., "React UI components and utilities for visualizing and managing UCD pipelines."
| A collection of core pipeline functionalities for the UCD project. | |
| React UI components and utilities for visualizing and managing UCD pipelines. |
| const phaseLabels: Record<PhaseOption, string> = { | ||
| Pipeline: "Pipeline", | ||
| Version: "Version", | ||
| Parse: "Parse", | ||
| Resolve: "Resolve", | ||
| Artifact: "Artifact", | ||
| File: "File", | ||
| Cache: "Cache", | ||
| Error: "Error", | ||
| }; |
There was a problem hiding this comment.
The phaseLabels record is redundant since each key exactly maps to its own value (e.g., Pipeline: "Pipeline"). This record serves no purpose and should be removed. If the labels are simply the same as the phase names, use the phase value directly instead of looking it up in this mapping.
| const phaseLabels: Record<PhaseOption, string> = { | |
| Pipeline: "Pipeline", | |
| Version: "Version", | |
| Parse: "Parse", | |
| Resolve: "Resolve", | |
| Artifact: "Artifact", | |
| File: "File", | |
| Cache: "Cache", | |
| Error: "Error", | |
| }; | |
| const phaseLabels: Record<PhaseOption, string> = Object.fromEntries( | |
| phaseOptions.map((phase) => [phase, phase as string]), | |
| ) as Record<PhaseOption, string>; |
🔗 Linked issue
📚 Description