Conversation
…in execution logs
…dicators in AllEventsView
…step selection in AllEventsView
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdded autolinker dependency and refactored the execution logs left sidebar into a hierarchical "Execution Steps" view with step-level grouping, status indicators, and sub-bubble expansion. Updated URL parsing logic, adjusted tab-switching behavior during execution, and extended the step data model with an optional Changes
Sequence DiagramsequenceDiagram
participant User
participant AllEventsView as Execution View
participant ExecStore as Execution Store
participant StepProcessor as Step Processor
participant StatusComp as Status Indicator
User->>AllEventsView: Load execution logs
AllEventsView->>StepProcessor: Extract step graph from workflow
StepProcessor-->>AllEventsView: Return hierarchical step graph
loop On Execution Events
ExecStore->>AllEventsView: Emit state update (bubbles, functions)
AllEventsView->>ExecStore: Query bubble execution states
ExecStore-->>AllEventsView: Return running/completed/error states
AllEventsView->>AllEventsView: Compute step and bubble statuses<br/>(getBubbleStatus, getStepStatus)
AllEventsView->>StatusComp: Render status indicators per step
StatusComp-->>AllEventsView: Display Loader/Check/X icons
end
User->>AllEventsView: Click step header or expand bubble
AllEventsView->>AllEventsView: Toggle expanded state<br/>(toggleBubbleExpansion)
AllEventsView->>AllEventsView: Render nested sub-bubbles
alt Error Occurs
ExecStore->>AllEventsView: Error event with context
AllEventsView->>AllEventsView: Show Pearl fix banner<br/>(handleFixWithPearl)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Suggested PR title from PearlTitle: Body: SummaryReplaces the flat execution logs sidebar with a hierarchical step-based view that shows execution progress and status for each step, bubble, and sub-bubble in real-time. ChangesUI Enhancements
Execution Behavior
URL Detection
Technical Changes
Dependencies
Visual ChangesThe execution logs sidebar now shows:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/bubble-studio/src/components/execution_logs/AllEventsView.tsx (1)
162-186: Consider extracting StatusIndicator outside the component.
StatusIndicatoris defined insideAllEventsView, causing it to be recreated on every render. While this works, extracting it as a separate component outside would improve performance slightly and make it reusable.🔎 Proposed refactor
// Move outside AllEventsView component const StatusIndicator = ({ status, size = 'normal', }: { status: 'pending' | 'running' | 'complete' | 'error'; size?: 'small' | 'normal'; }) => { const sizeClass = size === 'small' ? 'w-3 h-3' : 'w-3.5 h-3.5'; switch (status) { case 'running': return <Loader2 className={`${sizeClass} text-blue-400 animate-spin`} />; case 'complete': return <CheckCircle2 className={`${sizeClass} text-emerald-400`} />; case 'error': return <XCircle className={`${sizeClass} text-red-400`} />; default: return ( <Circle className={`${size === 'small' ? 'w-2.5 h-2.5' : 'w-3 h-3'} text-gray-600`} /> ); } };
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
apps/bubble-studio/package.jsonapps/bubble-studio/src/components/execution_logs/AllEventsView.tsxapps/bubble-studio/src/components/execution_logs/JsonRenderer.tsxapps/bubble-studio/src/hooks/useRunExecution.tsapps/bubble-studio/src/utils/workflowToSteps.ts
🧰 Additional context used
🧠 Learnings (7)
📚 Learning: 2025-12-22T09:55:47.864Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bubblelab.mdc:0-0
Timestamp: 2025-12-22T09:55:47.864Z
Learning: Applies to apps/bubble-studio/public/bubbles.json : Refer to apps/bubble-studio/public/bubbles.json for a condensed definition of all bubbles (building/bundling is required)
Applied to files:
apps/bubble-studio/package.json
📚 Learning: 2025-12-19T03:17:06.817Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bundling.mdc:0-0
Timestamp: 2025-12-19T03:17:06.817Z
Learning: Applies to apps/bubble-studio/public/bubble-types.txt : The bundle file at `apps/bubble-studio/public/bubble-types.txt` must be kept synchronized with `packages/bubble-core/dist/bubble-bundle.d.ts` after each build
Applied to files:
apps/bubble-studio/package.json
📚 Learning: 2025-12-22T09:55:47.864Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bubblelab.mdc:0-0
Timestamp: 2025-12-22T09:55:47.864Z
Learning: Applies to apps/bubble-studio/src/components/flow_visualizer/README.md : Refer to apps/bubble-studio/src/components/flow_visualizer/README.md for documentation on flow visualizer architecture and node rendering
Applied to files:
apps/bubble-studio/src/components/execution_logs/AllEventsView.tsx
📚 Learning: 2025-12-22T09:55:47.864Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bubblelab.mdc:0-0
Timestamp: 2025-12-22T09:55:47.864Z
Learning: Applies to packages/bubble-shared-schemas/src/bubbleflow-generation-prompts.ts : Refer to packages/bubble-shared-schemas/src/bubbleflow-generation-prompts.ts for documentation on how bubble flow is supposed to be generated
Applied to files:
apps/bubble-studio/src/components/execution_logs/AllEventsView.tsx
📚 Learning: 2025-12-19T03:17:06.817Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bundling.mdc:0-0
Timestamp: 2025-12-19T03:17:06.817Z
Learning: Applies to apps/bubble-studio/src/components/MonacoEditor.tsx : The Monaco Editor integration in `apps/bubble-studio/src/components/MonacoEditor.tsx` must fetch the bundled types from `/bubble-types.txt`, wrap them in a module declaration for `bubblelab/bubble-core`, and add them to Monaco's TypeScript type system
Applied to files:
apps/bubble-studio/src/components/execution_logs/JsonRenderer.tsxapps/bubble-studio/src/hooks/useRunExecution.ts
📚 Learning: 2025-12-19T03:17:06.817Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bundling.mdc:0-0
Timestamp: 2025-12-19T03:17:06.817Z
Learning: Applies to packages/bubble-core/src/index.ts : All new types and classes added to bubble-core must be exported from `packages/bubble-core/src/index.ts` to ensure they are included in the generated bundle
Applied to files:
apps/bubble-studio/src/hooks/useRunExecution.ts
📚 Learning: 2025-12-19T03:17:06.817Z
Learnt from: CR
Repo: bubblelabai/BubbleLab PR: 0
File: .cursor/rules/bundling.mdc:0-0
Timestamp: 2025-12-19T03:17:06.817Z
Learning: Applies to packages/bubble-core/dist/**/*.d.ts : All external imports (except relative imports) must be removed from the bundled output to prevent circular processing and external dependency issues
Applied to files:
apps/bubble-studio/src/hooks/useRunExecution.ts
🧬 Code graph analysis (3)
apps/bubble-studio/src/components/execution_logs/AllEventsView.tsx (3)
apps/bubble-studio/src/hooks/useLiveOutput.ts (1)
useLiveOutput(35-133)apps/bubble-studio/src/stores/executionStore.ts (1)
useExecutionStore(651-687)apps/bubble-studio/src/utils/workflowToSteps.ts (2)
extractStepGraph(57-427)StepData(9-37)
apps/bubble-studio/src/components/execution_logs/JsonRenderer.tsx (2)
apps/bubble-studio/src/components/execution_logs/downloadUtils.ts (1)
isDownloadableFileUrl(51-91)apps/bubble-studio/src/components/execution_logs/FileDownloadButton.tsx (1)
FileDownloadButton(16-144)
apps/bubble-studio/src/hooks/useRunExecution.ts (2)
apps/bubble-studio/src/stores/uiStore.ts (1)
useUIStore(207-307)apps/bubble-studio/src/stores/liveOutputStore.ts (1)
getLiveOutputStore(416-426)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Agent
- GitHub Check: test
- GitHub Check: Cloudflare Pages: bubblelab-documentation
🔇 Additional comments (17)
apps/bubble-studio/src/hooks/useRunExecution.ts (3)
86-92: LGTM: Tab reset at execution start.The optional chaining correctly handles the case where
getLiveOutputStoremight returnundefined. The intent to reset to the first tab at execution start is clear and well-documented.
159-161: LGTM: Intentional tab switching behavior change.The comments clearly document the UX rationale—avoiding jarring tab switches during execution by deferring navigation to completion events. This creates a smoother experience where users see output when it's ready.
Also applies to: 220-223
238-240: LGTM: Consistent tab switching for function calls.The same completion-based tab switching pattern is applied consistently to both bubble executions and function calls, which ensures uniform UX behavior.
Also applies to: 270-273
apps/bubble-studio/src/utils/workflowToSteps.ts (3)
17-17: LGTM: New variableId field for execution tracking.The optional
variableIdfield enables downstream correlation between steps and execution events in the console UI. This aligns with the step-based UI enhancements inAllEventsView.tsx.
94-95: LGTM: createStepBase signature update.The function signature is updated cleanly to accept the optional
variableIdparameter, and it's correctly assigned to the resultingStepDataobject.Also applies to: 112-112
185-186: LGTM: variableId propagation for function calls.The
variableIdis correctly passed from bothfunctionCallNodeandfnChild(parallel execution) tocreateStepBase, ensuring consistent tracking across all step types.Also applies to: 373-374
apps/bubble-studio/src/components/execution_logs/AllEventsView.tsx (7)
82-86: LGTM: Execution state integration.The hook correctly destructures the execution state properties needed for status indicators. This enables real-time visual feedback during execution.
108-119: LGTM: Memoized step graph extraction.The step graph extraction is correctly memoized with appropriate dependencies. The conversion of
bubbleParameterstoRecord<number, ...>format is necessary for compatibility withextractStepGraph.
122-133: LGTM: Bubble status computation.The status priority is correct: error → complete → running → pending. Checking both
bubbleWithErrorandbubbleResults[variableId] === falsecovers different error scenarios.
136-159: Verify step status aggregation logic.The step status logic looks correct, but consider whether
bubbleStatuses.some((s) => s === 'complete')returning'running'is the intended behavior. If some bubbles are complete and none are running, this marks the step as "running" (partially complete). This might be confusing if all remaining bubbles are still pending.Consider adding a comment clarifying this edge case, or adjusting the logic:
if ( bubbleStatuses.every((s) => s === 'complete') && bubbleStatuses.length > 0 ) return 'complete'; - if (bubbleStatuses.some((s) => s === 'complete')) return 'running'; // Partially complete + // Treat partially complete as "running" - some bubbles done, others pending + if (bubbleStatuses.some((s) => s === 'complete' || s === 'running')) return 'running'; return 'pending';
320-367: LGTM: Step header with click handling and status.The step header correctly handles click events, displays the step number with status-based styling, and shows the function name with description. The fallback to selecting the first bubble when no
stepVariableIdexists is appropriate.
429-444: Good: Nested button uses stopPropagation.The expand/collapse button correctly calls
e.stopPropagation()to prevent the parent button's click handler from firing. This is essential for proper nested button behavior.
476-540: LGTM: Sub-bubble expansion UI.The sub-bubble rendering with nested indentation and status indicators provides good visual hierarchy. The conditional rendering based on
hasSubBubbles && isExpandedis correct.apps/bubble-studio/src/components/execution_logs/JsonRenderer.tsx (3)
107-121: LGTM: Autolinker configuration.The Autolinker instance is correctly configured as a module-level singleton for performance. The configuration appropriately:
- Enables only URL detection (
urls: true)- Disables other patterns (email, phone, etc.)
- Preserves URL prefixes and trailing slashes for accurate display
128-187: LGTM: Robust URL parsing with Autolinker.The implementation correctly:
- Uses
autolinker.parse()to get match objects with offset information- Reconstructs the text with proper React keys for non-URL segments
- Sanitizes URLs to prevent
javascript:and dangerousdata:scheme injections- Handles trailing text after the last match
- Integrates
FileDownloadButtonfor downloadable URLsThis is a significant improvement over regex-based URL detection, especially for handling trailing punctuation correctly.
150-157: Good security practice: URL sanitization with case-insensitive scheme checking.The URL sanitization correctly blocks
javascript:,data:text/html, anddata:application/javascriptschemes using.toLowerCase()for case-insensitive matching, which handles mixed-case edge cases. Note: the normalization protection comes from the.toLowerCase()call in the code itself, not from Autolinker—Autolinker does not provide comprehensive scheme normalization or protection against advanced techniques like percent-encoded variants or Unicode tricks. The current approach is sound for typical use cases, particularly since the URL is used in a safehrefattribute context.apps/bubble-studio/package.json (1)
27-27: autolinker ^4.1.5 is current and secure.Version 4.1.5 is the latest release on npm with no known vulnerabilities. The dependency is safe to use.
There was a problem hiding this comment.
Pull request overview
This PR enhances the output logs interface by introducing a hierarchical step-based UI for execution tracking, improving URL detection in log output, and refining tab selection behavior during workflow execution.
Key Changes:
- Replaced the icon-only sidebar with a hierarchical step-based navigation (224px wide) that shows execution steps with nested bubbles and real-time status indicators
- Integrated Autolinker library for more accurate URL detection in log output, properly handling trailing punctuation
- Modified execution tab switching behavior to show output on completion rather than on start, keeping the output panel open from the beginning
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Added autolinker@4.1.5 dependency for improved URL detection |
| apps/bubble-studio/package.json | Added autolinker package to dependencies |
| apps/bubble-studio/src/utils/workflowToSteps.ts | Added variableId field to StepData for tracking execution in console output |
| apps/bubble-studio/src/hooks/useRunExecution.ts | Changed tab selection timing to open output panel on execution start and switch tabs on bubble/function completion |
| apps/bubble-studio/src/components/execution_logs/JsonRenderer.tsx | Replaced regex-based URL detection with Autolinker library for better handling of URLs with trailing punctuation |
| apps/bubble-studio/src/components/execution_logs/AllEventsView.tsx | Completely redesigned sidebar to show hierarchical step execution with expandable bubbles, status indicators, and step-based navigation |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
| <button | ||
| type="button" | ||
| onClick={() => setSelectedTab(tab.type)} | ||
| className={`relative w-full flex items-center gap-2 pl-4 pr-3 py-1.5 transition-all group ${ | ||
| isSelected | ||
| ? 'bg-[#1f6feb]/10' | ||
| : 'hover:bg-[#161b22]' | ||
| }`} | ||
| > | ||
| {/* Connecting dot */} | ||
| <div className="absolute left-[-3px] top-1/2 -translate-y-1/2 w-1.5 h-1.5 rounded-full bg-[#30363d]" /> | ||
|
|
||
| {/* Expand/collapse button for bubbles with sub-bubbles */} | ||
| {hasSubBubbles && ( | ||
| <button | ||
| type="button" | ||
| onClick={(e) => { | ||
| e.stopPropagation(); | ||
| toggleBubbleExpansion(bubbleId); | ||
| }} | ||
| className="flex-shrink-0 w-4 h-4 flex items-center justify-center text-gray-500 hover:text-gray-300 transition-colors" | ||
| > | ||
| {isExpanded ? ( | ||
| <ChevronDown className="w-3 h-3" /> | ||
| ) : ( | ||
| <ChevronRight className="w-3 h-3" /> | ||
| )} | ||
| </button> |
There was a problem hiding this comment.
Nested buttons are not allowed in HTML and create accessibility issues. The expand/collapse button is nested inside the bubble selection button, which is invalid HTML. Screen readers and keyboard navigation may not work correctly. Consider restructuring this to use separate sibling buttons or use a different approach such as wrapping the content in a div with click handlers, and making only the expand/collapse icon a button.
| // Extract step graph from workflow | ||
| const stepGraph = useMemo(() => { | ||
| if (!currentFlow.data?.workflow || !bubbleParameters) { | ||
| return { steps: [], edges: [] }; | ||
| } | ||
| // Convert bubbleParameters to the expected format (Record<number, ParsedBubbleWithInfo>) | ||
| const bubblesRecord: Record<number, (typeof bubbleParameters)[string]> = {}; | ||
| for (const [key, value] of Object.entries(bubbleParameters)) { | ||
| bubblesRecord[parseInt(key, 10)] = value; | ||
| } | ||
| return extractStepGraph(currentFlow.data.workflow, bubblesRecord); | ||
| }, [currentFlow.data?.workflow, bubbleParameters]); |
There was a problem hiding this comment.
The useMemo dependency on bubbleParameters may cause unnecessary re-computation. Since bubbleParameters is derived as currentFlow.data?.bubbleParameters || {}, it will create a new empty object reference on every render when bubbleParameters is undefined, causing the step graph to be re-extracted unnecessarily. Consider using currentFlow.data?.bubbleParameters directly as the dependency and handling the undefined case inside the useMemo callback, or use a stable empty object reference.
| function renderStringWithLinks(text: string): React.ReactNode { | ||
| const urlRegex = /(https?:\/\/[^\s"<>]+)/g; | ||
| const parts = text.split(urlRegex); | ||
| const matches = autolinker.parse(text); | ||
|
|
||
| return ( | ||
| <> | ||
| {parts.map((part, index) => { | ||
| // Check if part is a URL by checking if it starts with http:// or https:// | ||
| const isUrl = part.startsWith('http://') || part.startsWith('https://'); | ||
| if (isUrl) { | ||
| // Sanitize href to prevent javascript: and dangerous data URLs | ||
| const safeHref = | ||
| !part.toLowerCase().startsWith('javascript:') && | ||
| !part.toLowerCase().startsWith('data:text/html') && | ||
| !part.toLowerCase().startsWith('data:application/javascript') | ||
| ? part | ||
| : undefined; | ||
|
|
||
| const isDownloadable = safeHref && isDownloadableFileUrl(safeHref); | ||
|
|
||
| return ( | ||
| <span | ||
| key={index} | ||
| className="inline-flex items-center gap-1.5 flex-wrap" | ||
| > | ||
| <a | ||
| href={safeHref} | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| className="text-blue-400 hover:text-blue-300 underline break-all" | ||
| > | ||
| {part} | ||
| </a> | ||
| {isDownloadable && <FileDownloadButton url={safeHref} />} | ||
| </span> | ||
| ); | ||
| } | ||
| return <span key={index}>{part}</span>; | ||
| })} | ||
| </> | ||
| ); | ||
| if (matches.length === 0) { | ||
| return <>{text}</>; | ||
| } | ||
|
|
||
| const elements: React.ReactNode[] = []; | ||
| let lastIndex = 0; | ||
|
|
||
| matches.forEach((match, index) => { | ||
| const offset = match.getOffset(); | ||
| const matchedText = match.getMatchedText(); | ||
| const url = match.getAnchorHref(); | ||
|
|
||
| // Add text before this match | ||
| if (offset > lastIndex) { | ||
| elements.push( | ||
| <span key={`text-${index}`}>{text.slice(lastIndex, offset)}</span> | ||
| ); | ||
| } | ||
|
|
||
| // Sanitize href to prevent javascript: and dangerous data URLs | ||
| const safeHref = | ||
| url && | ||
| !url.toLowerCase().startsWith('javascript:') && | ||
| !url.toLowerCase().startsWith('data:text/html') && | ||
| !url.toLowerCase().startsWith('data:application/javascript') | ||
| ? url | ||
| : undefined; | ||
|
|
||
| const isDownloadable = safeHref && isDownloadableFileUrl(safeHref); | ||
|
|
||
| elements.push( | ||
| <span | ||
| key={`url-${index}`} | ||
| className="inline-flex items-center gap-1.5 flex-wrap" | ||
| > | ||
| <a | ||
| href={safeHref} | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| className="text-blue-400 hover:text-blue-300 underline break-all" | ||
| > | ||
| {matchedText} | ||
| </a> | ||
| {isDownloadable && safeHref && <FileDownloadButton url={safeHref} />} | ||
| </span> | ||
| ); | ||
|
|
||
| lastIndex = offset + matchedText.length; | ||
| }); | ||
|
|
||
| // Add remaining text after last match | ||
| if (lastIndex < text.length) { | ||
| elements.push(<span key="text-end">{text.slice(lastIndex)}</span>); | ||
| } | ||
|
|
||
| return <>{elements}</>; | ||
| } |
There was a problem hiding this comment.
The new URL detection logic using Autolinker lacks test coverage. The existing tests in JsonRenderer.test.tsx only cover HTML sanitization, but there are no tests for the updated renderStringWithLinks function. Consider adding tests to verify: (1) URLs are correctly detected and converted to clickable links, (2) trailing punctuation is handled correctly (e.g., "Check https://example.com." should not include the period in the URL), (3) XSS protection works (javascript: and dangerous data: URLs are sanitized), (4) download buttons appear for downloadable file URLs, and (5) text segments between URLs are preserved correctly.
2 participants
Summary
Related Issues
Type of Change
Bug fix
New feature
Breaking change
Refactor
Other (please describe):
Checklist
pnpm checkand all tests passScreenshots (if applicable)
CleanShot.2025-12-25.at.05.15.56.mp4
Additional Context
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.