Update step settings AI chat to display step metadata#1650
Update step settings AI chat to display step metadata#1650ravikiranvm merged 10 commits intomainfrom
Conversation
ravikiranvm
changed the title
ravikiranvm
changed the title
There was a problem hiding this comment.
Pull request overview
This PR updates the step settings AI chat to display step metadata information in the chat interface. The changes include increasing the chat container height and adding visual context about which step is being edited.
Key changes:
- Increased the docked chat container height from 450px to 700px to accommodate new metadata display
- Added step metadata display (logo, step name, index, and block name) to the AI chat top bar
- Threaded new optional props through multiple component layers to support the metadata display
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| step-settings-assistant-ui-chat-container.tsx | Added step metadata props and increased DOCKED_HEIGHT constant |
| assistant-ui-chat-container.tsx | Extended type definitions and forwarded step metadata props to AssistantTopBar |
| assistant-top-bar.tsx | Implemented conditional metadata display section with step icon and information |
| step-settings-assistant-ui-chat.tsx | Retrieved step metadata using hooks and passed to chat container |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import { ConnectionStatusProps } from './types'; | ||
|
|
||
| const COLLAPSED_HEIGHT = 57; | ||
| const DOCKED_HEIGHT = 450; |
There was a problem hiding this comment.
Removing this to make the height relative to the parent's
| stepLogoUrl={stepMetadata?.logoUrl} | ||
| stepDisplayName={step?.displayName} |
There was a problem hiding this comment.
We have a check to not display step metadata in the assistant top bar if these values are undefined.
And as far as the Logo URL if it is undefined, BlockIcon component falls it back to a skeleton logo
|
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. WalkthroughAdds a flowHelper utility to return a step with its 1-based index, surfaces step metadata in the builder, and threads step display props through chat containers into the assistant top bar, which conditionally renders step info and adjusts layout/title and docked height behavior. Changes
Sequence DiagramsequenceDiagram
participant Builder as step-settings-assistant-ui-chat
participant Helper as flowHelper
participant Hooks as blocksHooks
participant Container as StepSettingsAssistantUiChatContainer
participant Chat as AssistantUiChatContainer
participant TopBar as AssistantTopBar
Builder->>Helper: getStepWithIndex(flowVersion, selectedStep)
Helper-->>Builder: { step, stepIndex }
Builder->>Hooks: useStepMetadata(step) [enabled if step exists]
Hooks-->>Builder: stepMetadata
rect rgb(230,240,250)
note right of Builder: prepare display props
Builder->>Container: pass stepLogoUrl, stepDisplayName, stepIndex, blockDisplayName
Container->>Chat: forward step props
Chat->>TopBar: forward step props
TopBar->>TopBar: render step info (icon + label) when provided
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (7)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/react-ui/src/app/features/builder/ai-chat/step-settings-assistant-ui-chat.tsx (1)
85-93: LGTM! The memoization and hook usage are appropriate.The use of
useMemoto compute step and stepIndex is appropriate given the dependencies. TheuseStepMetadatahook is correctly enabled only when a step exists.One minor nitpick: Line 91 can use property shorthand:
const { stepMetadata } = blocksHooks.useStepMetadata({ - step: step, + step, enabled: !!step, });Regarding the past review comment about undefined values: The component handles undefined gracefully using optional chaining (
stepMetadata?.logoUrl,step?.displayName) and passesstepIndexwhich can beundefined. The downstream components (AssistantTopBar) useisNilchecks to determine whether to render step-specific UI, ensuring safe handling of undefined values.packages/ui-components/src/components/assistant-ui/assistant-top-bar.tsx (1)
50-58: LGTM! Proper null-safety with isNil checks.The
isStepSettingsModeguard usingisNilchecks for all four props ensures that the step-specific UI only renders when all required metadata is available. The conditionaldisplayTitlelogic appropriately switches between the animated title and "Generate with AI" text.This addresses the past review comment about using
isNilinstead of simple truthy checks.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/react-ui/src/app/features/builder/ai-chat/step-settings-assistant-ui-chat.tsx(3 hunks)packages/shared/src/lib/flows/flow-helper.ts(2 hunks)packages/shared/src/lib/flows/test/flow-helper.test.ts(2 hunks)packages/ui-components/src/components/assistant-ui/assistant-top-bar.tsx(4 hunks)packages/ui-components/src/components/assistant-ui/assistant-ui-chat-container.tsx(3 hunks)packages/ui-components/src/components/assistant-ui/step-settings-assistant-ui-chat-container.tsx(4 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/coding-guidelines.mdc)
**/*.{ts,tsx,js,jsx}: Indentation: 2 spaces for TypeScript/JavaScript
Braces required for all control blocks, even single-line
One space between keywords and parentheses:if (condition) {
Use camelCase for variables and functions
Use PascalCase for classes and types
Use UPPER_SNAKE_CASE for constants
Use lowercase with hyphens for file names (e.g.,user-profile.ts)
Preferconstoverletorvarin TypeScript/JavaScript
Prefer arrow functions for callbacks and functional components in TypeScript/JavaScript
Files:
packages/ui-components/src/components/assistant-ui/assistant-top-bar.tsxpackages/shared/src/lib/flows/flow-helper.tspackages/react-ui/src/app/features/builder/ai-chat/step-settings-assistant-ui-chat.tsxpackages/ui-components/src/components/assistant-ui/assistant-ui-chat-container.tsxpackages/ui-components/src/components/assistant-ui/step-settings-assistant-ui-chat-container.tsxpackages/shared/src/lib/flows/test/flow-helper.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/coding-guidelines.mdc)
**/*.{ts,tsx}: Use types and interfaces where appropriate in TypeScript
Use explicit return types for exported functions in TypeScript
Files:
packages/ui-components/src/components/assistant-ui/assistant-top-bar.tsxpackages/shared/src/lib/flows/flow-helper.tspackages/react-ui/src/app/features/builder/ai-chat/step-settings-assistant-ui-chat.tsxpackages/ui-components/src/components/assistant-ui/assistant-ui-chat-container.tsxpackages/ui-components/src/components/assistant-ui/step-settings-assistant-ui-chat-container.tsxpackages/shared/src/lib/flows/test/flow-helper.test.ts
**/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/coding-guidelines.mdc)
**/*.{tsx,ts}: Frontend tech stack must strictly use: React 18, Zustand, react-query v5, shadcn, and Axios withqspackage for query strings
Follow best practices for React hooks
Prefer small, composable components and extract helper functions where possible
Usecnutility to group tailwind classnames in React components
Files:
packages/ui-components/src/components/assistant-ui/assistant-top-bar.tsxpackages/shared/src/lib/flows/flow-helper.tspackages/react-ui/src/app/features/builder/ai-chat/step-settings-assistant-ui-chat.tsxpackages/ui-components/src/components/assistant-ui/assistant-ui-chat-container.tsxpackages/ui-components/src/components/assistant-ui/step-settings-assistant-ui-chat-container.tsxpackages/shared/src/lib/flows/test/flow-helper.test.ts
🧠 Learnings (3)
📚 Learning: 2025-11-25T10:22:44.853Z
Learnt from: CR
Repo: openops-cloud/openops PR: 0
File: .cursor/rules/coding-guidelines.mdc:0-0
Timestamp: 2025-11-25T10:22:44.853Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : One space between keywords and parentheses: `if (condition) {`
Applied to files:
packages/ui-components/src/components/assistant-ui/assistant-top-bar.tsx
📚 Learning: 2025-11-25T10:22:44.853Z
Learnt from: CR
Repo: openops-cloud/openops PR: 0
File: .cursor/rules/coding-guidelines.mdc:0-0
Timestamp: 2025-11-25T10:22:44.853Z
Learning: Applies to **/*.{tsx,ts} : Use `cn` utility to group tailwind classnames in React components
Applied to files:
packages/ui-components/src/components/assistant-ui/assistant-top-bar.tsx
📚 Learning: 2025-11-25T10:22:44.853Z
Learnt from: CR
Repo: openops-cloud/openops PR: 0
File: .cursor/rules/coding-guidelines.mdc:0-0
Timestamp: 2025-11-25T10:22:44.853Z
Learning: Applies to **/*.{tsx,ts} : Follow best practices for React hooks
Applied to files:
packages/react-ui/src/app/features/builder/ai-chat/step-settings-assistant-ui-chat.tsx
🧬 Code graph analysis (5)
packages/ui-components/src/components/assistant-ui/assistant-top-bar.tsx (1)
packages/shared/src/lib/common/utils/utils.ts (1)
isNil(7-11)
packages/react-ui/src/app/features/builder/ai-chat/step-settings-assistant-ui-chat.tsx (2)
packages/shared/src/lib/flows/flow-helper.ts (1)
flowHelper(1239-1342)packages/react-ui/src/app/features/blocks/lib/blocks-hook.ts (1)
blocksHooks(58-215)
packages/ui-components/src/components/assistant-ui/assistant-ui-chat-container.tsx (1)
packages/ui-components/src/components/assistant-ui/thread/thread.tsx (1)
ThreadProps(52-55)
packages/ui-components/src/components/assistant-ui/step-settings-assistant-ui-chat-container.tsx (1)
packages/ui-components/src/components/ai-chat-container/types.ts (1)
AI_CHAT_CONTAINER_SIZES(1-5)
packages/shared/src/lib/flows/test/flow-helper.test.ts (1)
packages/shared/src/lib/flows/flow-helper.ts (1)
flowHelper(1239-1342)
🪛 GitHub Check: SonarCloud Code Analysis
packages/shared/src/lib/flows/flow-helper.ts
[warning] 346-346: This assertion is unnecessary since it does not change the type of the expression.
⏰ 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). (7)
- GitHub Check: Test Blocks Shard 2
- GitHub Check: Test Engine and Libraries
- GitHub Check: Test Server API
- GitHub Check: Test Blocks Shard 1
- GitHub Check: Test Blocks Shard 3
- GitHub Check: Lint
- GitHub Check: Build Project
🔇 Additional comments (7)
packages/ui-components/src/components/assistant-ui/assistant-ui-chat-container.tsx (1)
20-25: LGTM! Clean prop threading.The extension of
ThreadPropswith the four new optional step-related props and their forwarding toAssistantTopBaris clean and consistent with the existing prop patterns.Also applies to: 46-49, 68-71
packages/shared/src/lib/flows/test/flow-helper.test.ts (1)
122-368: LGTM! Comprehensive test coverage for getStepWithIndex.The test suite thoroughly validates the new
getStepWithIndexfunction across multiple scenarios:
- Trigger and action indexing (1-based)
- Non-existent step handling (undefined)
- Nested structures (branch and loop actions)
- Index ordering verification
The helper function
createSimpleFlowVersionis well-structured and reusable for future tests.packages/ui-components/src/components/assistant-ui/step-settings-assistant-ui-chat-container.tsx (3)
19-19: LGTM! Improved height calculation.The change from a fixed
DOCKED_HEIGHTto a dynamic calculation usingDOCKED_HEIGHT_OFFSETmakes the docked height relative to the parent container, which is a better approach for responsive layouts.Also applies to: 77-77
41-44: LGTM! Proper prop threading for step metadata.The new optional props (
stepLogoUrl,stepDisplayName,stepIndex,blockDisplayName) are correctly declared, destructured, and forwarded to the innerAssistantUiChatContainer.Also applies to: 66-69, 119-122
116-116: LGTM! Title change aligns with step-settings context.The title change from "AI Chat" to "Generate with AI" better reflects the step-settings assistant functionality.
packages/ui-components/src/components/assistant-ui/assistant-top-bar.tsx (2)
61-136: LGTM! Well-structured layout refactor.The component restructure into a two-column flex layout is well-organized:
- Left side: history button, new chat button, and title
- Right side: children and close button
The layout maintains all previous functionality while accommodating the new step-settings mode. The use of
min-w-0andtruncateensures proper text overflow handling.
137-152: LGTM! Step metadata display implementation.The conditional step-settings section properly displays:
- Block icon with logo
- Block display name
- Step index and display name in italic format
The styling with
text-xs,text-gray-500, and proper spacing provides good visual hierarchy. The truncation handling prevents overflow issues.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/ui-components/src/components/assistant-ui/assistant-top-bar.tsx (2)
50-55: Consider relaxingisStepSettingsModeguard fromstepLogoUrlRight now
isStepSettingsModerequires all four props (includingstepLogoUrl) to be non‑nil, so the step metadata row won’t render if a step has no logo but does have names/index. If logo is truly mandatory that’s fine; otherwise, consider gating primarily on the textual fields (e.g.,blockDisplayName,stepIndex,stepDisplayName) so we still show step context even when the icon is missing.
28-42: Optional: add explicit return type toAssistantTopBarTo align with our TypeScript guideline of explicit return types for exported functions/components, you might consider annotating
AssistantTopBarwith something like: JSX.ElementorReact.FC<AssistantTopBarProps>in a follow‑up, even though this predates the current change.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/ui-components/src/components/assistant-ui/assistant-top-bar.tsx(4 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/coding-guidelines.mdc)
**/*.{ts,tsx,js,jsx}: Indentation: 2 spaces for TypeScript/JavaScript
Braces required for all control blocks, even single-line
One space between keywords and parentheses:if (condition) {
Use camelCase for variables and functions
Use PascalCase for classes and types
Use UPPER_SNAKE_CASE for constants
Use lowercase with hyphens for file names (e.g.,user-profile.ts)
Preferconstoverletorvarin TypeScript/JavaScript
Prefer arrow functions for callbacks and functional components in TypeScript/JavaScript
Files:
packages/ui-components/src/components/assistant-ui/assistant-top-bar.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/coding-guidelines.mdc)
**/*.{ts,tsx}: Use types and interfaces where appropriate in TypeScript
Use explicit return types for exported functions in TypeScript
Files:
packages/ui-components/src/components/assistant-ui/assistant-top-bar.tsx
**/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/coding-guidelines.mdc)
**/*.{tsx,ts}: Frontend tech stack must strictly use: React 18, Zustand, react-query v5, shadcn, and Axios withqspackage for query strings
Follow best practices for React hooks
Prefer small, composable components and extract helper functions where possible
Usecnutility to group tailwind classnames in React components
Files:
packages/ui-components/src/components/assistant-ui/assistant-top-bar.tsx
🧠 Learnings (2)
📚 Learning: 2025-11-25T10:22:44.853Z
Learnt from: CR
Repo: openops-cloud/openops PR: 0
File: .cursor/rules/coding-guidelines.mdc:0-0
Timestamp: 2025-11-25T10:22:44.853Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : One space between keywords and parentheses: `if (condition) {`
Applied to files:
packages/ui-components/src/components/assistant-ui/assistant-top-bar.tsx
📚 Learning: 2025-11-25T10:22:44.853Z
Learnt from: CR
Repo: openops-cloud/openops PR: 0
File: .cursor/rules/coding-guidelines.mdc:0-0
Timestamp: 2025-11-25T10:22:44.853Z
Learning: Applies to **/*.{tsx,ts} : Use `cn` utility to group tailwind classnames in React components
Applied to files:
packages/ui-components/src/components/assistant-ui/assistant-top-bar.tsx
🧬 Code graph analysis (1)
packages/ui-components/src/components/assistant-ui/assistant-top-bar.tsx (3)
packages/shared/src/lib/common/utils/utils.ts (1)
isNil(7-11)packages/ui-components/src/components/tooltip-wrapper.tsx (1)
TooltipWrapper(40-40)packages/ui-components/src/ui/button.tsx (1)
Button(141-141)
🔇 Additional comments (3)
packages/ui-components/src/components/assistant-ui/assistant-top-bar.tsx (3)
56-58: Step‑mode title override behavior looks correctUsing
displayTitleto switch tot('Generate with AI')whenisStepSettingsModeis true and falling back to the existinganimatedTitleotherwise keeps the old behavior intact while meeting the new requirement.
61-135: History / new‑chat / close cluster restructuring preserves behaviorThe new flex layout for the header actions looks good:
- History popover remains controlled via
isHistoryOpen/onHistoryOpenChangeand is correctly wrapped inTooltipWrapper.- The “New chat” and “Close” buttons both stop event propagation before invoking their callbacks, which avoids unintended parent handlers.
- Tailwind classes are grouped with
cnwhere dynamic (isHistoryOpen) styling is needed, matching the sharedcnusage guideline. Based on learnings, this aligns with our styling conventions.
137-151: Step metadata row wiring and spacing look consistent with prior feedbackThe conditional step info section correctly:
- Reuses
BlockIconwithlogoUrl/displayNameand no tooltip.- Shows
blockDisplayNamefollowed by a visually separated, italic( {stepIndex}. {stepDisplayName} ), withml-1handling the spacing after the block name instead of an extra string node, which addresses the earlier “empty string for spacing” concern.Assuming upstream always passes a 1‑based
stepIndex, the rendered label should match the intended UX.
|
Fixes OPS-3103
Approved by Olga
Additional Notes
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.