Skip to content

Feat : Highlight triggered and errored blocks #896

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

abdou6666
Copy link
Contributor

@abdou6666 abdou6666 commented Apr 7, 2025

Motivation

Screencast.from.07-04-25.13.03.35.webm

Before testing you might need to do hexabot stop hexabot dev
This PR introduces the feature of highlighting trigger blocks in the visual editor. Additionally, it adds error highlighting for plugins when they encounter issues, improving the visibility and debugging experience for developers.

Fixes #884

Type of change:

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

Checklist:

  • I have performed a self-review of my own code

Summary by CodeRabbit

Summary by CodeRabbit

  • New Features

    • Enhanced error tracking and live notifications during message processing provide clearer user feedback.
    • Upgraded real-time interactions through WebSocket, enabling dynamic highlights and error notifications in the visual editor.
    • Improved visual editor navigation with functions that auto-center blocks and enable smooth transitions between flows.
    • Added new event handling for block highlighting and error scenarios, improving user interaction.
    • Introduced new state management for node highlighting, error, and fallback statuses in the visual editor.
    • Integrated WebSocket query mechanism in the Diagrams component for real-time updates.
    • New methods for managing highlighted, errored, and fallback states in the visual editor's node model.
    • Added WebSocket subscription capabilities for user-specific interactions.
    • Added a new chatbot setting to enable flow debugging, which highlights executed, failed, or fallback blocks during flow runs for real-time visual debugging.
  • Style

    • Refined visual cues and animations for highlighted, error, and fallback states ensure a consistent and engaging user experience.
  • Chores

    • Various internal enhancements and cleanup have been made to boost overall stability and performance.

@abdou6666 abdou6666 self-assigned this Apr 7, 2025
Copy link

coderabbitai bot commented Apr 7, 2025

Walkthrough

The pull request updates various components across the API and frontend to enhance the handling, logging, and visual highlighting of blocks during message processing. In the service layer, method parameters are renamed and augmented with error logging and event emissions for both successful and failed highlight scenarios. The websocket gateway now processes these events and relays them to the frontend. Additionally, the visual editor has been extended with new state variables, methods, and event subscriptions to manage node highlighting, error, and fallback states, along with corresponding CSS animations and styling updates. Minor adjustments such as variable renaming, dependency injections, and copyright updates are also included.

Changes

File(s) Change Summary
api/src/chat/services/block.service.spec.ts, api/src/chat/controllers/block.controller.spec.ts Added subscriber-related imports and providers in test setups; replaced subscriberContextBlankInstance with subscriber mock in tests.
api/src/chat/services/block.service.ts, api/src/chat/services/bot.service.ts Updated processMessage signature from subscriberContext to full recipient object; added subscribe method for WebSocket subscription; added event emissions (hook:highlight:block) and logging for block highlight, error, and fallback scenarios during message processing.
api/src/utils/test/mocks/conversation.ts Added new export subscriber with associated context; updated copyright year and added import for Subscriber.
api/src/websocket/websocket.gateway.ts Added handleHighlightBlock event handler for hook:highlight:block events; injected SettingService and BlockRepository; emits socket events to rooms with block highlight data; added logging and early return if highlighting disabled or block not found.
api/types/event-emitter.d.ts Added new highlight property to IHookOperationMap with block operation requiring userId, blockId, and highlightType; added HighlightType union type (`'highlight'
api/src/setting/seeds/setting.seed-model.ts, api/src/setting/services/setting.service.ts Added new enable_debug setting (checkbox) in chatbot settings; added isHighlightEnabled() method in SettingService to retrieve this setting.
api/src/chat/controllers/subscriber.controller.spec.ts, api/src/websocket/websocket.gateway.spec.ts, api/src/user/controllers/auth.controller.spec.ts Enhanced test setups with caching, event emitter, logging, and settings modules and providers; added mocks for cache manager; integrated setting and block models and services for comprehensive test coverage.
api/src/websocket/websocket.module.ts Added BlockModel and SettingModel schemas to Mongoose feature imports; added BlockRepository, SettingSeeder, SettingService, and SettingRepository to providers; updated copyright year.
frontend/src/components/visual-editor/hooks/useVisualEditor.tsx Added boolean state variables _hasErrored, _isHighlighted, _hasFallbacked to nodes; introduced routing with useRouter; added functions to clear highlights, check node visibility, center nodes in view, and redirect flows; subscribed to "highlight:block" websocket event to handle visual highlighting and navigation on block trigger.
frontend/src/components/visual-editor/v2/CustomDiagramNodes/NodeModel.tsx Extended NodeModel with new _hasFallbacked boolean property and methods; updated constructor defaults; existing highlight and error state methods emit stateChanged events.
frontend/src/components/visual-editor/v2/CustomDiagramNodes/NodeWidget.tsx Added listener property to NodeWidget; registered and deregistered node state change listeners in lifecycle methods to force component updates; refactored class name assignment to include highlight, error, and fallback states.
frontend/src/styles/visual-editor.css Fixed CSS formatting and value consistency; added new glowing, pulsing animated styles for .node elements containing .high-lighted, .high-light-error, and .fallback-highlight children with distinct colors and animations for highlight, error, and fallback states.
frontend/src/components/visual-editor/v2/Diagrams.tsx Added useSocketGetQuery("/block/subscribe/") call to subscribe to block updates via WebSocket.
frontend/public/locales/en/chatbot_settings.json, frontend/public/locales/fr/chatbot_settings.json Added localization entries for the new enable_debug setting with label and help text in English and French.
frontend/src/websocket/socket-hooks.tsx Updated copyright year.

Sequence Diagram(s)

Highlight Block Flow

sequenceDiagram
    participant BS as BlockService/BotService
    participant EE as Event Emitter
    participant WG as WebsocketGateway
    participant FE as VisualEditor Frontend

    BS->>EE: Emit hook:highlight:block {userId, flowId, blockId, highlightType}
    EE->>WG: Deliver hook:highlight:block event
    WG->>WG: Invoke handleHighlightBlock
    WG->>FE: Emit "highlight:block" via socket with payload
    FE->>FE: handleHighlightFlow -> Clear highlights, redirect flow, center block & update highlight state
Loading

Assessment against linked issues

Objective Addressed Explanation
Highlight block and support cross-flow navigation ([#884])

Poem

I'm a rabbit coding with delight,
Hopping through nodes both day and night.
With pulses and highlights shining bright,
Errors and fallbacks now in sight.
Flows redirect and nodes take flight,
Debugging made clear, the future's bright! 🐇✨

✨ Finishing Touches
  • 📝 Generate Docstrings

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (6)
api/src/chat/services/block.service.ts (1)

571-687: Consider refactoring repeated error emission logic.

While the error emission logic is well-implemented, it's repeated in three different places with nearly identical code. Consider extracting this logic into a helper method to improve maintainability and reduce duplication.

+ /**
+  * Emits an error highlight event for the block
+  *
+  * @param block - The block that encountered an error
+  * @param recipient - The message recipient
+  */
+ private emitBlockErrorHighlight(block: Block | BlockFull, recipient: Subscriber) {
+   const flowId = typeof block.category === 'object'
+     ? block!.category.id
+     : block.category;
+   if (flowId) {
+     this.logger.log('triggered: hook:highlight:error');
+     this.eventEmitter.emit('hook:highlight:error', {
+       flowId,
+       userId: recipient.foreign_id,
+       blockId: block.id,
+     });
+   } else {
+     this.logger.warn('Unable to trigger: hook:highlight:error');
+   }
+ }

You could then replace each occurrence with a single call to this method.

🧰 Tools
🪛 Biome (1.9.4)

[error] 618-618: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

frontend/src/components/visual-editor/v2/CustomDiagramNodes/NodeModel.tsx (2)

41-42: Using underscore-prefixed properties for private-like fields.
This is a common convention in TypeScript, but consider using real private fields (#myField) if strict encapsulation is needed.


83-85: Fire an event when toggling highlight for consistency.
Currently, setHighlighted does not fire an event, unlike setHasErrored. Consider aligning them:

 setHighlighted(isHighlighted: boolean) {
   this._isHighlighted = isHighlighted;
+  this.fireEvent({ isHighlighted }, "stateChanged");
 }
frontend/src/components/visual-editor/hooks/useVisualEditor.tsx (3)

275-296: Remove Promise wrapping in an async function.
Since removeHighlights is already async, you can simplify:

-  async function removeHighlights() {
-    return new Promise((resolve) => {
-      if (!engine) {
-        return;
-      }
-      // ...
-      engine.repaintCanvas();
-      resolve(true);
-    });
+  async function removeHighlights() {
+    if (!engine) return;
+    // ...
+    engine.repaintCanvas();
+    return true;
   }

352-373: Using setTimeout may be fragile.
Loading and rendering delays might exceed 200ms in some cases. Consider an event-based approach to ensure the block highlight occurs once the diagram is ready.


375-394: Similar consideration for error highlighting.
Likewise, the 220ms timeout may need to be replaced with a more robust sync or callback mechanism.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between beca726 and 008b412.

📒 Files selected for processing (11)
  • api/src/chat/services/block.service.spec.ts (3 hunks)
  • api/src/chat/services/block.service.ts (4 hunks)
  • api/src/chat/services/bot.service.ts (3 hunks)
  • api/src/utils/test/mocks/conversation.ts (3 hunks)
  • api/src/websocket/websocket.gateway.ts (2 hunks)
  • api/types/event-emitter.d.ts (1 hunks)
  • frontend/src/components/visual-editor/hooks/useVisualEditor.tsx (4 hunks)
  • frontend/src/components/visual-editor/v2/CustomDiagramNodes/NodeModel.tsx (5 hunks)
  • frontend/src/components/visual-editor/v2/CustomDiagramNodes/NodeWidget.tsx (3 hunks)
  • frontend/src/styles/visual-editor.css (4 hunks)
  • frontend/src/websocket/socket-hooks.tsx (2 hunks)
🧰 Additional context used
🧬 Code Definitions (4)
api/src/chat/services/block.service.spec.ts (1)
api/src/utils/test/mocks/conversation.ts (1)
  • subscriber (38-40)
api/src/chat/services/bot.service.ts (1)
api/src/chat/schemas/conversation.schema.ts (1)
  • getDefaultConversationContext (24-39)
api/src/chat/services/block.service.ts (2)
api/src/chat/schemas/types/message.ts (1)
  • StdOutgoingEnvelope (325-325)
api/src/chat/schemas/types/subscriberContext.ts (1)
  • SubscriberContext (15-15)
frontend/src/components/visual-editor/hooks/useVisualEditor.tsx (3)
frontend/src/components/visual-editor/v2/CustomDiagramNodes/NodeModel.tsx (1)
  • NodeModel (31-126)
frontend/src/components/visual-editor/v2/CustomDiagramNodes/NodeWidget.tsx (2)
  • BLOCK_WIDTH (38-38)
  • BLOCK_HEIGHT (39-39)
frontend/src/websocket/socket-hooks.tsx (1)
  • useSubscribe (75-83)
⏰ Context from checks skipped due to timeout of 90000ms (3)
  • GitHub Check: API-Tests
  • GitHub Check: Frontend Tests
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (46)
frontend/src/websocket/socket-hooks.tsx (1)

2-2: Copyright year updated correctly.

The copyright year has been updated to 2025, ensuring legal compliance.

api/src/utils/test/mocks/conversation.ts (3)

2-2: Copyright year updated correctly.

The copyright year has been updated to 2025, ensuring legal compliance.


11-11: Appropriate import added for Subscriber type.

The Subscriber import is correctly added to support the new subscriber mock object.


38-40: Well-structured mock object for testing.

The new subscriber constant enhances the test mocks by providing a properly structured Subscriber object. This will help maintain consistency across tests and support the new block highlighting functionality.

The implementation aligns with the PR objective to support block highlighting features, making it easier to test the relevant functionality.

api/src/chat/services/block.service.spec.ts (3)

49-50: Updated import to use the new subscriber mock.

The import statement has been correctly updated to use the new subscriber constant instead of subscriberContextBlankInstance.


432-433: Test updated to use the new subscriber object.

The test for processing list messages has been properly updated to use the new subscriber object, which maintains consistency with implementation changes in the service layer.


466-467: Test updated to use the new subscriber object.

The second test case for processing list messages with different parameters has also been properly updated to use the subscriber object.

api/types/event-emitter.d.ts (1)

123-137: Good implementation of type definitions for block highlighting events.

The new highlight property added to the IHookOperationMap interface provides well-structured type definitions for both normal block highlighting and error highlighting. This supports the PR's main objective to enhance the visual editor with highlighting features.

The structure includes all necessary fields (userId, flowId, blockId) for both block and error scenarios, ensuring a consistent event structure between different highlighting operations. This will make implementing and maintaining the feature easier throughout the codebase.

frontend/src/components/visual-editor/v2/CustomDiagramNodes/NodeWidget.tsx (5)

17-17: Import added for listening to node state changes.

This import is essential for implementing the state change listening mechanism. Good addition.


246-246: Added property to store listener reference.

Adding a property to store the listener reference follows best practices for React component state management.


258-262: Implemented ComponentDidMount to register state change listener.

Good implementation of the componentDidMount lifecycle method to register a listener on the node. This ensures the component re-renders when the node's state changes (highlighted or errored).


264-268: Proper cleanup in ComponentWillUnmount.

Excellent implementation of cleanup logic in componentWillUnmount. This prevents memory leaks by properly deregistering the listener when the component is unmounted.


272-277: Consolidated styling logic.

Good refactoring of the CSS class handling logic to include highlight and error states. The use of a variable makes the code more readable and maintainable.

api/src/chat/services/bot.service.ts (4)

139-142: Changed processMessage parameter from context to recipient.

This change aligns with the updated method signature in the BlockService. Now passing the entire recipient object instead of just the context.


143-147: Added event emission for successful block highlighting.

Excellent addition of event emission for highlighting triggered blocks. This event provides all necessary information: flowId, blockId, and userId, which will allow the frontend to highlight the correct block when a message flows through it.


315-321: Added error highlighting event emission.

Good implementation of error highlighting event emission. The conditional check ensures that errors are only reported for non-fallback blocks, preventing false positives.


509-516: Improved fallback handling with proper recipient context.

Good improvement to the global fallback handling. Creating a structured clone of the recipient and initializing the context variables ensures proper data flow.

frontend/src/styles/visual-editor.css (7)

37-38: CSS formatting improvement.

Minor formatting improvement for color value. Using lowercase for hex colors is a common CSS best practice.


186-186: CSS transition formatting improvement.

Minor formatting improvement for transition timing function. Using explicit decimal notation (0.4s vs .4s) improves readability.


197-198: CSS scale formatting improvement.

Improved readability by using explicit decimal notation (0.75 vs .75) for scale values in transform properties.

Also applies to: 201-202


215-223: Added highlighted node styling.

Excellent implementation of CSS styling for highlighted nodes. The styling includes elevation (z-index), scaling, and a pulsing animation effect to draw attention to the highlighted node.


225-242: Created highlight pulse animation.

Well-defined keyframe animation for highlighted nodes that creates a smooth, attention-grabbing blue pulse effect. The animation transitions properly between different scales and shadow intensities.


244-252: Added error node styling.

Great implementation of the error styling that makes errors visually distinct with red highlighting. This maintains consistency with the highlight styling while clearly indicating an error state.


254-271: Created error pulse animation.

Well-implemented keyframe animation for error states using red color to clearly indicate problems. The animation sequence matches the highlight animation for consistency.

api/src/chat/services/block.service.ts (5)

477-480: Updated method signature to use recipient instead of subscriberContext.

Good change to the processMessage method signature, replacing subscriberContext with the recipient object. This provides more context and flexibility to the method.


481-481: Extract subscriberContext from recipient.

Proper extraction of the subscriberContext from the recipient object, maintaining backward compatibility with the rest of the method.


571-586: Added error event emission for deprecated attachment URLs.

Good implementation of error reporting for deprecated attachment URLs. The code properly extracts the flowId and emits the error event with all necessary details.


635-650: Added error event emission for content retrieval failures.

Well-implemented error reporting for content retrieval failures. This ensures that blocks with content that cannot be loaded are properly highlighted as errors in the UI.


672-687: Added error event emission for plugin processing failures.

Excellent implementation of error reporting for plugin processing failures. This completes the error handling for all major failure points in the block processing pipeline.

api/src/websocket/websocket.gateway.ts (3)

9-13: Ensure proper handling of event data.
Importing typed definitions from @nestjs/event-emitter is beneficial. Verify that event data broadcast through these classes does not contain sensitive information or PII without proper safeguards.


411-422: Review the “todo” and validate broadcast scope.
Currently, handleHighlightBlock emits to all connected clients. The comment suggests restricting to a specific subscriber or flow. Confirm whether the payload includes private data and, if so, consider scoping the broadcast more narrowly.


424-431: Confirm error broadcast scope and address “todo.”
Likewise, highlightBlockErrored emits to all clients. Ensure users/subscribers that receive these error events are authorized and have a valid session.

frontend/src/components/visual-editor/v2/CustomDiagramNodes/NodeModel.tsx (5)

2-2: License year update noted.
No further remarks.


12-12: Alias usage is clear.
Renaming DefaultNodeModel to StormNodeModel helps distinguish it from the custom node.


27-28: New interface properties for highlighting and error states.
The extended NodeModelOptions interface is appropriately typed for your new features.


44-46: Defaults for highlight and error states.
Providing default values for _isHighlighted and _hasErrored ensures consistent initialization.


91-97: Implementation for error state is consistent and clear.
The event firing in setHasErrored is a good way to notify listeners.

frontend/src/components/visual-editor/hooks/useVisualEditor.tsx (9)

11-11: Using Next.js router.
No issues integrating next/router.


16-16: Expanded import to include RouterType.
No concerns; usage appears straightforward.


24-24: WebSocket subscription utility.
The useSubscribe hook is consistent with the rest of the codebase.


259-259: Local router reference.
Implementing local router usage for flow navigation is fine.


297-319: Block visibility check.
The logic to compare transformed block coordinates to the canvas bounding box seems sound.


321-335: Centering a block in the viewport.
This straightforward offset logic is correct.


337-350: Redirect function looks good.
The sequence of setting the selected flow ID and routing is appropriate.


395-395: Subscription to “highlight:flow.”
No concerns; hooking into the highlight flow event is straightforward.


397-397: Subscription to “highlight:error.”
No concerns; the logic is consistent with flow-highlighting.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 008b412 and 1e67cce.

📒 Files selected for processing (1)
  • frontend/src/components/visual-editor/hooks/useVisualEditor.tsx (4 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
frontend/src/components/visual-editor/hooks/useVisualEditor.tsx (3)
frontend/src/components/visual-editor/v2/CustomDiagramNodes/NodeModel.tsx (1)
  • NodeModel (31-126)
frontend/src/components/visual-editor/v2/CustomDiagramNodes/NodeWidget.tsx (2)
  • BLOCK_WIDTH (38-38)
  • BLOCK_HEIGHT (39-39)
frontend/src/websocket/socket-hooks.tsx (1)
  • useSubscribe (75-83)
⏰ Context from checks skipped due to timeout of 90000ms (3)
  • GitHub Check: Frontend Tests
  • GitHub Check: API-Tests
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (6)
frontend/src/components/visual-editor/hooks/useVisualEditor.tsx (6)

11-11: New imports and hooks added properly to support highlighting functionality.

The added imports for useRouter, extended EntityType with RouterType, and useSubscribe are correctly implemented to support the new highlighting and navigation features.

Also applies to: 16-16, 24-24, 259-259


47-48: Node model properties correctly initialized.

The new properties _hasErrored and _isHighlighted are properly initialized as false in the NodeModel constructor options, which aligns with the implementation in the NodeModel class.


275-296: Well-implemented highlight removal function.

The removeHighlights function correctly:

  • Returns a Promise for proper async handling
  • Iterates through all nodes to reset both highlight and error states
  • Repaints the canvas to reflect visual changes
  • Properly resolves the Promise

297-319: Block visibility check function is correctly implemented.

The isBlockVisibleOnCanvas function properly calculates whether a block is within the visible area of the canvas, accurately accounting for zoom level, canvas offset, and block dimensions.


321-335: Block centering function works correctly.

The centerBlockInView function correctly calculates the offset needed to center a block in the canvas view and updates the canvas offset accordingly.


395-397: Event subscriptions are properly implemented.

The WebSocket event subscriptions for highlight:block and highlight:error are correctly implemented using the useSubscribe hook, which will properly handle component lifecycle for subscription cleanup.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (2)
api/src/websocket/websocket.gateway.ts (1)

412-425: Event handlers for block highlighting feature.

These event handlers properly implement the server-side logic for broadcasting block highlighting and error events to the appropriate WebSocket clients.

A few improvements could be made:

  1. Since these methods are marked as async but don't contain any await operations, the async keyword could be removed
  2. Consider adding error handling around the socket emission
-  @OnEvent('hook:highlight:block')
-  async handleHighlightBlock(
-    payload: IHookOperationMap['highlight']['operations']['block'],
-  ) {
-    this.io.to(`blocks:${payload.userId}`).emit('highlight:block', payload);
-  }
+  @OnEvent('hook:highlight:block')
+  handleHighlightBlock(
+    payload: IHookOperationMap['highlight']['operations']['block'],
+  ) {
+    try {
+      this.io.to(`blocks:${payload.userId}`).emit('highlight:block', payload);
+    } catch (error) {
+      this.logger.error('Failed to emit highlight:block event', error);
+    }
+  }

-  @OnEvent('hook:highlight:error')
-  async highlightBlockErrored(
-    payload: IHookOperationMap['highlight']['operations']['error'],
-  ) {
-    this.logger.warn('hook:highlight:error ', payload);
-    this.io.to(`blocks:${payload.userId}`).emit('highlight:error', payload);
-  }
+  @OnEvent('hook:highlight:error')
+  highlightBlockErrored(
+    payload: IHookOperationMap['highlight']['operations']['error'],
+  ) {
+    this.logger.warn('hook:highlight:error ', payload);
+    try {
+      this.io.to(`blocks:${payload.userId}`).emit('highlight:error', payload);
+    } catch (error) {
+      this.logger.error('Failed to emit highlight:error event', error);
+    }
+  }
api/src/chat/services/block.service.ts (1)

609-624: Duplicated error handling logic.

This error handling code properly emits events for the block highlighting feature, but the same pattern is repeated in three different places.

Consider refactoring this repeated error handling logic into a helper method to improve maintainability.

+ private emitHighlightError(block: Block | BlockFull, recipient: Subscriber) {
+   const flowId = typeof block.category === 'object' 
+     ? block!.category.id 
+     : block.category;
+   
+   if (flowId) {
+     this.logger.log('triggered: hook:highlight:error');
+     this.eventEmitter.emit('hook:highlight:error', {
+       flowId,
+       userId: recipient.id,
+       blockId: block.id,
+     });
+   } else {
+     this.logger.warn('Unable to trigger: hook:highlight:error');
+   }
+ }

-  const flowId =
-    typeof block.category === 'object'
-      ? block!.category.id
-      : block.category;
-  if (flowId) {
-    this.logger.log('triggered: hook:highlight:error');
-    this.eventEmitter.emit('hook:highlight:error', {
-      flowId,
-      userId: recipient.id,
-      blockId: block.id,
-    });
-  } else {
-    this.logger.warn('Unable to trigger: hook:highlight:error');
-  }
+  this.emitHighlightError(block, recipient);

Apply this refactoring to the other two occurrences of this pattern at lines 673-688 and 710-725.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 57d2cdb and c92ae83.

📒 Files selected for processing (5)
  • api/src/chat/services/block.service.ts (7 hunks)
  • api/src/chat/services/bot.service.ts (3 hunks)
  • api/src/websocket/websocket.gateway.ts (2 hunks)
  • frontend/src/components/visual-editor/v2/Diagrams.tsx (2 hunks)
  • frontend/src/websocket/socket-hooks.tsx (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • frontend/src/websocket/socket-hooks.tsx
  • api/src/chat/services/bot.service.ts
🧰 Additional context used
🧬 Code Definitions (2)
frontend/src/components/visual-editor/v2/Diagrams.tsx (1)
frontend/src/websocket/socket-hooks.tsx (1)
  • useSocketGetQuery (87-105)
api/src/chat/services/block.service.ts (2)
api/src/chat/schemas/types/message.ts (1)
  • StdOutgoingEnvelope (325-325)
api/src/chat/schemas/types/subscriberContext.ts (1)
  • SubscriberContext (15-15)
🪛 Biome (1.9.4)
api/src/chat/services/block.service.ts

[error] 82-82: This is an unexpected use of the debugger statement.

Unsafe fix: Remove debugger statement

(lint/suspicious/noDebugger)

⏰ Context from checks skipped due to timeout of 90000ms (3)
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: API-Tests
  • GitHub Check: Frontend Tests
🔇 Additional comments (5)
frontend/src/components/visual-editor/v2/Diagrams.tsx (1)

55-55: Import for new block highlighting feature.

The import useSocketGetQuery is added to enable real-time block highlighting functionality, aligning with the PR objectives.

api/src/websocket/websocket.gateway.ts (1)

9-13: Import dependencies for event-driven block highlighting.

The added imports from @nestjs/event-emitter enable the gateway to listen for and handle block highlighting events.

api/src/chat/services/block.service.ts (3)

9-37: Updated imports to support WebSocket functionality.

Added imports for WebSocket-related functionality that enables block highlighting through real-time communication.


62-77: Updated constructor to support WebSocket integration.

The constructor now properly handles an optional WebSocket gateway dependency, enabling the service to emit events for block highlighting.


515-520: Parameter renamed for improved clarity.

The parameter has been renamed from subscriberContext to recipient to better represent its purpose, with the subscriber context now derived from the recipient object.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (2)
api/src/chat/services/block.service.ts (2)

672-686: Duplicate error handling logic.

This is the second instance of the same error handling logic. Consider using the helper method suggested earlier to reduce code duplication.


709-723: Duplicate error handling logic.

This is the third instance of the same error handling logic. Consider using the helper method suggested earlier to reduce code duplication.

🧹 Nitpick comments (1)
api/src/chat/services/block.service.ts (1)

607-623: Consider extracting the error highlighting logic to avoid duplication.

This error handling and event emission logic is duplicated in three locations (here and at lines 672-686, 709-723). Consider extracting this into a helper method.

+ private emitBlockHighlightError(block: Block | BlockFull, recipient: Subscriber) {
+   const flowId =
+     typeof block.category === 'object'
+       ? // @ts-expect-error : block always has category
+         block!.category.id
+       : block.category;
+   if (flowId) {
+     this.logger.log('triggered: hook:highlight:error');
+     this.eventEmitter.emit('hook:highlight:error', {
+       flowId,
+       userId: recipient.id,
+       blockId: block.id,
+     });
+   } else {
+     this.logger.warn('Unable to trigger: hook:highlight:error');
+   }
+ }

  // Then replace each occurrence with:
- const flowId =
-   typeof block.category === 'object'
-     ? // @ts-expect-error : block always has category
-       block!.category.id
-     : block.category;
- if (flowId) {
-   this.logger.log('triggered: hook:highlight:error');
-   this.eventEmitter.emit('hook:highlight:error', {
-     flowId,
-     userId: recipient.id,
-     blockId: block.id,
-   });
- } else {
-   this.logger.warn('Unable to trigger: hook:highlight:error');
- }
+ this.emitBlockHighlightError(block, recipient);
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9783705 and 82b7eba.

📒 Files selected for processing (1)
  • api/src/chat/services/block.service.ts (7 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: API-Tests
  • GitHub Check: Frontend Tests
🔇 Additional comments (4)
api/src/chat/services/block.service.ts (4)

9-36: LGTM: WebSocket imports and dependencies are properly added.

The required WebSocket-related imports and dependencies have been correctly added to support the new highlight blocks functionality.


62-77: WebSocket gateway initialization looks good.

The private gateway property and initialization in the constructor are well-implemented with proper null checking. The Optional decorator is correctly used to make the gateway a non-required dependency.


79-95: WebSocket subscription endpoint is well-implemented.

The socket subscription endpoint handles both GET and POST methods, joining users to their block-specific socket rooms. The error handling with proper logging is a good practice.


514-518: Good parameter refactoring.

Changing from subscriberContext to recipient and then extracting subscriberContext from recipient.context improves the API design and is consistent with the PR objective to highlight blocks.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
api/src/websocket/websocket.gateway.ts (2)

412-419: Well-implemented event handler for block highlighting.

The handleHighlightBlock method correctly handles the 'hook:highlight:block' event and emits the appropriate socket event to the client. The logging statement is helpful for debugging purposes.

Consider adding this room type to the Room enum for consistency with other room naming patterns in this file.

// In the Room enum (line 44)
+  BLOCKS_USER = 'blocks',
  @OnEvent('hook:highlight:block')
  async handleHighlightBlock(
    payload: IHookOperationMap['highlight']['operations']['block'],
  ) {
    this.logger.log('highlighting block', payload);
-   this.io.to(`blocks:${payload.userId}`).emit('highlight:block', payload);
+   this.io.to(`${Room.BLOCKS_USER}:${payload.userId}`).emit('highlight:block', payload);
  }

420-426: Good implementation of error highlighting event handler.

The highlightBlockErrored method appropriately handles the 'hook:highlight:error' event with proper warning level logging. This aligns with the PR objective of implementing error highlighting for plugins that encounter issues.

Consider adding JSDoc comments to document the purpose and behavior of these methods, which would aid future maintainers. Also, apply the same room naming suggestion as above for consistency.

+  /**
+   * Handles events when a block should be highlighted with an error state
+   * @param payload The payload containing error and user information
+   */
  @OnEvent('hook:highlight:error')
  async highlightBlockErrored(
    payload: IHookOperationMap['highlight']['operations']['error'],
  ) {
    this.logger.warn('hook:highlight:error', payload);
-   this.io.to(`blocks:${payload.userId}`).emit('highlight:error', payload);
+   this.io.to(`${Room.BLOCKS_USER}:${payload.userId}`).emit('highlight:error', payload);
  }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 82b7eba and f7a1395.

📒 Files selected for processing (3)
  • api/src/chat/services/block.service.ts (7 hunks)
  • api/src/websocket/websocket.gateway.ts (2 hunks)
  • frontend/src/websocket/socket-hooks.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • frontend/src/websocket/socket-hooks.tsx
  • api/src/chat/services/block.service.ts
⏰ Context from checks skipped due to timeout of 90000ms (3)
  • GitHub Check: API-Tests
  • GitHub Check: Frontend Tests
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
api/src/websocket/websocket.gateway.ts (1)

9-13: Clean import structure for event handling.

The additional import of IHookOperationMap from '@nestjs/event-emitter' is appropriate for the new event handlers being added. The import structure maintains consistency with the existing code style.

@medchedli
Copy link
Contributor

I'm noticed this error shows up in the API logs: TypeError: Cannot read properties of null (reading 'id') originating from triggerBlock

This error occurs when handling a local fallback (user provides invalid input). It seems the block object passed to triggerBlock in this fallback scenario has block.category set to null. The code then tries to access block.category.id, leading to the error.

[Nest] 2065  - 04/24/2025, 2:16:27 PM   DEBUG [BotService] Conversation has been captured! Responding ...
[Nest] 2065  - 04/24/2025, 2:16:27 PM   DEBUG [BlockService] Matched patterns: []
[Nest] 2065  - 04/24/2025, 2:16:27 PM   DEBUG [BotService] Responding ...
[Nest] 2065  - 04/24/2025, 2:16:27 PM   DEBUG [BotService] 680a479a02a1c4b93d21bd3f
[Nest] 2065  - 04/24/2025, 2:16:27 PM   DEBUG [BotService] Respond to nested conversion! Go next 
[Nest] 2065  - 04/24/2025, 2:16:27 PM   DEBUG [BotService] 5efd9c51a518c9444f737177
[Nest] 2065  - 04/24/2025, 2:16:27 PM   ERROR [BotService] Unable to process/send message.
[Nest] 2065  - 04/24/2025, 2:16:27 PM   ERROR [BotService] TypeError: Cannot read properties of null (reading 'id')
    at BotService.triggerBlock (/app/src/chat/services/bot.service.ts:155:33)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at BotService.handleIncomingMessage (/app/src/chat/services/bot.service.ts:328:11)
    at BotService.processConversationMessage (/app/src/chat/services/bot.service.ts:386:14)
    at BotService.handleMessageEvent (/app/src/chat/services/bot.service.ts:484:24)
[Nest] 2065  - 04/24/2025, 2:16:27 PM   DEBUG [ChatService] Conversation has ended successfully.
[Nest] 2065  - 04/24/2025, 2:16:27 PM   DEBUG [ChatService] 680a479a02a1c4b93d21bd3f

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🔭 Outside diff range comments (1)
api/src/chat/controllers/subscriber.controller.spec.ts (1)

1-237: 💡 Verification agent

🧩 Analysis chain

Verify null safety for category access in triggerBlock.

Based on PR comments, there's a reported TypeError: "Cannot read properties of null (reading 'id')" in the triggerBlock function when handling fallback scenarios with block.category set to null.


🏁 Script executed:

#!/bin/bash
# Verify if there's null checking for block.category in the triggerBlock function

echo "Searching for triggerBlock implementations..."
rg -A 5 "triggerBlock.*\(" --type ts

echo "Checking for null safety patterns around category.id access..."
rg "category\?.id|category\s+&&\s+category\.id|category\s+!==\s+null" --type ts

Length of output: 2708


Guard against null category in triggerBlock before accessing id

The call to matchedBlock!.category!.id in triggerBlock can throw when category is null. We didn’t find any existing null-safety checks around this access. Please update this location (and any similar emits) to handle a missing category, for example:

• File: api/src/chat/services/bot.service.ts
Around the event emit in triggerBlock:

 // before
 this.eventEmitter.emit('hook:highlight:error', {
   flowId: matchedBlock!.category!.id,
   userId: convo.sender.id,
 });
 
 // after
 this.eventEmitter.emit('hook:highlight:error', {
-    flowId: matchedBlock!.category!.id,
+    flowId: matchedBlock?.category?.id ?? fallbackFlowId,
   userId: convo.sender.id,
 });

• Decide on a suitable fallback for flowId (e.g. skip the emit, use a default ID, or handle the error upstream).
• Apply the same check wherever block.category.id or matchedBlock.category.id is accessed.

This will prevent the TypeError: Cannot read properties of null (reading 'id').

🧹 Nitpick comments (1)
api/src/chat/services/block.service.ts (1)

1-2: Update copyright year to current year.

The copyright notice shows the year 2025, which appears to be a future date. Consider updating it to the current year (2024).

-/* Copyright © 2025 Hexastack. All rights reserved.
+/* Copyright © 2024 Hexastack. All rights reserved.
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f7a1395 and 6138006.

📒 Files selected for processing (11)
  • api/src/chat/controllers/block.controller.spec.ts (3 hunks)
  • api/src/chat/controllers/subscriber.controller.spec.ts (4 hunks)
  • api/src/chat/services/block.service.spec.ts (6 hunks)
  • api/src/chat/services/block.service.ts (7 hunks)
  • api/src/setting/seeds/setting.seed-model.ts (1 hunks)
  • api/src/setting/services/setting.service.ts (1 hunks)
  • api/src/user/controllers/auth.controller.spec.ts (3 hunks)
  • api/src/websocket/websocket.gateway.spec.ts (2 hunks)
  • api/src/websocket/websocket.gateway.ts (4 hunks)
  • frontend/public/locales/en/chatbot_settings.json (1 hunks)
  • frontend/public/locales/fr/chatbot_settings.json (1 hunks)
✅ Files skipped from review due to trivial changes (4)
  • api/src/chat/controllers/block.controller.spec.ts
  • frontend/public/locales/en/chatbot_settings.json
  • api/src/user/controllers/auth.controller.spec.ts
  • frontend/public/locales/fr/chatbot_settings.json
🚧 Files skipped from review as they are similar to previous changes (2)
  • api/src/chat/services/block.service.spec.ts
  • api/src/websocket/websocket.gateway.ts
⏰ Context from checks skipped due to timeout of 90000ms (3)
  • GitHub Check: Frontend Tests
  • GitHub Check: API-Tests
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (10)
api/src/setting/services/setting.service.ts (1)

168-171: Well-implemented isHighlightEnabled method.

The method correctly retrieves settings and provides a default value of false if the debug setting is not defined. This is a good practice for feature toggles.

api/src/setting/seeds/setting.seed-model.ts (1)

88-94: Appropriate addition of the debugging toggle.

The new enable_debug setting is well-structured: placed in the correct group, uses the appropriate checkbox type, and has a sensible default of false. Setting it disabled by default is a good practice for debugging features.

api/src/websocket/websocket.gateway.spec.ts (2)

9-21: Necessary imports for testing highlight functionality.

The additions of the cache, event emitter, logger, and setting module imports are appropriate for supporting testing of the new highlight features.


38-75: Complete test environment configuration.

The test module setup has been properly extended with all necessary providers and imports. The event emitter configuration, cache module, and setting service dependencies are essential for testing the new highlight functionality.

api/src/chat/controllers/subscriber.controller.spec.ts (3)

9-21: Appropriate imports for test module setup.

These imports support the testing of functionality that may interact with the highlighting feature.


68-101: Well-configured test module imports.

The test setup has been properly enhanced with cache support, event emitter, logger, and settings module. This ensures all necessary dependencies for the highlighting feature are available during testing.


117-127: Proper mock providers for testing.

The addition of the setting-related services and mock cache manager ensures proper testing of functionality that depends on these services.

api/src/chat/services/block.service.ts (3)

82-115: Well-implemented WebSocket subscription endpoint.

The subscription mechanism is properly implemented with appropriate error handling and logging. The method correctly extracts the subscriber ID from the session, finds the corresponding subscriber, and joins the socket room.


534-538: Good refactor: renamed parameter for improved clarity.

Changing from subscriberContext to recipient with a Subscriber type provides better clarity and type safety, while deriving the context from the recipient object.


89-104:

❌ Incorrect review comment

Handle potential null/undefined block category in fallback scenarios.

The PR objectives mention a TypeError when accessing block.category.id in fallback scenarios where category could be null. While the current implementation adds a check, consider making this more robust throughout the code.


🏁 Script executed:

#!/bin/bash
# Search for places where block.category is accessed without null checks
echo "Searching for potential null access on block.category..."
rg -n "block\\.category\\." --no-heading api/src/chat/

# Also check for places where the triggerBlock function is called
echo "Searching for triggerBlock function calls..."
rg -n "triggerBlock" --no-heading api/src/

Length of output: 950


Ignore null/undefined block.category suggestion
A search across api/src/chat revealed no occurrences of block.category (and thus no unguarded block.category.id accesses). There are no fallback code paths needing additional null checks for block.category.

Likely an incorrect or invalid review comment.

@abdou6666 abdou6666 force-pushed the feat/highlight-triggered-and-errored-blocks branch from 6138006 to 6ee59d4 Compare May 5, 2025 16:35
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (2)
api/src/chat/services/block.service.ts (2)

628-643: 🛠️ Refactor suggestion

Extract duplicated error emission logic to a helper method.

This error emission logic is duplicated in three places (lines 628-643, 692-707, and 729-744), violating the DRY principle.

Create a reusable helper method:

+  /**
+   * Emits a block error highlight event if the block has a valid category
+   *
+   * @param block - The block that encountered an error
+   * @param recipientId - The ID of the recipient
+   */
+  private emitBlockError(block: Block | BlockFull, recipientId: string): void {
+    const flowId =
+      typeof block.category === 'object'
+        ? block.category?.id
+        : block.category;
+    
+    if (flowId) {
+      this.logger.log('triggered: hook:highlight:error');
+      this.eventEmitter.emit('hook:highlight:error', {
+        flowId,
+        userId: recipientId,
+        blockId: block.id,
+      });
+    } else {
+      this.logger.warn('Unable to trigger: hook:highlight:error');
+    }
+  }

Then use it in all three places:

-  const flowId =
-    typeof block.category === 'object'
-      ? // @ts-expect-error : block always has category
-        block!.category.id
-      : block.category;
-  if (flowId) {
-    this.logger.log('triggered: hook:highlight:error');
-    this.eventEmitter.emit('hook:highlight:error', {
-      flowId,
-      userId: recipient.id,
-      blockId: block.id,
-    });
-  } else {
-    this.logger.warn('Unable to trigger: hook:highlight:error');
-  }
+  this.emitBlockError(block, recipient.id);

629-632: ⚠️ Potential issue

Fix type handling instead of using @ts-expect-error.

Using @ts-expect-error and non-null assertion (!) is error-prone and masks potential issues. The comment "block always has category" may not be true in all cases, as shown by the null check in the subsequent lines.

Use optional chaining to safely handle null categories:

  const flowId =
    typeof block.category === 'object'
-      ? // @ts-expect-error : block always has category
-        block!.category.id
+      ? block.category?.id
      : block.category;

This change will prevent the TypeError mentioned in PR feedback: "Cannot read properties of null (reading 'id')".

🧹 Nitpick comments (1)
frontend/src/components/visual-editor/hooks/useVisualEditor.tsx (1)

352-379: Refactor duplicate highlight handler logic.

The handleHighlightFlow and handleHighlightErroredBlock functions contain significant code duplication. Consider refactoring to a shared utility function.

+async function highlightBlock(payload: any, isError = false) {
+  await removeHighlights();
+  await redirectToTriggeredFlow(selectedCategoryId, payload.flowId);
+
+  return new Promise<void>((resolve) => {
+    setTimeout(() => {
+      const block = engine?.getModel().getNode(payload.blockId) as NodeModel | undefined;
+
+      if (!block) {
+        resolve();
+        return;
+      }
+
+      if (!isBlockVisibleOnCanvas(block)) {
+        centerBlockInView(block);
+      }
+
+      if (isError) {
+        block.setHasErrored(true);
+      } else {
+        block.setSelected(true);
+        block.setHighlighted(true);
+      }
+
+      engine.repaintCanvas();
+      resolve();
+    }, 200);
+  });
+}
+
+async function handleHighlightFlow(payload: {
+  flowId: string;
+  blockId: string;
+  userId: string;
+}) {
+  await highlightBlock(payload);
+}
+
+async function handleHighlightErroredBlock(payload: {
+  flowId: string;
+  blockId: string;
+  userId: string;
+}) {
+  await highlightBlock(payload, true);
+}

Also applies to: 381-406

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6138006 and 6ee59d4.

📒 Files selected for processing (20)
  • api/src/chat/controllers/block.controller.spec.ts (3 hunks)
  • api/src/chat/controllers/subscriber.controller.spec.ts (4 hunks)
  • api/src/chat/services/block.service.spec.ts (6 hunks)
  • api/src/chat/services/block.service.ts (7 hunks)
  • api/src/chat/services/bot.service.ts (3 hunks)
  • api/src/setting/seeds/setting.seed-model.ts (1 hunks)
  • api/src/setting/services/setting.service.ts (1 hunks)
  • api/src/user/controllers/auth.controller.spec.ts (3 hunks)
  • api/src/utils/test/mocks/conversation.ts (3 hunks)
  • api/src/websocket/websocket.gateway.spec.ts (2 hunks)
  • api/src/websocket/websocket.gateway.ts (4 hunks)
  • api/types/event-emitter.d.ts (1 hunks)
  • frontend/public/locales/en/chatbot_settings.json (1 hunks)
  • frontend/public/locales/fr/chatbot_settings.json (1 hunks)
  • frontend/src/components/visual-editor/hooks/useVisualEditor.tsx (4 hunks)
  • frontend/src/components/visual-editor/v2/CustomDiagramNodes/NodeModel.tsx (5 hunks)
  • frontend/src/components/visual-editor/v2/CustomDiagramNodes/NodeWidget.tsx (3 hunks)
  • frontend/src/components/visual-editor/v2/Diagrams.tsx (2 hunks)
  • frontend/src/styles/visual-editor.css (4 hunks)
  • frontend/src/websocket/socket-hooks.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (18)
  • frontend/public/locales/en/chatbot_settings.json
  • frontend/src/websocket/socket-hooks.tsx
  • api/src/utils/test/mocks/conversation.ts
  • api/src/chat/services/block.service.spec.ts
  • frontend/src/components/visual-editor/v2/Diagrams.tsx
  • api/src/setting/services/setting.service.ts
  • api/src/setting/seeds/setting.seed-model.ts
  • api/src/websocket/websocket.gateway.spec.ts
  • api/src/user/controllers/auth.controller.spec.ts
  • frontend/public/locales/fr/chatbot_settings.json
  • api/src/chat/controllers/block.controller.spec.ts
  • api/src/chat/services/bot.service.ts
  • api/src/websocket/websocket.gateway.ts
  • frontend/src/components/visual-editor/v2/CustomDiagramNodes/NodeWidget.tsx
  • api/src/chat/controllers/subscriber.controller.spec.ts
  • frontend/src/styles/visual-editor.css
  • api/types/event-emitter.d.ts
  • frontend/src/components/visual-editor/v2/CustomDiagramNodes/NodeModel.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
frontend/src/components/visual-editor/hooks/useVisualEditor.tsx (3)
frontend/src/components/visual-editor/v2/CustomDiagramNodes/NodeModel.tsx (1)
  • NodeModel (31-126)
frontend/src/components/visual-editor/v2/CustomDiagramNodes/NodeWidget.tsx (2)
  • BLOCK_WIDTH (38-38)
  • BLOCK_HEIGHT (39-39)
frontend/src/websocket/socket-hooks.tsx (1)
  • useSubscribe (74-82)
⏰ Context from checks skipped due to timeout of 90000ms (3)
  • GitHub Check: Frontend Tests
  • GitHub Check: API-Tests
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (10)
api/src/chat/services/block.service.ts (4)

9-36: Dependencies are well organized with proper importing strategy.

The imports are logically grouped and organized. Good job adding the necessary WebSocket-related imports for the new functionality.


64-80: Good use of dependency injection with Optional decorator.

Using @Optional() for the gateway parameter is the right approach, making the code more testable while maintaining backward compatibility.


82-115: Well-structured WebSocket subscription method.

The subscription method properly handles authentication checking, subscriber lookup, room joining, and error cases. The error handling is comprehensive with appropriate logging.


531-539: API signature change improves data access.

Changing the method signature from accepting subscriberContext to recipient of type Subscriber is a good design choice. This provides access to the full subscriber entity while still allowing access to the context.

frontend/src/components/visual-editor/hooks/useVisualEditor.tsx (6)

47-49: Good initialization of node state properties.

Adding _hasErrored and _isHighlighted properties with default values ensures consistent state initialization. These properties align well with the NodeModel class enhancements.


275-296: Well-implemented highlight removal function.

The removeHighlights function properly clears all highlighted and errored states from nodes and repaints the canvas. Returning a Promise allows for proper sequencing of operations.


297-319: Good implementation of viewport visibility check.

The isBlockVisibleOnCanvas function correctly calculates whether a block is visible in the current viewport, taking into account zoom level, canvas dimensions, and scroll position.


321-335: Well-implemented block centering function.

The centerBlockInView function properly calculates the offset needed to center a block in the viewport, considering zoom level and block dimensions.


337-350: Flow redirection logic is clean and efficient.

The flow redirection function handles both the state update and navigation appropriately. It properly checks if a redirect is needed to avoid unnecessary navigation.


407-409: Good use of WebSocket subscription hooks.

The WebSocket event subscriptions are properly implemented using the custom hook. The event names match the backend emitted events, creating a cohesive real-time experience.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🔭 Outside diff range comments (1)
api/src/chat/services/block.service.ts (1)

726-750: ⚠️ Potential issue

Handle potential null category in error highlighting.

According to the PR comments, there's a TypeError that occurs when trying to access block.category.id but category is null. This needs to be addressed.

The PR comments mention a TypeError: "Cannot read properties of null (reading 'id')" in the triggerBlock function. This likely relates to the error emission code where you're accessing block.category.id when category is null. Ensure you handle this case:

  private emitBlockHighlight(
    block: Block | BlockFull,
    userId: string,
    highlightType: 'highlight' | 'error' | 'fallback'
  ): void {
-    const flowId =
-      typeof block.category === 'object'
-        ? block.category?.id
-        : block.category;
+    let flowId;
+    if (!block.category) {
+      this.logger.warn('Block has null category, cannot determine flow ID');
+      return;
+    }
+    
+    flowId = typeof block.category === 'object'
+      ? block.category?.id
+      : block.category;
    
    if (flowId) {
      this.logger.log(`triggered: hook:highlight:${highlightType}`);
      this.eventEmitter.emit('hook:highlight:block', {
        userId,
        blockId: block.id,
        highlightType,
      });
    } else {
      this.logger.warn(`Unable to trigger: hook:highlight:${highlightType}`);
    }
  }
♻️ Duplicate comments (2)
api/src/chat/services/block.service.ts (2)

682-688: Same emission logic duplication.

This is another instance of the duplicated error emission logic.


710-716: Another instance of duplicated emission logic.

This is the third occurrence of the duplicated error emission logic.

🧹 Nitpick comments (4)
api/src/websocket/websocket.module.ts (1)

2-2: Minor update to copyright year.

The copyright year has been updated to 2025, which is consistent with other files but exceeds the current year (2024).

-Copyright © 2025 Hexastack. All rights reserved.
+Copyright © 2024 Hexastack. All rights reserved.
frontend/src/components/visual-editor/hooks/useVisualEditor.tsx (2)

356-358: Consider using requestAnimationFrame instead of setTimeout.

Using setTimeout for timing in UI operations can be unreliable. For better synchronization with rendering cycles, consider using requestAnimationFrame.

function wait(ms: number) {
-  return new Promise((resolve) => setTimeout(resolve, ms));
+  return new Promise((resolve) => {
+    const start = performance.now();
+    function checkTime() {
+      if (performance.now() - start >= ms) {
+        resolve(true);
+      } else {
+        requestAnimationFrame(checkTime);
+      }
+    }
+    requestAnimationFrame(checkTime);
+  });
}

360-394: Refactor highlight handling to reduce code duplication.

The highlight handling logic switches between three different highlight types but follows the same flow for each. This could be simplified to reduce code duplication.

async function handleHighlightFlow(payload: {
  flowId: string;
  blockId: string;
  highlightType: "highlight" | "error" | "fallback";
}) {
  await redirectToTriggeredFlow(selectedCategoryId, payload.flowId);

  await clearHighlights();
  const block = engine?.getModel().getNode(payload.blockId) as
    | NodeModel
    | undefined;

  if (!block) {
    return;
  }

  if (!isBlockVisibleOnCanvas(block)) {
    centerBlockInView(block);
  }

-  switch (payload.highlightType) {
-    case "highlight":
-      block.setHighlighted(true);
-      break;
-    case "error":
-      block.setHasErrored(true);
-      break;
-    case "fallback":
-      block.setHasFallbacked(true);
-      break;
-  }
+  // Set the appropriate highlight state based on type
+  const highlightMethods = {
+    highlight: 'setHighlighted',
+    error: 'setHasErrored',
+    fallback: 'setHasFallbacked'
+  };
+  const method = highlightMethods[payload.highlightType];
+  block[method](true);

  block.setSelected(true);
}
api/src/chat/services/block.service.ts (1)

82-115: Well-implemented WebSocket subscription endpoint.

The subscribe method is properly implemented with socket decorators and includes good error handling. However, room naming could be more consistent.

Consider using a constant for the room name prefix to ensure consistency:

+  private readonly BLOCKS_ROOM_PREFIX = 'blocks:';
+
   @SocketGet('/block/subscribe/')
   @SocketPost('/block/subscribe/')
   async subscribe(
     @SocketReq() req: SocketRequest,
     @SocketRes() res: SocketResponse,
   ) {
     try {
       const subscriberForeignId = req.session.passport?.user?.id;
       if (!subscriberForeignId) {
         this.logger.warn('Missing subscriber foreign ID in session');
         throw new Error('Invalid session or user not authenticated');
       }

       const subscriber = await this.subscriberService.findOne({
         foreign_id: subscriberForeignId,
       });

       if (!subscriber) {
         this.logger.warn(
           `Subscriber not found for foreign ID ${subscriberForeignId}`,
         );
         throw new Error('Subscriber not found');
       }
-      const room = `blocks:${subscriber.id}`;
+      const room = `${this.BLOCKS_ROOM_PREFIX}${subscriber.id}`;
       this.gateway.io.socketsJoin(room);
       this.logger.log('Subscribed to socket room', room);
       return res.status(200).json({
         success: true,
       });
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6ee59d4 and 8ba8388.

📒 Files selected for processing (10)
  • api/src/chat/services/block.service.ts (7 hunks)
  • api/src/chat/services/bot.service.ts (3 hunks)
  • api/src/websocket/websocket.gateway.spec.ts (2 hunks)
  • api/src/websocket/websocket.gateway.ts (5 hunks)
  • api/src/websocket/websocket.module.ts (1 hunks)
  • api/types/event-emitter.d.ts (2 hunks)
  • frontend/src/components/visual-editor/hooks/useVisualEditor.tsx (4 hunks)
  • frontend/src/components/visual-editor/v2/CustomDiagramNodes/NodeModel.tsx (5 hunks)
  • frontend/src/components/visual-editor/v2/CustomDiagramNodes/NodeWidget.tsx (3 hunks)
  • frontend/src/styles/visual-editor.css (4 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
  • api/src/chat/services/bot.service.ts
  • frontend/src/components/visual-editor/v2/CustomDiagramNodes/NodeWidget.tsx
  • api/types/event-emitter.d.ts
  • api/src/websocket/websocket.gateway.ts
  • frontend/src/styles/visual-editor.css
  • frontend/src/components/visual-editor/v2/CustomDiagramNodes/NodeModel.tsx
🧰 Additional context used
🧬 Code Graph Analysis (2)
api/src/websocket/websocket.module.ts (1)
api/src/chat/schemas/block.schema.ts (1)
  • BlockModel (186-189)
api/src/websocket/websocket.gateway.spec.ts (1)
api/src/chat/schemas/block.schema.ts (1)
  • BlockModel (186-189)
🔇 Additional comments (12)
api/src/websocket/websocket.module.ts (2)

10-17: Good modular imports for block highlighting functionality.

These imports provide the necessary components for the new block highlighting feature. The module structure follows good NestJS practices for feature organization.


24-32: Well-structured module configuration with proper dependency registration.

The module now correctly registers the BlockModel and SettingModel with Mongoose, and provides all necessary services and repositories for the highlight block feature. This follows the NestJS best practices for module composition.

frontend/src/components/visual-editor/hooks/useVisualEditor.tsx (5)

47-49: Good addition of block state properties.

These new properties will track different visual states for nodes (highlighted, errored, or fallbacked), which aligns with the PR objective to highlight triggered and errored blocks.


276-299: Well-implemented highlight clearing function.

The clearHighlights function properly resets all highlight states on nodes, checking each possible state. This is essential for ensuring that only one block is highlighted at a time.


300-322: Good implementation of visibility detection.

The isBlockVisibleOnCanvas function correctly calculates whether a block is within the viewport by considering zoom level, offsets, and canvas dimensions.


324-338: Well-implemented block centering logic.

The centerBlockInView function properly calculates the offsets needed to center a block in the canvas viewport, taking into account zoom level and canvas dimensions.


340-354: Good implementation of flow navigation.

The redirectToTriggeredFlow function correctly handles changing flows when a highlighted block is in a different flow than the currently visible one.

api/src/websocket/websocket.gateway.spec.ts (3)

9-22: Good comprehensive test setup with all required dependencies.

The imports have been properly updated to include all the necessary components for testing the WebsocketGateway with the new block highlighting functionality.


42-46: Complete provider registration for testing.

All the necessary services and repositories have been registered as providers, ensuring proper dependency injection during tests.


49-78: Well-configured test modules with appropriate settings.

The test setup includes well-configured EventEmitterModule and CacheModule with detailed options that match production requirements. The MongooseModule is also properly set up to use test models.

api/src/chat/services/block.service.ts (2)

64-80: Use Optional decorator for WebsocketGateway dependency.

Good use of the @optional decorator to handle cases where WebsocketGateway might not be provided. This improves the service's reusability in different contexts.


538-538: Good parameter refactoring.

Replacing subscriberContext with recipient and deriving the context from it is a good refactoring that makes the code more maintainable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

💡 [REQUEST] - Add ability to highlight a block when it gets triggered
2 participants