adds route rules#161
Conversation
Summary by CodeRabbit
Summary by CodeRabbit
WalkthroughA comprehensive routing rules feature was introduced, including backend API methods, TypeScript type definitions, and a full React-based UI. This encompasses a visual node-based rule editor, a listing and management interface, navigation updates, and reusable breadcrumb components. Supporting dependencies and sidebar navigation were also updated to integrate the new functionality. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant RoutingPage
participant RoutingList
participant ApiService
participant RuleEditor
participant NewRoutingPage
User->>RoutingPage: Navigates to /routing
RoutingPage->>RoutingList: Render
RoutingList->>ApiService: getRoutingRules()
ApiService-->>RoutingList: Returns rules
User->>RoutingList: Clicks "Add Rule"
RoutingList->>NewRoutingPage: Navigate to /routing/new
NewRoutingPage->>RuleEditor: Render editor
User->>RuleEditor: Edits and saves rule
RuleEditor->>NewRoutingPage: onSave(rule)
NewRoutingPage->>ApiService: createRoutingRule(rule)
ApiService-->>NewRoutingPage: Returns result
NewRoutingPage->>RoutingList: Navigate back, show toast
Estimated code review effort3 (~45 minutes) Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed 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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
ui/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (9)
ui/app/routing/new/page.tsx(1 hunks)ui/app/routing/page.tsx(1 hunks)ui/components/routing-rules/RoutingList.tsx(1 hunks)ui/components/routing-rules/RuleEditor.tsx(1 hunks)ui/components/sidebar.tsx(2 hunks)ui/components/ui/breadcrumb.tsx(1 hunks)ui/lib/api.ts(2 hunks)ui/lib/types/routing.ts(1 hunks)ui/package.json(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (5)
ui/app/routing/page.tsx (1)
ui/components/routing-rules/RoutingList.tsx (1)
RoutingList(22-278)
ui/components/routing-rules/RoutingList.tsx (8)
ui/hooks/use-toast.ts (1)
useToast(9-21)ui/lib/types/routing.ts (1)
RoutingRule(48-59)ui/lib/api.ts (1)
apiService(257-257)ui/components/ui/card.tsx (3)
CardHeader(50-50)CardTitle(50-50)CardDescription(50-50)ui/components/ui/button.tsx (1)
Button(71-71)ui/components/ui/badge.tsx (1)
Badge(37-37)ui/components/ui/table.tsx (6)
Table(64-64)TableHeader(64-64)TableRow(64-64)TableHead(64-64)TableBody(64-64)TableCell(64-64)ui/components/ui/switch.tsx (1)
Switch(29-29)
ui/lib/api.ts (1)
ui/lib/types/routing.ts (4)
ListRoutingRulesResponse(75-78)RoutingRule(48-59)CreateRoutingRuleRequest(61-69)UpdateRoutingRuleRequest(71-73)
ui/components/routing-rules/RuleEditor.tsx (5)
ui/lib/types/routing.ts (3)
RoutingRule(48-59)RoutingCondition(21-27)RoutingAction(39-46)ui/components/ui/badge.tsx (1)
Badge(37-37)ui/components/ui/button.tsx (1)
Button(71-71)ui/components/ui/card.tsx (4)
Card(50-50)CardHeader(50-50)CardTitle(50-50)CardContent(50-50)ui/components/ui/separator.tsx (1)
Separator(43-43)
ui/components/ui/breadcrumb.tsx (1)
ui/lib/utils.ts (1)
cn(4-6)
🪛 Biome (1.9.4)
ui/components/ui/breadcrumb.tsx
[error] 54-61: The HTML element with the interactive role "link" is not focusable.
A non-interactive HTML element that is not focusable may not be reachable for users that rely on keyboard navigation, even with an added role like "link".
Add a tabIndex attribute to make this element focusable.
(lint/a11y/useFocusableInteractive)
🔇 Additional comments (17)
ui/package.json (1)
49-49: LGTM! Clean dependency addition.The
reactflowpackage addition is properly positioned and uses a reasonable version constraint to support the new routing rules visual editor.ui/components/ui/breadcrumb.tsx (5)
1-9: LGTM! Clean imports and basic breadcrumb structure.The imports are well-organized and the basic
Breadcrumbcomponent follows good accessibility practices with proper ARIA labeling.
11-32: LGTM! Well-structured list and item components.The
BreadcrumbListandBreadcrumbItemcomponents use proper semantic HTML with good styling and TypeScript types.
34-50: LGTM! Flexible link component with good patterns.The
BreadcrumbLinkcomponent properly handles theasChildpattern for composition flexibility while maintaining proper styling.
65-99: LGTM! Proper separator and ellipsis components.Both
BreadcrumbSeparatorandBreadcrumbEllipsiscomponents use appropriate ARIA attributes and provide good accessibility with screen reader support.
101-109: LGTM! Clean component exports.All components are properly exported for use in other parts of the application.
ui/lib/types/routing.ts (3)
3-27: LGTM! Comprehensive condition type definitions.The
ConditionOperatorandConditionTypetypes cover all standard comparison operations and request aspects. TheRoutingConditioninterface properly models individual conditions with all necessary fields.
29-46: LGTM! Well-structured grouping and action types.The
ConditionGroupinterface allows for logical grouping of conditions, and theRoutingActioninterface supports various action types with appropriate optional fields for flexibility.
48-78: LGTM! Complete rule and API interface definitions.The
RoutingRuleinterface ties all components together with proper metadata fields. The request/response interfaces follow good API patterns with appropriate field inclusion/exclusion.ui/components/sidebar.tsx (2)
3-3: LGTM! Clean import addition.The
ShuffleIconimport is properly added and semantically appropriate for routing functionality.
61-66: LGTM! Well-integrated navigation item.The new "Routing rules" navigation item follows existing patterns perfectly with appropriate icon, URL, and description. The integration is clean and consistent.
ui/app/routing/page.tsx (1)
1-5: LGTM! Clean page component implementation.The
RoutingPagecomponent is a minimal and appropriate wrapper that follows Next.js page component patterns. The import path is correct and the implementation is clean.ui/app/routing/new/page.tsx (1)
58-86: Well-structured component with good UX patternsThe component has a clean structure with proper breadcrumb navigation and descriptive text that helps users understand the purpose of the page.
ui/components/routing-rules/RoutingList.tsx (2)
170-278: Excellent UI implementation with comprehensive statesThe component handles all UI states well - loading, empty, and populated states. The table layout is clean and provides all necessary information at a glance.
64-89: Fix state update consistency and success messageThere are two issues in this function:
- If the API call succeeds but there's an error between lines 67-77, the local state won't be updated
- The success message shows the opposite of what actually happened (shows "enabled" when it was actually disabled)
Apply this fix:
const handleToggleRule = async (rule: RoutingRule) => { try { const updatedRule = { ...rule, enabled: !rule.enabled } const [data, error] = await apiService.updateRoutingRule(rule.id, updatedRule) if (error) { toast({ title: "Error", description: error, variant: "destructive", }) return } - setRules(rules.map(r => r.id === rule.id ? { ...r, enabled: !r.enabled } : r)) + setRules(rules.map(r => r.id === rule.id ? updatedRule : r)) toast({ title: "Success", - description: `Rule ${rule.enabled ? 'disabled' : 'enabled'} successfully`, + description: `Rule ${updatedRule.enabled ? 'enabled' : 'disabled'} successfully`, }) } catch (error) { toast({ title: "Error", description: "Failed to update rule", variant: "destructive", }) } }Likely an incorrect or invalid review comment.
ui/lib/api.ts (1)
144-189: Well-implemented API methods following established patternsThe new routing rule endpoints are implemented consistently with the existing API patterns. Proper HTTP verbs, error handling, and TypeScript types are used throughout.
ui/components/routing-rules/RuleEditor.tsx (1)
699-699: Verify React Flow Pro license for hideAttributionThe
hideAttribution: trueoption requires a React Flow Pro license. Ensure you have the appropriate license to hide the attribution.If you don't have a Pro license, remove this line to comply with the React Flow license terms.
| function BreadcrumbPage({ className, ...props }: React.ComponentProps<"span">) { | ||
| return ( | ||
| <span | ||
| data-slot="breadcrumb-page" | ||
| role="link" | ||
| aria-disabled="true" | ||
| aria-current="page" | ||
| className={cn("text-foreground font-normal", className)} | ||
| {...props} | ||
| /> | ||
| ) | ||
| } |
There was a problem hiding this comment.
Fix accessibility issue with BreadcrumbPage component.
The static analysis tool correctly identified that this component has role="link" but is not focusable, which creates an accessibility barrier for keyboard users.
Apply this fix to make the component properly focusable:
<span
data-slot="breadcrumb-page"
role="link"
aria-disabled="true"
aria-current="page"
+ tabIndex={0}
className={cn("text-foreground font-normal", className)}
{...props}
/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function BreadcrumbPage({ className, ...props }: React.ComponentProps<"span">) { | |
| return ( | |
| <span | |
| data-slot="breadcrumb-page" | |
| role="link" | |
| aria-disabled="true" | |
| aria-current="page" | |
| className={cn("text-foreground font-normal", className)} | |
| {...props} | |
| /> | |
| ) | |
| } | |
| function BreadcrumbPage({ className, ...props }: React.ComponentProps<"span">) { | |
| return ( | |
| <span | |
| data-slot="breadcrumb-page" | |
| role="link" | |
| aria-disabled="true" | |
| aria-current="page" | |
| tabIndex={0} | |
| className={cn("text-foreground font-normal", className)} | |
| {...props} | |
| /> | |
| ) | |
| } |
🧰 Tools
🪛 Biome (1.9.4)
[error] 54-61: The HTML element with the interactive role "link" is not focusable.
A non-interactive HTML element that is not focusable may not be reachable for users that rely on keyboard navigation, even with an added role like "link".
Add a tabIndex attribute to make this element focusable.
(lint/a11y/useFocusableInteractive)
🤖 Prompt for AI Agents
In ui/components/ui/breadcrumb.tsx around lines 52 to 63, the BreadcrumbPage
component has role="link" but is not keyboard focusable, causing an
accessibility issue. To fix this, add a tabIndex={0} attribute to the span
element to make it focusable by keyboard users, ensuring it behaves correctly as
a link role element.
| const [data, error] = await apiService.createRoutingRule(createRequest) | ||
|
|
||
| if (error) { | ||
| toast({ | ||
| title: "Error", | ||
| description: error, | ||
| variant: "destructive", | ||
| }) | ||
| return | ||
| } | ||
|
|
||
| toast({ | ||
| title: "Rule created", | ||
| description: `Routing rule "${rule.name}" has been created successfully.`, | ||
| }) | ||
|
|
||
| router.push('/routing') | ||
| } catch (error) { | ||
| toast({ | ||
| title: "Error", | ||
| description: "Failed to create routing rule. Please try again.", | ||
| variant: "destructive", | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Remove redundant error handling in the catch block
The apiService.createRoutingRule already handles errors and returns them in the tuple format. The try-catch block with a generic error message might hide specific API errors that are already being handled.
Consider removing the try-catch block or making the error message more specific:
- try {
const createRequest = {
name: rule.name,
description: rule.description,
enabled: rule.enabled,
priority: rule.priority,
condition_groups: rule.condition_groups,
group_operator: rule.group_operator,
actions: rule.actions
}
const [data, error] = await apiService.createRoutingRule(createRequest)
if (error) {
toast({
title: "Error",
description: error,
variant: "destructive",
})
return
}
toast({
title: "Rule created",
description: `Routing rule "${rule.name}" has been created successfully.`,
})
router.push('/routing')
- } catch (error) {
- toast({
- title: "Error",
- description: "Failed to create routing rule. Please try again.",
- variant: "destructive",
- })
- }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In ui/app/routing/new/page.tsx between lines 28 and 52, the catch block for
error handling is redundant because apiService.createRoutingRule returns errors
in a tuple that is already handled. Remove the try-catch block entirely to avoid
hiding specific API errors and rely on the existing error handling logic that
displays detailed error messages via toast notifications.
| if (!confirm(`Are you sure you want to delete the rule "${rule.name}"?`)) { | ||
| return | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider using a modal dialog instead of native confirm
The native confirm() dialog blocks the UI thread and doesn't match your app's design system. Consider using a proper modal dialog component for better UX.
Would you like me to implement a confirmation modal using your UI components?
🤖 Prompt for AI Agents
In ui/components/routing-rules/RoutingList.tsx around lines 92 to 94, replace
the native confirm() call with a modal dialog component from your UI library to
avoid blocking the UI thread and maintain design consistency. Implement a state
variable to control the modal visibility and pass the rule name to the modal for
confirmation. Trigger the modal on delete action and handle user confirmation or
cancellation within the modal callbacks.
| const handleSave = useCallback(() => { | ||
| // Create a sample rule from the visual flow | ||
| const newRule: RoutingRule = { | ||
| id: rule?.id || crypto.randomUUID(), | ||
| name: 'Team-based routing', | ||
| description: 'Route requests based on x-bf-team header', | ||
| enabled: true, | ||
| priority: 1, | ||
| condition_groups: [{ | ||
| id: 'condition-group-1', | ||
| operator: 'AND', | ||
| conditions: [{ | ||
| id: 'condition-1', | ||
| type: 'header', | ||
| field: 'x-bf-team', | ||
| operator: 'equals', | ||
| value: 'engineering' | ||
| }] | ||
| }], | ||
| group_operator: 'AND', | ||
| actions: [{ | ||
| id: 'action-1', | ||
| type: 'route_to_provider', | ||
| provider: 'openai', | ||
| priority: 1 | ||
| }], | ||
| created_at: rule?.created_at || new Date().toISOString(), | ||
| updated_at: new Date().toISOString() | ||
| } | ||
|
|
||
| onSave(newRule) | ||
| }, [rule, onSave]) |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
The save function ignores the visual editor state
The handleSave function creates a hardcoded routing rule instead of converting the actual visual flow (nodes and edges) into a routing rule. This makes the visual editor non-functional.
The function should analyze the nodes and edges state to build the routing rule dynamically. Would you like me to implement the logic to convert the visual flow into a proper routing rule structure?
🤖 Prompt for AI Agents
In ui/components/routing-rules/RuleEditor.tsx around lines 632 to 663, the
handleSave function currently creates a hardcoded routing rule instead of using
the visual editor's nodes and edges state. To fix this, update handleSave to
dynamically parse the nodes and edges representing the visual flow and convert
them into the corresponding routing rule structure before calling onSave. This
will ensure the saved rule reflects the actual visual editor state.
| const TriggerNode = ({ data, id }: { data: TriggerNodeData; id: string }) => { | ||
| const handleDelete = () => { | ||
| if (data.onDelete) { | ||
| data.onDelete(id) | ||
| } | ||
| } | ||
|
|
||
| return ( | ||
| <div className="relative"> | ||
| {/* Completed tag above the step - right side */} | ||
| <div className="absolute -top-8 right-0 z-10"> | ||
| <Badge variant={data.completed ? 'default' : 'secondary'} className="text-xs"> | ||
| {data.completed && <Check className="h-3 w-3 mr-1" />} | ||
| {data.completed ? 'Completed' : 'Draft'} | ||
| </Badge> | ||
| </div> | ||
|
|
||
| <div className="bg-card border-2 border-primary/20 rounded-lg p-4 min-w-[350px] shadow-sm relative"> | ||
| <Handle | ||
| type="source" | ||
| position={Position.Bottom} | ||
| className="w-3 h-3 bg-primary border-2 border-background" | ||
| style={{ bottom: -6 }} | ||
| /> | ||
|
|
||
| {/* Delete button */} | ||
| {data.isEditable && ( | ||
| <Button | ||
| variant="ghost" | ||
| size="icon" | ||
| className="absolute top-2 right-2 h-6 w-6 hover:bg-destructive hover:text-destructive-foreground" | ||
| onClick={handleDelete} | ||
| > | ||
| <X className="h-3 w-3" /> | ||
| </Button> | ||
| )} | ||
|
|
||
| <div className="flex items-start gap-3 pr-8"> | ||
| <div className="bg-blue-100 dark:bg-blue-900 rounded-lg p-2 mt-1"> | ||
| <Play className="h-4 w-4 text-blue-600 dark:text-blue-400" /> | ||
| </div> | ||
|
|
||
| <div className="flex-1"> | ||
| <div className="flex items-center justify-between mb-1"> | ||
| <h3 className="font-medium text-sm">{data.name}</h3> | ||
| <Badge variant="outline" className="text-xs"> | ||
| {data.service} | ||
| </Badge> | ||
| </div> | ||
| <p className="text-muted-foreground text-xs">{data.description}</p> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| ) | ||
| } | ||
|
|
||
| const ConditionNode = ({ data, id }: { data: ConditionNodeData; id: string }) => { | ||
| const handleDelete = () => { | ||
| if (data.onDelete) { | ||
| data.onDelete(id) | ||
| } | ||
| } | ||
|
|
||
| return ( | ||
| <div className="relative"> | ||
| {/* Completed tag above the step - right side */} | ||
| <div className="absolute -top-8 right-0 z-10"> | ||
| <Badge variant={data.completed ? 'default' : 'secondary'} className="text-xs"> | ||
| {data.completed && <Check className="h-3 w-3 mr-1" />} | ||
| {data.completed ? 'Completed' : 'Draft'} | ||
| </Badge> | ||
| </div> | ||
|
|
||
| <div className="bg-card border-2 border-primary/20 rounded-lg p-4 min-w-[350px] shadow-sm relative"> | ||
| <Handle | ||
| type="target" | ||
| position={Position.Top} | ||
| className="w-3 h-3 bg-primary border-2 border-background" | ||
| style={{ top: -6 }} | ||
| /> | ||
| <Handle | ||
| type="source" | ||
| position={Position.Bottom} | ||
| className="w-3 h-3 bg-primary border-2 border-background" | ||
| style={{ bottom: -6 }} | ||
| /> | ||
|
|
||
| {/* Delete button */} | ||
| {data.isEditable && ( | ||
| <Button | ||
| variant="ghost" | ||
| size="icon" | ||
| className="absolute top-2 right-2 h-6 w-6 hover:bg-destructive hover:text-destructive-foreground" | ||
| onClick={handleDelete} | ||
| > | ||
| <X className="h-3 w-3" /> | ||
| </Button> | ||
| )} | ||
|
|
||
| <div className="flex items-start gap-3 pr-8"> | ||
| <div className="bg-yellow-100 dark:bg-yellow-900 rounded-lg p-2 mt-1"> | ||
| <AlertTriangle className="h-4 w-4 text-yellow-600 dark:text-yellow-400" /> | ||
| </div> | ||
|
|
||
| <div className="flex-1"> | ||
| <div className="flex items-center justify-between mb-1"> | ||
| <h3 className="font-medium text-sm">{data.name}</h3> | ||
| <Badge variant="outline" className="text-xs"> | ||
| {data.service} | ||
| </Badge> | ||
| </div> | ||
| <p className="text-muted-foreground text-xs">{data.description}</p> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| ) | ||
| } | ||
|
|
||
| const ActionNode = ({ data, id }: { data: ActionNodeData; id: string }) => { | ||
| const handleDelete = () => { | ||
| if (data.onDelete) { | ||
| data.onDelete(id) | ||
| } | ||
| } | ||
|
|
||
| return ( | ||
| <div className="relative"> | ||
| {/* Completed tag above the step - right side */} | ||
| <div className="absolute -top-8 right-0 z-10"> | ||
| <Badge variant={data.completed ? 'default' : 'secondary'} className="text-xs"> | ||
| {data.completed && <Check className="h-3 w-3 mr-1" />} | ||
| {data.completed ? 'Completed' : 'Draft'} | ||
| </Badge> | ||
| </div> | ||
|
|
||
| <div className="bg-card border-2 border-primary/20 rounded-lg p-4 min-w-[350px] shadow-sm relative"> | ||
| <Handle | ||
| type="target" | ||
| position={Position.Top} | ||
| className="w-3 h-3 bg-primary border-2 border-background" | ||
| style={{ top: -6 }} | ||
| /> | ||
| <Handle | ||
| type="source" | ||
| position={Position.Bottom} | ||
| className="w-3 h-3 bg-primary border-2 border-background" | ||
| style={{ bottom: -6 }} | ||
| /> | ||
|
|
||
| {/* Delete button */} | ||
| {data.isEditable && ( | ||
| <Button | ||
| variant="ghost" | ||
| size="icon" | ||
| className="absolute top-2 right-2 h-6 w-6 hover:bg-destructive hover:text-destructive-foreground" | ||
| onClick={handleDelete} | ||
| > | ||
| <X className="h-3 w-3" /> | ||
| </Button> | ||
| )} | ||
|
|
||
| <div className="flex items-start gap-3 pr-8"> | ||
| <div className="bg-green-100 dark:bg-green-900 rounded-lg p-2 mt-1"> | ||
| <MessageSquare className="h-4 w-4 text-green-600 dark:text-green-400" /> | ||
| </div> | ||
|
|
||
| <div className="flex-1"> | ||
| <div className="flex items-center justify-between mb-1"> | ||
| <h3 className="font-medium text-sm">{data.name}</h3> | ||
| <Badge variant="outline" className="text-xs"> | ||
| {data.service} | ||
| </Badge> | ||
| </div> | ||
| <p className="text-muted-foreground text-xs">{data.description}</p> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| ) | ||
| } | ||
|
|
||
| const BranchActionNode = ({ data, id }: { data: BranchActionNodeData; id: string }) => { | ||
| const handleDelete = () => { | ||
| if (data.onDelete) { | ||
| data.onDelete(id) | ||
| } | ||
| } | ||
|
|
||
| return ( | ||
| <div className="relative"> | ||
| {/* Completed tag above the step - right side */} | ||
| <div className="absolute -top-8 right-0 z-10"> | ||
| <Badge variant={data.completed ? 'default' : 'secondary'} className="text-xs"> | ||
| {data.completed && <Check className="h-3 w-3 mr-1" />} | ||
| {data.completed ? 'Completed' : 'Draft'} | ||
| </Badge> | ||
| </div> | ||
|
|
||
| <div className="bg-card border-2 border-primary/20 rounded-lg p-4 min-w-[350px] shadow-sm relative"> | ||
| <Handle | ||
| type="target" | ||
| position={Position.Top} | ||
| className="w-3 h-3 bg-primary border-2 border-background" | ||
| style={{ top: -6 }} | ||
| /> | ||
| <Handle | ||
| type="source" | ||
| position={Position.Bottom} | ||
| className="w-3 h-3 bg-primary border-2 border-background" | ||
| style={{ bottom: -6 }} | ||
| /> | ||
|
|
||
| {/* Delete button */} | ||
| {data.isEditable && ( | ||
| <Button | ||
| variant="ghost" | ||
| size="icon" | ||
| className="absolute top-2 right-2 h-6 w-6 hover:bg-destructive hover:text-destructive-foreground" | ||
| onClick={handleDelete} | ||
| > | ||
| <X className="h-3 w-3" /> | ||
| </Button> | ||
| )} | ||
|
|
||
| <div className="flex items-start gap-3 pr-8"> | ||
| <div className="bg-purple-100 dark:bg-purple-900 rounded-lg p-2 mt-1"> | ||
| <Mail className="h-4 w-4 text-purple-600 dark:text-purple-400" /> | ||
| </div> | ||
|
|
||
| <div className="flex-1"> | ||
| <div className="flex items-center justify-between mb-1"> | ||
| <h3 className="font-medium text-sm">{data.name}</h3> | ||
| <Badge variant="outline" className="text-xs"> | ||
| {data.service} | ||
| </Badge> | ||
| </div> | ||
| <p className="text-muted-foreground text-xs">{data.description}</p> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| ) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Reduce code duplication in node components
All four node components share nearly identical structure with only minor differences in colors and icons. Consider extracting a base node component to reduce duplication and improve maintainability.
Create a base node component:
interface BaseNodeProps {
id: string
data: BaseNodeData
icon: React.ComponentType<{ className?: string }>
iconColor: string
showTargetHandle?: boolean
showSourceHandle?: boolean
}
const BaseNode = ({ id, data, icon: Icon, iconColor, showTargetHandle = false, showSourceHandle = true }: BaseNodeProps) => {
const handleDelete = () => {
if (data.onDelete) {
data.onDelete(id)
}
}
return (
<div className="relative">
<div className="absolute -top-8 right-0 z-10">
<Badge variant={data.completed ? 'default' : 'secondary'} className="text-xs">
{data.completed && <Check className="h-3 w-3 mr-1" />}
{data.completed ? 'Completed' : 'Draft'}
</Badge>
</div>
<div className="bg-card border-2 border-primary/20 rounded-lg p-4 min-w-[350px] shadow-sm relative">
{showTargetHandle && (
<Handle
type="target"
position={Position.Top}
className="w-3 h-3 bg-primary border-2 border-background"
style={{ top: -6 }}
/>
)}
{showSourceHandle && (
<Handle
type="source"
position={Position.Bottom}
className="w-3 h-3 bg-primary border-2 border-background"
style={{ bottom: -6 }}
/>
)}
{data.isEditable && (
<Button
variant="ghost"
size="icon"
className="absolute top-2 right-2 h-6 w-6 hover:bg-destructive hover:text-destructive-foreground"
onClick={handleDelete}
>
<X className="h-3 w-3" />
</Button>
)}
<div className="flex items-start gap-3 pr-8">
<div className={`rounded-lg p-2 mt-1 ${iconColor}`}>
<Icon className="h-4 w-4" />
</div>
<div className="flex-1">
<div className="flex items-center justify-between mb-1">
<h3 className="font-medium text-sm">{data.name}</h3>
<Badge variant="outline" className="text-xs">
{data.service}
</Badge>
</div>
<p className="text-muted-foreground text-xs">{data.description}</p>
</div>
</div>
</div>
</div>
)
}Then simplify each node component:
const TriggerNode = ({ data, id }: { data: TriggerNodeData; id: string }) => (
<BaseNode
id={id}
data={data}
icon={Play}
iconColor="bg-blue-100 dark:bg-blue-900"
showSourceHandle={true}
showTargetHandle={false}
/>
)🤖 Prompt for AI Agents
In ui/components/routing-rules/RuleEditor.tsx around lines 135 to 379, the four
node components (TriggerNode, ConditionNode, ActionNode, BranchActionNode) have
nearly identical structure with only minor differences in icon, colors, and
handle positions, causing code duplication. To fix this, create a reusable
BaseNode component that accepts props for id, data, icon, iconColor, and
booleans to show target/source handles. Move the shared JSX structure into
BaseNode and replace the individual node components with simple wrappers that
pass their specific icon, colors, and handle visibility to BaseNode. This will
reduce duplication and improve maintainability.
| const handleSave = useCallback(() => { | ||
| // Create a sample rule from the visual flow | ||
| const newRule: RoutingRule = { | ||
| id: rule?.id || crypto.randomUUID(), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use a more compatible UUID generation method
crypto.randomUUID() is not available in all browsers and environments. Consider using a more compatible approach.
- id: rule?.id || crypto.randomUUID(),
+ id: rule?.id || `rule-${Date.now()}-${Math.random().toString(36).substr(2, 9)}`,Or better yet, let the backend generate the ID by omitting it from the create request.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| id: rule?.id || crypto.randomUUID(), | |
| id: rule?.id || `rule-${Date.now()}-${Math.random().toString(36).substr(2, 9)}`, |
🤖 Prompt for AI Agents
In ui/components/routing-rules/RuleEditor.tsx at line 635, replace the use of
crypto.randomUUID() for generating IDs because it lacks compatibility across all
browsers and environments. Instead, either remove the ID generation here and let
the backend assign the ID upon creation, or use a widely supported UUID
generation library like uuid. Adjust the code to omit the ID when creating a new
rule so the backend can handle ID assignment.
| const position = { | ||
| x: event.clientX - reactFlowBounds.left - 175, // Center the node | ||
| y: event.clientY - reactFlowBounds.top - 60, // Account for tag above | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Extract magic numbers as named constants
The hardcoded values for node positioning should be extracted as named constants for better maintainability and clarity.
+const NODE_WIDTH = 350 // matches min-w-[350px] in node components
+const NODE_CENTER_OFFSET = NODE_WIDTH / 2
+const NODE_TAG_HEIGHT = 60 // accounts for badge above node
+
const onDrop = useCallback((event: React.DragEvent) => {
event.preventDefault()
if (!draggedNodeType) return
const reactFlowBounds = (event.target as HTMLElement).getBoundingClientRect()
const position = {
- x: event.clientX - reactFlowBounds.left - 175, // Center the node
- y: event.clientY - reactFlowBounds.top - 60, // Account for tag above
+ x: event.clientX - reactFlowBounds.left - NODE_CENTER_OFFSET,
+ y: event.clientY - reactFlowBounds.top - NODE_TAG_HEIGHT,
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const position = { | |
| x: event.clientX - reactFlowBounds.left - 175, // Center the node | |
| y: event.clientY - reactFlowBounds.top - 60, // Account for tag above | |
| } | |
| // Extracted constants for node dimensions | |
| const NODE_WIDTH = 350 // matches min-w-[350px] in node components | |
| const NODE_CENTER_OFFSET = NODE_WIDTH / 2 | |
| const NODE_TAG_HEIGHT = 60 // accounts for badge above node | |
| const onDrop = useCallback((event: React.DragEvent) => { | |
| event.preventDefault() | |
| if (!draggedNodeType) return | |
| const reactFlowBounds = (event.target as HTMLElement).getBoundingClientRect() | |
| const position = { | |
| x: event.clientX - reactFlowBounds.left - NODE_CENTER_OFFSET, | |
| y: event.clientY - reactFlowBounds.top - NODE_TAG_HEIGHT, | |
| } | |
| // …rest of onDrop logic | |
| }, [draggedNodeType]) |
🤖 Prompt for AI Agents
In ui/components/routing-rules/RuleEditor.tsx around lines 606 to 609, the
position calculation uses hardcoded numeric offsets (175 and 60) which are magic
numbers. Extract these values into clearly named constants defined near the top
of the file or in a relevant constants section. Replace the numeric literals in
the position object with these named constants to improve code readability and
maintainability.
akshaydeo
force-pushed
the
07-14-adds_route_rules
branch
from
78d8d60 to
3218cde
Compare
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (7)
ui/app/routing/new/page.tsx (1)
17-52: Remove redundant try-catch block.The
apiService.createRoutingRulealready returns errors in tuple format that are properly handled. The try-catch block with a generic error message could hide specific API errors that provide better user feedback.Consider removing the try-catch wrapper to rely on the existing error handling:
- const handleSave = async (rule: RoutingRule) => { - try { const createRequest = { name: rule.name, description: rule.description, enabled: rule.enabled, priority: rule.priority, condition_groups: rule.condition_groups, group_operator: rule.group_operator, actions: rule.actions } const [data, error] = await apiService.createRoutingRule(createRequest) if (error) { toast({ title: "Error", description: error, variant: "destructive", }) return } toast({ title: "Rule created", description: `Routing rule "${rule.name}" has been created successfully.`, }) router.push('/routing') - } catch (error) { - toast({ - title: "Error", - description: "Failed to create routing rule. Please try again.", - variant: "destructive", - }) - } + }ui/components/routing-rules/RoutingList.tsx (1)
91-94: Use a modal dialog instead of native confirm.The native
confirm()dialog blocks the UI thread and doesn't match your app's design system. Consider using a proper modal dialog component for better UX.ui/components/ui/breadcrumb.tsx (1)
52-63: Fix accessibility issue with BreadcrumbPage component.The static analysis tool correctly identified that this component has
role="link"but is not focusable, which creates an accessibility barrier for keyboard users.Apply this fix to make the component properly focusable:
<span data-slot="breadcrumb-page" role="link" aria-disabled="true" aria-current="page" + tabIndex={0} className={cn("text-foreground font-normal", className)} {...props} />ui/components/routing-rules/RuleEditor.tsx (4)
632-663: The save function ignores the visual editor state.The
handleSavefunction creates a hardcoded routing rule instead of converting the actual visual flow (nodes and edges) into a routing rule. This makes the visual editor non-functional.
135-379: Reduce code duplication in node components.All four node components share nearly identical structure with only minor differences in colors and icons. Consider extracting a base node component to reduce duplication and improve maintainability.
635-635: Use a more compatible UUID generation method.
crypto.randomUUID()is not available in all browsers and environments. Consider using a more compatible approach.- id: rule?.id || crypto.randomUUID(), + id: rule?.id || `rule-${Date.now()}-${Math.random().toString(36).substr(2, 9)}`,
606-609: Extract magic numbers as named constants.The hardcoded values for node positioning should be extracted as named constants for better maintainability and clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
ui/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (36)
ui/app/docs/page.tsx(2 hunks)ui/app/plugins/page.tsx(7 hunks)ui/app/routing/new/page.tsx(1 hunks)ui/app/routing/page.tsx(1 hunks)ui/components/config/core-settings-list.tsx(5 hunks)ui/components/config/mcp-clients-lists.tsx(2 hunks)ui/components/config/provider-form.tsx(3 hunks)ui/components/config/providers-list.tsx(1 hunks)ui/components/logs/empty-state.tsx(1 hunks)ui/components/logs/log-detail-sheet.tsx(2 hunks)ui/components/logs/logs-table.tsx(2 hunks)ui/components/routing-rules/RoutingList.tsx(1 hunks)ui/components/routing-rules/RuleEditor.tsx(1 hunks)ui/components/sidebar.tsx(4 hunks)ui/components/ui/accordion.tsx(3 hunks)ui/components/ui/alert-dialog.tsx(3 hunks)ui/components/ui/alert.tsx(2 hunks)ui/components/ui/badge.tsx(1 hunks)ui/components/ui/breadcrumb.tsx(1 hunks)ui/components/ui/button.tsx(2 hunks)ui/components/ui/command.tsx(3 hunks)ui/components/ui/dialog.tsx(2 hunks)ui/components/ui/dropdown-menu.tsx(4 hunks)ui/components/ui/hover-card.tsx(3 hunks)ui/components/ui/input.tsx(1 hunks)ui/components/ui/popover.tsx(3 hunks)ui/components/ui/select.tsx(4 hunks)ui/components/ui/sidebar.tsx(10 hunks)ui/components/ui/skeleton.tsx(1 hunks)ui/components/ui/tabs.tsx(4 hunks)ui/components/ui/tag-input.tsx(2 hunks)ui/components/ui/textarea.tsx(1 hunks)ui/components/ui/tooltip.tsx(3 hunks)ui/lib/api.ts(2 hunks)ui/lib/types/routing.ts(1 hunks)ui/package.json(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#84
File: transports/bifrost-http/main.go:2-2
Timestamp: 2025-06-15T16:05:13.489Z
Learning: For the Bifrost project, HTTP transport integration routers for new providers (like Mistral and Ollama) are implemented in separate PRs from the core provider support, following a focused PR strategy.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#158
File: .github/workflows/transports-release.yml:30-43
Timestamp: 2025-07-18T11:12:28.852Z
Learning: Pratham-Mishra04 relies on branch protection rules and mandatory code reviews for security in the bifrost repository, preferring process controls over technical security measures like environment variable isolation for GitHub Actions workflows. All commits are reviewed before merging to main branch.
ui/app/plugins/page.tsx (2)
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#159
File: docs/usage/http-transport/README.md:0-0
Timestamp: 2025-07-18T10:52:24.927Z
Learning: In the Bifrost project, the npx wrapper (`npx -y bifrost@latest -port 8080`) correctly handles single-dash long flags like `-port` without requiring double dashes or the `--` separator. The implementation properly forwards flags to the underlying Go binary.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#97
File: transports/Dockerfile:37-42
Timestamp: 2025-06-19T09:06:25.750Z
Learning: In Docker configurations for this project, plugin-specific environment variables (like MAXIM_LOG_REPO_ID) should not be included in the Dockerfile's ENV section. The architectural goal is to keep Docker images plugin-agnostic and externalize all plugin configuration to runtime via docker run -e flags, rather than baking plugin config into the image.
ui/components/config/provider-form.tsx (1)
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#85
File: core/providers/anthropic.go:150-156
Timestamp: 2025-06-15T16:07:53.140Z
Learning: In the Bifrost codebase, constructor functions are allowed to mutate input ProviderConfig objects in-place (e.g., setting default BaseURL values and trimming trailing slashes). This pattern is acceptable and doesn't need to be flagged as a code review issue.
ui/app/routing/new/page.tsx (1)
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#159
File: ci/npx/bin.js:111-120
Timestamp: 2025-07-18T10:54:58.381Z
Learning: Pratham-Mishra04 considers the error handling in the downloadBinary method of ci/npx/bin.js to be sufficient for the use case. The method handles HTTP response errors with `if (!res.ok)` check and `process.exit(1)`, and they prefer not to add additional try-catch blocks around the function call for edge cases.
🧬 Code Graph Analysis (11)
ui/components/ui/tag-input.tsx (1)
ui/lib/utils.ts (1)
cn(4-6)
ui/components/ui/skeleton.tsx (1)
ui/lib/utils.ts (1)
cn(4-6)
ui/app/routing/page.tsx (1)
ui/components/routing-rules/RoutingList.tsx (1)
RoutingList(22-278)
ui/components/logs/empty-state.tsx (1)
ui/components/ui/tabs.tsx (2)
Tabs(39-39)TabsList(39-39)
ui/components/ui/tabs.tsx (1)
ui/lib/utils.ts (1)
cn(4-6)
ui/components/logs/log-detail-sheet.tsx (1)
ui/lib/constants/logs.ts (1)
REQUEST_TYPE_COLORS(33-38)
ui/components/ui/command.tsx (1)
ui/lib/utils.ts (1)
cn(4-6)
ui/components/ui/sidebar.tsx (1)
ui/lib/utils.ts (1)
cn(4-6)
ui/components/ui/breadcrumb.tsx (1)
ui/lib/utils.ts (1)
cn(4-6)
ui/components/routing-rules/RoutingList.tsx (8)
ui/hooks/use-toast.ts (1)
useToast(9-21)ui/lib/types/routing.ts (1)
RoutingRule(48-59)ui/lib/api.ts (1)
apiService(257-257)ui/components/ui/card.tsx (3)
CardHeader(50-50)CardTitle(50-50)CardDescription(50-50)ui/components/ui/button.tsx (1)
Button(71-71)ui/components/ui/badge.tsx (1)
Badge(37-37)ui/components/ui/table.tsx (6)
Table(64-64)TableHeader(64-64)TableRow(64-64)TableHead(64-64)TableBody(64-64)TableCell(64-64)ui/components/ui/switch.tsx (1)
Switch(29-29)
ui/lib/api.ts (1)
ui/lib/types/routing.ts (4)
ListRoutingRulesResponse(75-78)RoutingRule(48-59)CreateRoutingRuleRequest(61-69)UpdateRoutingRuleRequest(71-73)
🪛 Biome (1.9.4)
ui/components/ui/breadcrumb.tsx
[error] 54-61: The HTML element with the interactive role "link" is not focusable.
A non-interactive HTML element that is not focusable may not be reachable for users that rely on keyboard navigation, even with an added role like "link".
Add a tabIndex attribute to make this element focusable.
(lint/a11y/useFocusableInteractive)
🔇 Additional comments (56)
ui/components/ui/skeleton.tsx (1)
4-4: LGTM! Styling consistency update.The border-radius change from
rounded-mdtorounded-smaligns with the UI standardization effort across the codebase.ui/components/config/providers-list.tsx (1)
86-86: LGTM! Consistent styling update.The border-radius change maintains consistency with the UI standardization effort across components.
ui/app/plugins/page.tsx (1)
212-212: LGTM! Comprehensive styling consistency updates.All border-radius changes from
rounded-mdtorounded-smacross various UI elements (tabs content, alerts, code snippets, icon containers) maintain visual consistency with the broader UI standardization effort.Also applies to: 222-222, 232-232, 281-281, 299-299, 317-317, 341-341
ui/app/docs/page.tsx (2)
2-6: LGTM! Import organization improvement.The import reordering and consolidation improves code organization and readability.
94-94: LGTM! Consistent styling update.The border-radius change from
rounded-lgtorounded-smaligns with the UI standardization effort.ui/components/ui/dialog.tsx (2)
52-52: LGTM! Consistent styling update.The border-radius change from
rounded-lgtorounded-smmaintains consistency with the UI standardization effort.
90-101: LGTM! Export formatting improvement.The vertical alignment and indentation of exports improves code organization and readability.
ui/components/ui/textarea.tsx (1)
10-10: LGTM – styling change onlyThe switch from
rounded-md→rounded-smkeeps the component API untouched and aligns with the new design system.ui/components/ui/input.tsx (1)
14-14: Consistent radius update acknowledgedSame radius reduction as other form controls; no functional impact observed.
ui/components/ui/badge.tsx (1)
8-8: Minor style tweak looks goodRadius shrink matches rest of UI; no behavior change.
ui/components/ui/tag-input.tsx (2)
4-7: LGTM: Import organization improved.The import reordering enhances code organization by grouping related imports together without affecting functionality.
41-41: LGTM: Styling consistency update.The border radius change from
rounded-mdtorounded-smaligns with the broader UI refinement pattern across the codebase for consistent visual styling.ui/components/logs/empty-state.tsx (1)
220-221: LGTM: Consistent border radius styling.The border radius changes from
rounded-lgtorounded-smandrounded-t-lgtorounded-t-smalign with the broader UI consistency updates across the codebase.ui/components/logs/logs-table.tsx (2)
4-9: LGTM: Import organization improved.The import reordering enhances code clarity by grouping related imports together without affecting functionality.
89-89: LGTM: Styling consistency update.The border radius change from
rounded-mdtorounded-smmaintains visual consistency with the broader UI refinement across the codebase.ui/components/ui/button.tsx (2)
3-3: LGTM: Import reordering.Moving the React import below other imports improves code organization without affecting functionality.
9-9: LGTM: Consistent button styling.The border radius changes from
rounded-mdtorounded-smacross all button variants ensure visual consistency throughout the application as part of the broader UI refinement.Also applies to: 24-25
ui/components/config/core-settings-list.tsx (2)
6-11: LGTM: Import organization enhanced.The import reordering improves code structure by grouping related imports together without impacting functionality.
149-149: LGTM: Consistent container styling.The border radius changes from
rounded-lgtorounded-smon all setting containers maintain visual consistency with the broader UI styling updates across the application.Also applies to: 173-173, 190-190, 206-206
ui/components/ui/alert-dialog.tsx (3)
4-7: Import reordering looks good.The reordering of React and utility imports follows a consistent pattern and improves code organization.
41-41: Border radius standardization is appropriate.The change from
rounded-lgtorounded-smaligns with the broader UI consistency update mentioned in the PR summary.
83-85: Export statement formatting improvement.The single-line export format is more concise while maintaining readability.
ui/components/ui/hover-card.tsx (1)
4-4: Consistent UI standardization changes look good.The import reordering, border radius change to
rounded-sm, and export formatting all align with the broader UI consistency improvements across the codebase.Also applies to: 29-29, 38-38
ui/components/ui/alert.tsx (1)
2-2: Consistent styling and formatting updates approved.These changes follow the same pattern of UI standardization seen across other components - import reordering, border radius consistency, and export formatting improvements.
Also applies to: 7-7, 41-41
ui/components/ui/select.tsx (2)
5-5: Border radius standardization across select components.The consistent application of
rounded-smto both SelectTrigger and SelectContent components, along with import reordering, maintains the UI standardization effort.Also applies to: 34-34, 53-53
137-147: Export formatting improvement.The consistent indentation and trailing commas improve code readability and maintainability.
ui/components/sidebar.tsx (3)
3-3: ShuffleIcon import added for routing feature.The import is appropriately added to support the new routing rules navigation item.
61-66: Routing rules navigation item properly integrated.The new navigation item is well-positioned in the menu structure and includes appropriate icon, URL, and description. The placement between "Providers" and "MCP clients" makes logical sense for the user workflow.
159-159: Sidebar styling consistency maintained.The border radius changes to
rounded-smfor both main navigation and external links maintain consistency with the broader UI standardization effort.Also applies to: 205-205
ui/components/ui/accordion.tsx (1)
5-5: LGTM: Consistent styling and import organization improvements.These changes align with the broader UI consistency effort across the codebase:
- Import reordering follows conventional external-before-internal pattern
- Border radius standardization to
rounded-smmaintains visual consistency- Export reordering doesn't impact functionality
Also applies to: 23-23, 47-48
ui/components/ui/tabs.tsx (1)
4-4: LGTM: Systematic UI consistency improvements.The changes follow the same pattern applied across UI components:
- Import organization improvement
- Border radius standardization (
rounded-lg/rounded-md→rounded-sm)- Export statement reordering
These maintain visual consistency without affecting component functionality.
Also applies to: 16-16, 27-27, 39-40
ui/components/ui/tooltip.tsx (1)
4-4: LGTM: Consistent with UI standardization effort.The changes align perfectly with the systematic UI updates:
- Import statement organization
- Border radius consistency (
rounded-md→rounded-sm)- Export statement reordering
No functional impact on tooltip behavior.
Also applies to: 31-31, 43-44
ui/components/config/mcp-clients-lists.tsx (2)
15-24: LGTM: Import formatting improvement.The indentation adjustment improves code readability and maintains consistent formatting for multi-line imports.
115-115: LGTM: Border radius consistency maintained.The change from
rounded-mdtorounded-smaligns with the systematic UI styling standardization across the application.ui/components/ui/popover.tsx (1)
4-4: LGTM: Completes the systematic UI consistency update.These final changes maintain the established pattern:
- Import organization following external-first convention
- Border radius standardization to
rounded-sm- Export statement reordering for consistency
The systematic approach across all UI components ensures visual cohesion without functional impact.
Also applies to: 33-33, 48-49
ui/components/ui/dropdown-menu.tsx (3)
5-5: Import reordering looks good.Moving React import after external dependencies follows a logical import organization pattern.
28-28: Consistent border-radius styling update.The change from
rounded-mdtorounded-smaligns with the UI-wide styling standardization across the codebase.Also applies to: 184-184
193-199: Export formatting improvement.The more compact export format improves readability while maintaining the same exported components.
ui/components/logs/log-detail-sheet.tsx (2)
4-6: Import consolidation improves organization.Grouping related imports together (UI components, constants) makes the import section more readable and maintainable.
Also applies to: 11-11
103-103: Consistent styling update for request type badge.The border-radius change to
rounded-smmaintains visual consistency with the UI-wide styling standardization.ui/components/ui/sidebar.tsx (4)
6-6: Import reordering follows good practices.Moving React and utility imports after external dependencies improves import organization.
Also applies to: 25-26
247-247: Comprehensive border-radius standardization.The consistent update from
rounded-mdtorounded-smacross all sidebar elements maintains visual coherence with the UI-wide styling update.Also applies to: 408-408, 429-429, 477-477, 564-564, 589-589, 618-618, 623-623, 689-689
5-5: ShuffleIcon import supports new routing feature.The ShuffleIcon is appropriately imported to support the new routing rules navigation item.
702-726: Export formatting improved.The more compact export format enhances readability while maintaining all exported components.
ui/app/routing/new/page.tsx (1)
58-86: Well-structured component UI and navigation.The breadcrumb navigation, card structure, and RuleEditor integration provide a clean and intuitive user experience for creating routing rules.
ui/components/ui/command.tsx (3)
5-5: Import organization improvement.Moving React and utility imports after external dependencies follows the established import ordering pattern.
Also applies to: 8-8
14-14: Consistent border-radius styling.The update from
rounded-mdtorounded-smmaintains visual consistency with the UI-wide styling standardization.Also applies to: 55-55
114-114: Export reordering maintained.All components remain exported while the order has been reorganized for better logical grouping.
ui/components/config/provider-form.tsx (2)
14-36: LGTM! Import formatting looks cleaner.The reformatted imports improve readability and maintain consistent indentation.
464-464: Consistent border-radius styling applied.The change from
rounded-lg/rounded-mdtorounded-smaligns with the broader UI consistency updates across the codebase.Also applies to: 541-541
ui/components/routing-rules/RoutingList.tsx (2)
32-54: Well-structured data fetching with proper error handling.The
loadRulesfunction correctly manages loading state and provides user-friendly error messages via toast notifications.
64-89: Optimistic updates enhance user experience.The toggle functionality correctly updates the UI optimistically while handling API calls asynchronously, providing immediate feedback to users.
ui/lib/api.ts (2)
13-13: Import statement properly includes routing types.The routing types are correctly imported to support the new API methods.
144-189: Well-implemented routing rule API methods.The new routing rule endpoints follow the established patterns in the codebase:
- Consistent error handling with
getErrorMessage- Standard
[data, error]return tuple- RESTful naming conventions
- Proper TypeScript typing
ui/components/ui/breadcrumb.tsx (1)
7-109: Well-structured breadcrumb components with good accessibility.The breadcrumb components are properly implemented with:
- Correct semantic HTML elements
- Appropriate ARIA labels and roles
- Flexible composition with the
asChildprop pattern- Consistent styling utilities
Just address the focusable issue on
BreadcrumbPagementioned above.ui/components/routing-rules/RuleEditor.tsx (1)
699-699: No Pro license required for hideAttribution
ThehideAttributionflag is part of React Flow’s MIT-licensed core and can be used in any project without a Pro subscription. You’re free to remove the attribution; the team only asks (but does not require) that you review their “removing attribution” guide and consider financial support for commercial use.• File:
ui/components/routing-rules/RuleEditor.tsx
– Line 699:proOptions={{ hideAttribution: true }}remains valid under the MIT license.
• Reference: https://reactflow.dev/api-reference/types/pro-options
• Optional: see “Removing Attribution” guide for best practices.Likely an incorrect or invalid review comment.
| "next-themes": "^0.4.6", | ||
| "react": "^19.0.0", | ||
| "react-dom": "^19.0.0", | ||
| "reactflow": "^11.11.4", |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Consider code-splitting the heavy reactflow dependency
reactflow pulls in ~200 KB (min+gzip). Unless you guard it behind a dynamic import inside the rule-editor page, this will inflate the baseline bundle for every route.
You can keep TTI low by:
-import ReactFlow from 'reactflow'
+const ReactFlow = dynamic(() => import('reactflow'), { ssr: false })and moving the import inside the component that actually needs it.
Also verify whether the caret range ^11.11.4 risks pulling a future major (v12) with breaking changes; consider pinning ("11.11.4") or adding an upper bound.
🏁 Script executed:
#!/bin/bash
set -e
echo "🔍 Searching for ReactFlow imports in the repository..."
rg -n "import .*reactflow" -n .
echo
echo "📦 Fetching reactflow@11.11.4 bundle size from Bundlephobia..."
curl -s "https://bundlephobia.com/api/size?package=reactflow@11.11.4" | jq '{gzip:.gzip, size:.size}'Length of output: 530
🏁 Script executed:
#!/bin/bash
set -e
echo "📂 Showing dependencies in ui/package.json..."
sed -n '1,200p' ui/package.json
echo
echo "🔍 Checking for static ReactFlow component imports in RuleEditor.tsx..."
sed -n '1,50p' ui/components/routing-rules/RuleEditor.tsx | sed -n '1,20p'
echo
echo "🔍 Searching for 'dynamic(' usage across ui..."
rg -n "dynamic\(" -n uiLength of output: 3368
Implement dynamic loading for ReactFlow in RuleEditor
To avoid inflating the initial bundle with ReactFlow (~159 KB uncompressed, ~50 KB gzipped), load it only where it’s needed:
• File: ui/components/routing-rules/RuleEditor.tsx
– Replace the top-level imports of ReactFlow and its styles with a dynamic import.
– Example diff:
-import ReactFlow, {
- Background,
- Controls,
- Handle,
- Position,
- addEdge,
- useEdgesState,
- useNodesState,
- type Connection,
- type Edge,
- type Node,
- type NodeTypes
-} from 'reactflow'
-import 'reactflow/dist/style.css'
+import dynamic from 'next/dynamic'
+
+const ReactFlow = dynamic(
+ () => import('reactflow').then((mod) => mod.default),
+ { ssr: false }
+)
+import 'reactflow/dist/style.css'• Move any ReactFlow-specific logic inside your component so it isn’t parsed on routes that don’t render the editor.
Version range note: ^11.11.4 already forbids fetching v12 (semver <12.0.0), so no upper-bound change is needed.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In ui/components/routing-rules/RuleEditor.tsx around the top-level imports,
replace the static import of ReactFlow and its styles with a dynamic import
using React.lazy or a similar mechanism. Move all ReactFlow-specific logic
inside the component function to ensure it is only loaded and parsed when the
RuleEditor is rendered, preventing it from inflating the initial bundle size.
| import RoutingList from '@/components/routing-rules/RoutingList' | ||
|
|
||
| export default function RoutingPage() { | ||
| return <RoutingList /> | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Add explicit page metadata
Next.js app-router pages benefit from exporting metadata (or a generateMetadata) for title & description, e.g.:
export const metadata = {
title: 'Routing rules',
description: 'Manage API request routing logic',
}Helps SEO, improves tab captions, and keeps parity with other pages.
🤖 Prompt for AI Agents
In ui/app/routing/page.tsx lines 1 to 5, add an explicit export of a metadata
object with title and description properties for the page. This metadata should
include a title like 'Routing rules' and a description such as 'Manage API
request routing logic' to improve SEO and tab captions. Add this export above or
alongside the RoutingPage component export.
| <div className="text-sm text-gray-500"> | ||
| {new Date(rule.updated_at).toLocaleDateString()} | ||
| </div> |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider using a consistent date format.
Using toLocaleDateString() without options can produce inconsistent formats across different locales and browsers. Consider using a date formatting library or specifying locale and format options.
-{new Date(rule.updated_at).toLocaleDateString()}
+{new Date(rule.updated_at).toLocaleDateString('en-US', {
+ year: 'numeric',
+ month: 'short',
+ day: 'numeric'
+})}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div className="text-sm text-gray-500"> | |
| {new Date(rule.updated_at).toLocaleDateString()} | |
| </div> | |
| <div className="text-sm text-gray-500"> | |
| {new Date(rule.updated_at).toLocaleDateString('en-US', { | |
| year: 'numeric', | |
| month: 'short', | |
| day: 'numeric', | |
| })} | |
| </div> |
🤖 Prompt for AI Agents
In ui/components/routing-rules/RoutingList.tsx around lines 248 to 250, the use
of toLocaleDateString() without specifying locale or format options can lead to
inconsistent date displays across different environments. Update the code to use
a consistent date format by either specifying locale and formatting options in
toLocaleDateString or by integrating a date formatting library like date-fns or
moment.js to format the date uniformly.
| let nodeId = 0 | ||
| const getNodeId = () => `node_${nodeId++}` |
There was a problem hiding this comment.
Avoid global state for node ID generation.
The global nodeId variable could cause issues if multiple RuleEditor instances are rendered. Consider using component-scoped state or a more robust ID generation approach.
-let nodeId = 0
-const getNodeId = () => `node_${nodeId++}`
+const getNodeId = () => `node_${Date.now()}_${Math.random().toString(36).substr(2, 9)}`🤖 Prompt for AI Agents
In ui/components/routing-rules/RuleEditor.tsx around lines 441 to 442, avoid
using the global variable nodeId for generating node IDs as it can cause
conflicts when multiple RuleEditor instances are rendered. Refactor to use
component-scoped state or a unique ID generation method such as React's useState
or useRef hooks to maintain the node ID counter within each component instance,
ensuring IDs are unique and isolated per instance.
| export interface CreateRoutingRuleRequest { | ||
| name: string | ||
| description?: string | ||
| enabled: boolean | ||
| priority: number | ||
| condition_groups: ConditionGroup[] | ||
| group_operator: LogicalOperator | ||
| actions: RoutingAction[] | ||
| } | ||
|
|
||
| export interface UpdateRoutingRuleRequest extends CreateRoutingRuleRequest { | ||
| id: string | ||
| } | ||
|
|
||
| export interface ListRoutingRulesResponse { | ||
| rules: RoutingRule[] | ||
| total: number | ||
| } No newline at end of file |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Well-structured API interfaces with minor considerations.
The request/response interfaces follow good REST API patterns and properly separate creation from update operations.
Consider whether id should be included in UpdateRoutingRuleRequest body, as REST APIs typically pass the ID in the URL path rather than the request body. This depends on your API design preferences.
Also apply the same naming convention fixes mentioned for RoutingRule to these interfaces for consistency.
🤖 Prompt for AI Agents
In ui/lib/types/routing.ts around lines 61 to 78, consider removing the 'id'
field from the UpdateRoutingRuleRequest interface since REST APIs usually pass
the resource ID in the URL path, not the request body. Also, review and apply
consistent naming conventions to these interfaces as done for RoutingRule,
ensuring uniform style across all related types.
| export type ConditionOperator = | ||
| | "equals" | ||
| | "not_equals" | ||
| | "contains" | ||
| | "not_contains" | ||
| | "starts_with" | ||
| | "ends_with" | ||
| | "exists" | ||
| | "not_exists" | ||
| | "regex_match" | ||
|
|
||
| export type ConditionType = | ||
| | "header" | ||
| | "path" | ||
| | "method" | ||
| | "query_param" | ||
| | "body_contains" | ||
|
|
||
| export interface RoutingCondition { | ||
| id: string | ||
| type: ConditionType | ||
| field: string // header name, query param name, etc. | ||
| operator: ConditionOperator | ||
| value: string | ||
| } | ||
|
|
||
| export type LogicalOperator = "AND" | "OR" | ||
|
|
||
| export interface ConditionGroup { | ||
| id: string | ||
| operator: LogicalOperator | ||
| conditions: RoutingCondition[] | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Well-designed condition type system with comprehensive operator support.
The condition types provide good coverage of HTTP request components and comparison operations. The structure properly separates concerns between individual conditions and grouped logical operations.
Consider these optional refinements:
- Some operators like "exists" and "not_exists" may not require a
valuefield inRoutingCondition - Consider adding validation constraints or documentation for
fieldnaming conventions - Be mindful that
regex_matchoperations could be performance-sensitive and security-relevant
🤖 Prompt for AI Agents
In ui/lib/types/routing.ts between lines 3 and 35, update the RoutingCondition
interface to make the value field optional for operators like "exists" and
"not_exists" that do not require a value. Add comments or documentation to
clarify expected naming conventions for the field property. Optionally, include
notes or warnings about potential performance and security considerations when
using the "regex_match" operator. These changes improve clarity and correctness
without altering the existing type structure.
| export interface RoutingRule { | ||
| id: string | ||
| name: string | ||
| description?: string | ||
| enabled: boolean | ||
| priority: number | ||
| condition_groups: ConditionGroup[] | ||
| group_operator: LogicalOperator // How to combine condition groups | ||
| actions: RoutingAction[] | ||
| created_at: string | ||
| updated_at: string | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Address naming convention inconsistency.
The interface structure is well-designed for complex routing logic, but there's an inconsistency in naming conventions.
Apply consistent camelCase naming:
- condition_groups: ConditionGroup[]
- group_operator: LogicalOperator // How to combine condition groups
+ conditionGroups: ConditionGroup[]
+ groupOperator: LogicalOperator // How to combine condition groupsThe same changes should be applied to related interfaces (CreateRoutingRuleRequest, UpdateRoutingRuleRequest) to maintain consistency.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export interface RoutingRule { | |
| id: string | |
| name: string | |
| description?: string | |
| enabled: boolean | |
| priority: number | |
| condition_groups: ConditionGroup[] | |
| group_operator: LogicalOperator // How to combine condition groups | |
| actions: RoutingAction[] | |
| created_at: string | |
| updated_at: string | |
| } | |
| export interface RoutingRule { | |
| id: string | |
| name: string | |
| description?: string | |
| enabled: boolean | |
| priority: number | |
| conditionGroups: ConditionGroup[] | |
| groupOperator: LogicalOperator // How to combine condition groups | |
| actions: RoutingAction[] | |
| created_at: string | |
| updated_at: string | |
| } |
🤖 Prompt for AI Agents
In ui/lib/types/routing.ts around lines 48 to 59, the interface RoutingRule uses
snake_case for some property names like condition_groups and group_operator,
which is inconsistent with camelCase naming conventions used elsewhere. Rename
these properties to conditionGroups and groupOperator respectively. Also, apply
the same camelCase naming convention changes to related interfaces
CreateRoutingRuleRequest and UpdateRoutingRuleRequest to ensure consistency
across the codebase.
| export type ActionType = "route_to_provider" | "route_to_model" | "set_header" | "reject" | ||
|
|
||
| export interface RoutingAction { | ||
| id: string | ||
| type: ActionType | ||
| provider?: string | ||
| model?: string | ||
| headers?: Record<string, string> | ||
| priority?: number | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using discriminated unions for better type safety.
The action system covers essential routing scenarios, but the current structure with optional fields could lead to runtime errors if required fields are missing for specific action types.
Consider refactoring to a discriminated union pattern:
-export interface RoutingAction {
- id: string
- type: ActionType
- provider?: string
- model?: string
- headers?: Record<string, string>
- priority?: number
-}
+export type RoutingAction =
+ | {
+ id: string
+ type: "route_to_provider"
+ provider: string
+ priority?: number
+ }
+ | {
+ id: string
+ type: "route_to_model"
+ model: string
+ priority?: number
+ }
+ | {
+ id: string
+ type: "set_header"
+ headers: Record<string, string>
+ priority?: number
+ }
+ | {
+ id: string
+ type: "reject"
+ priority?: number
+ }Also clarify the relationship between action-level priority and rule-level priority in documentation.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export type ActionType = "route_to_provider" | "route_to_model" | "set_header" | "reject" | |
| export interface RoutingAction { | |
| id: string | |
| type: ActionType | |
| provider?: string | |
| model?: string | |
| headers?: Record<string, string> | |
| priority?: number | |
| } | |
| export type ActionType = "route_to_provider" | "route_to_model" | "set_header" | "reject" | |
| export type RoutingAction = | |
| | { | |
| id: string | |
| type: "route_to_provider" | |
| provider: string | |
| priority?: number | |
| } | |
| | { | |
| id: string | |
| type: "route_to_model" | |
| model: string | |
| priority?: number | |
| } | |
| | { | |
| id: string | |
| type: "set_header" | |
| headers: Record<string, string> | |
| priority?: number | |
| } | |
| | { | |
| id: string | |
| type: "reject" | |
| priority?: number | |
| } |
🤖 Prompt for AI Agents
In ui/lib/types/routing.ts around lines 37 to 46, refactor the RoutingAction
interface into a discriminated union type where each action type has its own
interface with required fields instead of optional ones. Define separate
interfaces for "route_to_provider", "route_to_model", "set_header", and "reject"
actions, each specifying the required properties for that action type. Then
create a union type of these interfaces for RoutingAction. Additionally, add
documentation clarifying how the action-level priority relates to the rule-level
priority.
3218cde to
690115e
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (15)
ui/package.json (1)
49-49: Bundle size concern for ReactFlow already raisedThe addition of ReactFlow dependency is necessary for the routing rules visual editor, but there's already a valid recommendation to implement dynamic loading to avoid bundle size inflation.
ui/app/routing/page.tsx (1)
1-5: Add page metadata for better SEO and UXThis page should export metadata for improved SEO, tab captions, and consistency with Next.js best practices.
export const metadata = { title: 'Routing Rules', description: 'Manage API request routing logic' }ui/app/routing/new/page.tsx (1)
45-51: Remove redundant error handling in the catch blockThe
apiService.createRoutingRulealready handles errors and returns them in the tuple format. The try-catch block with a generic error message might hide specific API errors that are already being handled.ui/components/routing-rules/RoutingList.tsx (2)
92-94: Consider using a modal dialog instead of native confirmThe native
confirm()dialog blocks the UI thread and doesn't match your app's design system. Consider using a proper modal dialog component for better UX.
248-250: Consider using a consistent date formatUsing
toLocaleDateString()without options can produce inconsistent formats across different locales and browsers. Consider using a date formatting library or specifying locale and format options.ui/components/ui/breadcrumb.tsx (1)
52-63: Fix accessibility issue with BreadcrumbPage componentThe static analysis tool correctly identified that this component has
role="link"but is not focusable, which creates an accessibility barrier for keyboard users.ui/components/routing-rules/RuleEditor.tsx (5)
135-379: Reduce code duplication in node componentsAll four node components share nearly identical structure with only minor differences in colors and icons. Consider extracting a base node component to reduce duplication and improve maintainability.
441-442: Avoid global state for node ID generation.The global
nodeIdvariable could cause issues if multipleRuleEditorinstances are rendered. Consider using component-scoped state or a more robust ID generation approach.
606-609: Extract magic numbers as named constantsThe hardcoded values for node positioning should be extracted as named constants for better maintainability and clarity.
632-663: The save function ignores the visual editor stateThe
handleSavefunction creates a hardcoded routing rule instead of converting the actual visual flow (nodes and edges) into a routing rule. This makes the visual editor non-functional.
635-635: Use a more compatible UUID generation method
crypto.randomUUID()is not available in all browsers and environments. Consider using a more compatible approach.ui/lib/types/routing.ts (4)
3-35: Well-designed condition type system with comprehensive operator support.The condition types provide good coverage of HTTP request components and comparison operations. The structure properly separates concerns between individual conditions and grouped logical operations.
Consider these optional refinements:
- Some operators like "exists" and "not_exists" may not require a
valuefield inRoutingCondition- Consider adding validation constraints or documentation for
fieldnaming conventions- Be mindful that
regex_matchoperations could be performance-sensitive and security-relevant
37-46: Consider using discriminated unions for better type safety.The action system covers essential routing scenarios, but the current structure with optional fields could lead to runtime errors if required fields are missing for specific action types.
48-59: Address naming convention inconsistency.The interface structure is well-designed for complex routing logic, but there's an inconsistency in naming conventions.
61-78: Well-structured API interfaces with minor considerations.The request/response interfaces follow good REST API patterns and properly separate creation from update operations.
Consider whether
idshould be included inUpdateRoutingRuleRequestbody, as REST APIs typically pass the ID in the URL path rather than the request body. This depends on your API design preferences.Also apply the same naming convention fixes mentioned for
RoutingRuleto these interfaces for consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
ui/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (37)
ui/app/docs/page.tsx(2 hunks)ui/app/page.tsx(1 hunks)ui/app/plugins/page.tsx(7 hunks)ui/app/routing/new/page.tsx(1 hunks)ui/app/routing/page.tsx(1 hunks)ui/components/config/core-settings-list.tsx(5 hunks)ui/components/config/mcp-clients-lists.tsx(2 hunks)ui/components/config/provider-form.tsx(3 hunks)ui/components/config/providers-list.tsx(1 hunks)ui/components/logs/empty-state.tsx(1 hunks)ui/components/logs/log-detail-sheet.tsx(2 hunks)ui/components/logs/logs-table.tsx(2 hunks)ui/components/routing-rules/RoutingList.tsx(1 hunks)ui/components/routing-rules/RuleEditor.tsx(1 hunks)ui/components/sidebar.tsx(4 hunks)ui/components/ui/accordion.tsx(3 hunks)ui/components/ui/alert-dialog.tsx(3 hunks)ui/components/ui/alert.tsx(2 hunks)ui/components/ui/badge.tsx(1 hunks)ui/components/ui/breadcrumb.tsx(1 hunks)ui/components/ui/button.tsx(2 hunks)ui/components/ui/command.tsx(3 hunks)ui/components/ui/dialog.tsx(2 hunks)ui/components/ui/dropdown-menu.tsx(4 hunks)ui/components/ui/hover-card.tsx(3 hunks)ui/components/ui/input.tsx(1 hunks)ui/components/ui/popover.tsx(3 hunks)ui/components/ui/select.tsx(4 hunks)ui/components/ui/sidebar.tsx(10 hunks)ui/components/ui/skeleton.tsx(1 hunks)ui/components/ui/tabs.tsx(4 hunks)ui/components/ui/tag-input.tsx(2 hunks)ui/components/ui/textarea.tsx(1 hunks)ui/components/ui/tooltip.tsx(3 hunks)ui/lib/api.ts(2 hunks)ui/lib/types/routing.ts(1 hunks)ui/package.json(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#84
File: transports/bifrost-http/main.go:2-2
Timestamp: 2025-06-15T16:05:13.489Z
Learning: For the Bifrost project, HTTP transport integration routers for new providers (like Mistral and Ollama) are implemented in separate PRs from the core provider support, following a focused PR strategy.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#158
File: .github/workflows/transports-release.yml:30-43
Timestamp: 2025-07-18T11:12:28.852Z
Learning: Pratham-Mishra04 relies on branch protection rules and mandatory code reviews for security in the bifrost repository, preferring process controls over technical security measures like environment variable isolation for GitHub Actions workflows. All commits are reviewed before merging to main branch.
ui/app/plugins/page.tsx (2)
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#159
File: docs/usage/http-transport/README.md:0-0
Timestamp: 2025-07-18T10:52:24.927Z
Learning: In the Bifrost project, the npx wrapper (`npx -y bifrost@latest -port 8080`) correctly handles single-dash long flags like `-port` without requiring double dashes or the `--` separator. The implementation properly forwards flags to the underlying Go binary.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#97
File: transports/Dockerfile:37-42
Timestamp: 2025-06-19T09:06:25.750Z
Learning: In Docker configurations for this project, plugin-specific environment variables (like MAXIM_LOG_REPO_ID) should not be included in the Dockerfile's ENV section. The architectural goal is to keep Docker images plugin-agnostic and externalize all plugin configuration to runtime via docker run -e flags, rather than baking plugin config into the image.
ui/app/routing/new/page.tsx (1)
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#159
File: ci/npx/bin.js:111-120
Timestamp: 2025-07-18T10:54:58.381Z
Learning: Pratham-Mishra04 considers the error handling in the downloadBinary method of ci/npx/bin.js to be sufficient for the use case. The method handles HTTP response errors with `if (!res.ok)` check and `process.exit(1)`, and they prefer not to add additional try-catch blocks around the function call for edge cases.
ui/components/config/provider-form.tsx (1)
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#85
File: core/providers/anthropic.go:150-156
Timestamp: 2025-06-15T16:07:53.140Z
Learning: In the Bifrost codebase, constructor functions are allowed to mutate input ProviderConfig objects in-place (e.g., setting default BaseURL values and trimming trailing slashes). This pattern is acceptable and doesn't need to be flagged as a code review issue.
🧬 Code Graph Analysis (10)
ui/components/ui/skeleton.tsx (1)
ui/lib/utils.ts (1)
cn(4-6)
ui/components/logs/empty-state.tsx (1)
ui/components/ui/tabs.tsx (2)
Tabs(39-39)TabsList(39-39)
ui/components/ui/tag-input.tsx (1)
ui/lib/utils.ts (1)
cn(4-6)
ui/components/ui/tabs.tsx (1)
ui/lib/utils.ts (1)
cn(4-6)
ui/components/logs/log-detail-sheet.tsx (1)
ui/lib/constants/logs.ts (1)
REQUEST_TYPE_COLORS(33-38)
ui/components/ui/sidebar.tsx (2)
ui/lib/utils.ts (1)
cn(4-6)ui/components/ui/skeleton.tsx (1)
Skeleton(7-7)
ui/lib/api.ts (1)
ui/lib/types/routing.ts (4)
ListRoutingRulesResponse(75-78)RoutingRule(48-59)CreateRoutingRuleRequest(61-69)UpdateRoutingRuleRequest(71-73)
ui/components/routing-rules/RoutingList.tsx (3)
ui/hooks/use-toast.ts (1)
useToast(9-21)ui/lib/types/routing.ts (1)
RoutingRule(48-59)ui/lib/api.ts (1)
apiService(257-257)
ui/components/ui/command.tsx (1)
ui/lib/utils.ts (1)
cn(4-6)
ui/components/ui/breadcrumb.tsx (1)
ui/lib/utils.ts (1)
cn(4-6)
🪛 Biome (1.9.4)
ui/components/ui/breadcrumb.tsx
[error] 54-61: The HTML element with the interactive role "link" is not focusable.
A non-interactive HTML element that is not focusable may not be reachable for users that rely on keyboard navigation, even with an added role like "link".
Add a tabIndex attribute to make this element focusable.
(lint/a11y/useFocusableInteractive)
🔇 Additional comments (52)
ui/app/page.tsx (1)
14-14: Minor import reordering - looks good!This is a trivial change that reorders React hooks in the import statement without affecting functionality.
ui/components/ui/textarea.tsx (1)
10-10: Good styling consistency update!The change from
rounded-mdtorounded-smaligns with the broader UI consistency effort across components in this PR.ui/components/config/providers-list.tsx (1)
86-86: Consistent styling update approved!The border-radius change from
rounded-mdtorounded-smmaintains consistency with other UI components in this PR.ui/components/ui/badge.tsx (2)
3-3: Import reordering looks good!Moving the React import after the other library imports is a minor organizational change without functional impact.
8-8: Styling consistency improvement approved!The change from
rounded-mdtorounded-smin the badge component aligns with the broader UI consistency effort across the codebase.ui/components/ui/dialog.tsx (2)
52-52: UI consistency improvement approved!The border-radius change from
rounded-lgtorounded-smfollows the consistent styling pattern applied across UI components in this PR.
90-101: Export formatting improvement looks good!The improved indentation and vertical alignment of the export statements enhances code readability without affecting functionality.
ui/components/config/core-settings-list.tsx (2)
6-11: LGTM: Import reordering improves organization.The import statements have been logically reorganized for better readability and consistency.
149-149: LGTM: Consistent border-radius styling.The border-radius reduction from
rounded-lgtorounded-smcreates a more uniform visual style across form containers. This aligns well with the broader UI consistency effort.Also applies to: 173-173, 190-190, 206-206
ui/components/ui/tabs.tsx (3)
4-4: LGTM: Consistent import ordering.Moving React import after the primitive import follows a logical organization pattern.
16-16: LGTM: Unified border-radius styling.The reduction from
rounded-lg/rounded-mdtorounded-smcreates a more consistent visual style across the tabs components.Also applies to: 27-27
39-39: LGTM: Consistent export ordering.The export reordering aligns with the standardization effort across UI components.
ui/components/logs/empty-state.tsx (1)
220-221: LGTM: Consistent border-radius styling for tabs.The border-radius reduction on both the Tabs container and TabsList maintains visual consistency with the broader UI refinement effort.
ui/components/ui/hover-card.tsx (3)
4-4: LGTM: Consistent import organization.The import reordering follows the established pattern across UI components.
29-29: LGTM: Unified border-radius styling.The change from
rounded-mdtorounded-smmaintains consistency with the broader UI styling refinement.
38-38: LGTM: Standardized export ordering.The export reordering contributes to consistency across the UI component library.
ui/app/plugins/page.tsx (1)
212-212: LGTM: Comprehensive border-radius consistency.The systematic reduction from
rounded-mdtorounded-smacross code containers, alerts, and icon elements creates a unified visual style throughout the plugins page. This aligns perfectly with the broader UI consistency effort.Also applies to: 222-222, 232-232, 281-281, 299-299, 317-317, 341-341
ui/components/ui/accordion.tsx (1)
5-5: LGTM: Consistent styling improvementsThe changes improve code organization and visual consistency:
- Import ordering follows best practices (external libraries before React)
- Border-radius change from
rounded-mdtorounded-smaligns with the UI design system updates- Export reordering maintains functionality while improving readability
Also applies to: 23-23, 47-47
ui/components/ui/dropdown-menu.tsx (1)
5-5: LGTM: Consistent UI styling updatesThe changes maintain the same styling consistency pattern:
- Import reordering improves code organization
- Border-radius updates (
rounded-md→rounded-sm) align with the design system refinement- Export formatting is more compact and readable
Also applies to: 28-28, 184-184, 193-199
ui/components/ui/select.tsx (1)
5-5: LGTM: UI consistency improvementsThese changes follow the same beneficial pattern seen across UI components:
- Import organization follows best practices
- Border-radius standardization (
rounded-md→rounded-sm) improves visual consistency- Export formatting enhances readability
Also applies to: 34-34, 53-53, 137-147
ui/components/ui/command.tsx (1)
5-5: LGTM: Consistent design system updatesThe changes maintain the established pattern of UI improvements:
- Import organization follows standard conventions
- Border-radius updates enhance visual consistency across components
- Export formatting improves code readability
Also applies to: 14-14, 55-55, 114-114
ui/components/ui/input.tsx (1)
14-14: LGTM: Consistent styling updateThe border radius change from
rounded-mdtorounded-smaligns with the broader UI standardization effort across components in this PR.ui/components/ui/skeleton.tsx (1)
4-4: LGTM: Consistent styling updateThe border radius change from
rounded-mdtorounded-smmaintains visual consistency with other UI components in this PR.ui/components/ui/tag-input.tsx (2)
3-7: LGTM: Clean import organizationThe import reordering improves code organization without any functional changes.
41-41: LGTM: Consistent styling updateThe border radius change from
rounded-mdtorounded-smmaintains visual consistency with the broader UI standardization in this PR.ui/components/logs/logs-table.tsx (2)
4-9: LGTM: Improved import organizationThe import reordering and regrouping enhances code readability without affecting functionality.
89-89: LGTM: Consistent styling updateThe border radius change from
rounded-mdtorounded-smaligns with the UI standardization across components in this PR.ui/components/ui/button.tsx (1)
3-3: LGTM! Styling consistency improvements.The import reordering and border-radius changes from
rounded-mdtorounded-smalign with the broader UI consistency updates across the codebase. These changes maintain functionality while improving visual consistency.Also applies to: 9-9, 24-25
ui/components/ui/alert-dialog.tsx (1)
4-4: LGTM! Consistent styling and formatting improvements.The import reordering, border-radius change from
rounded-lgtorounded-sm, and export formatting update all contribute to better consistency across the UI components without affecting functionality.Also applies to: 7-7, 41-41, 83-85
ui/components/ui/alert.tsx (1)
2-2: LGTM! Consistent styling updates.The import reordering, border-radius change to
rounded-sm, and export reordering are all part of the broader UI consistency improvements without any functional impact.Also applies to: 7-7, 41-41
ui/components/sidebar.tsx (2)
3-3: Excellent addition of routing rules navigation.The new "Routing rules" navigation item correctly integrates the routing functionality into the sidebar with appropriate icon, URL, and description. This enables users to access the new routing rules feature introduced in this PR.
Also applies to: 61-66
159-159: LGTM! Consistent styling updates.The border-radius changes from
rounded-lgtorounded-smmaintain visual consistency with other UI components across the application.Also applies to: 205-205
ui/app/docs/page.tsx (1)
2-6: LGTM! Clean import organization and styling consistency.The import reordering, unused import removal, and border-radius change from
rounded-lgtorounded-smimprove code organization and maintain visual consistency without affecting functionality.Also applies to: 94-94
ui/components/ui/tooltip.tsx (3)
4-4: Import reordering looks good.Moving React import after Radix primitive imports improves consistency with other UI components.
31-31: Border radius standardization approved.The change from
rounded-mdtorounded-smaligns with the broader UI consistency effort across the component library.
43-44: Export statement formatting improvement.The export reordering maintains all exports while improving organization.
ui/components/config/mcp-clients-lists.tsx (2)
14-24: Import formatting improvement.The indented list style for AlertDialog component imports improves readability without affecting functionality.
115-115: Consistent border radius styling.The change from
rounded-mdtorounded-smmaintains visual consistency with other UI components in the design system.ui/components/ui/popover.tsx (3)
4-4: Import ordering consistency.Moving React import after Radix primitive imports follows the established pattern across UI components.
33-33: Border radius standardization.Updating from
rounded-mdtorounded-smmaintains consistency with the updated design system.
48-50: Export statement organization.The reordering and formatting of exports improves code organization while maintaining all exported entities.
ui/components/logs/log-detail-sheet.tsx (2)
4-12: Import organization improvement.The consolidation and logical grouping of imports improves code readability and maintainability.
103-103: Consistent border radius styling.The change from
rounded-mdtorounded-smfor the request type label maintains visual consistency with the updated design system while preserving the dynamic color styling fromREQUEST_TYPE_COLORS.ui/components/ui/sidebar.tsx (5)
6-6: Import ordering consistency.Moving React import after other library imports follows the established pattern across the UI component library.
12-26: Import statement formatting.The improved indentation and organization of Sheet and Tooltip component imports enhances readability.
247-247: Sidebar container styling update.The change from
rounded-mdtorounded-smfor the floating sidebar variant maintains consistency with the updated design system.
408-408: Comprehensive border radius standardization.The systematic update from
rounded-mdtorounded-smacross all sidebar sub-components (group labels, actions, menu buttons, badges, skeletons, and sub-buttons) creates a cohesive visual design while maintaining all functionality.Also applies to: 429-429, 477-477, 564-564, 589-589, 618-618, 623-623, 689-689
702-727: Export statement formatting improvement.The compact export formatting improves code organization while maintaining all exported entities.
ui/components/config/provider-form.tsx (1)
464-464: LGTM! Border-radius styling updates look goodThe consistent reduction of border-radius from larger values to
rounded-smaligns with the design system updates across the UI components.Also applies to: 541-541
ui/lib/api.ts (1)
144-189: LGTM! Routing rule API methods follow established patternsThe new routing rule API methods are well-implemented and consistent with the existing codebase:
- Proper error handling using the established
getErrorMessagemethod- Consistent
ServiceResponse<T>return type pattern- RESTful endpoint structure
- Appropriate HTTP methods for each operation
- Good TypeScript typing with imported interfaces
The implementation follows the same patterns as existing provider and MCP endpoints.
ui/components/ui/breadcrumb.tsx (2)
83-99: Good accessibility implementation in BreadcrumbEllipsisThe component properly uses
role="presentation",aria-hidden="true", and includes a screen-reader-only label. This follows accessibility best practices for decorative elements.
65-81: Proper separator implementationThe BreadcrumbSeparator component correctly uses
role="presentation"andaria-hidden="true"to hide decorative elements from screen readers while providing visual separation.
| const handleSave = async (rule: RoutingRule) => { | ||
| try { | ||
| const createRequest = { | ||
| name: rule.name, | ||
| description: rule.description, | ||
| enabled: rule.enabled, | ||
| priority: rule.priority, | ||
| condition_groups: rule.condition_groups, | ||
| group_operator: rule.group_operator, | ||
| actions: rule.actions | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider extracting request mapping logic
The manual mapping from RoutingRule to the create request object could be extracted into a utility function or the API service could accept the full RoutingRule object directly to reduce code duplication and improve maintainability.
+const mapRuleToCreateRequest = (rule: RoutingRule) => ({
+ name: rule.name,
+ description: rule.description,
+ enabled: rule.enabled,
+ priority: rule.priority,
+ condition_groups: rule.condition_groups,
+ group_operator: rule.group_operator,
+ actions: rule.actions
+})
const handleSave = async (rule: RoutingRule) => {
- const createRequest = {
- name: rule.name,
- description: rule.description,
- enabled: rule.enabled,
- priority: rule.priority,
- condition_groups: rule.condition_groups,
- group_operator: rule.group_operator,
- actions: rule.actions
- }
+ const createRequest = mapRuleToCreateRequest(rule)🤖 Prompt for AI Agents
In ui/app/routing/new/page.tsx around lines 16 to 26, the code manually maps
properties from the RoutingRule object to a new createRequest object. To improve
maintainability and reduce duplication, extract this mapping logic into a
separate utility function that takes a RoutingRule and returns the create
request object, then call this function inside handleSave. Alternatively, modify
the API service to accept the full RoutingRule object directly if possible.
| const updatedRule = { ...rule, enabled: !rule.enabled } | ||
| const [data, error] = await apiService.updateRoutingRule(rule.id, updatedRule) | ||
| if (error) { | ||
| toast({ | ||
| title: "Error", | ||
| description: error, | ||
| variant: "destructive", | ||
| }) | ||
| return | ||
| } | ||
|
|
||
| setRules(rules.map(r => r.id === rule.id ? { ...r, enabled: !r.enabled } : r)) | ||
| toast({ |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Optimize state update logic
The state update logic correctly uses functional updates, but consider using the actual updated rule from the API response instead of manually toggling the enabled state to ensure consistency with the server state.
-setRules(rules.map(r => r.id === rule.id ? { ...r, enabled: !r.enabled } : r))
+if (data) {
+ setRules(rules.map(r => r.id === rule.id ? data : r))
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const updatedRule = { ...rule, enabled: !rule.enabled } | |
| const [data, error] = await apiService.updateRoutingRule(rule.id, updatedRule) | |
| if (error) { | |
| toast({ | |
| title: "Error", | |
| description: error, | |
| variant: "destructive", | |
| }) | |
| return | |
| } | |
| setRules(rules.map(r => r.id === rule.id ? { ...r, enabled: !r.enabled } : r)) | |
| toast({ | |
| const updatedRule = { ...rule, enabled: !rule.enabled } | |
| const [data, error] = await apiService.updateRoutingRule(rule.id, updatedRule) | |
| if (error) { | |
| toast({ | |
| title: "Error", | |
| description: error, | |
| variant: "destructive", | |
| }) | |
| return | |
| } | |
| if (data) { | |
| setRules(rules.map(r => r.id === rule.id ? data : r)) | |
| } | |
| toast({ |
🤖 Prompt for AI Agents
In ui/components/routing-rules/RoutingList.tsx around lines 66 to 78, the state
update manually toggles the enabled property instead of using the updated rule
returned from the API, which may cause inconsistencies. Modify the setRules call
to update the rule using the actual updatedRule data received from the API
response instead of toggling enabled manually, ensuring the local state matches
the server state accurately.
| const handleEditRule = (rule: RoutingRule) => { | ||
| router.push(`/routing/edit/${rule.id}`) | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify edit route exists
The edit functionality routes to /routing/edit/${rule.id} but this route wasn't included in the files for review. Ensure this route and corresponding page component exist.
🏁 Script executed:
#!/bin/bash
# Check if edit route page exists
fd -t f "page.tsx" ui/app/routing/editLength of output: 145
🏁 Script executed:
#!/bin/bash
# List all page.tsx files and filter for 'routing'
fd -t f page.tsx | grep -i routingLength of output: 88
Add missing edit route for /routing/edit/:id
There isn’t a page component under ui/app/routing/edit or a dynamic segment for handling /routing/edit/${rule.id}, so navigating there will 404. Please add the edit route:
• Create ui/app/routing/edit/[id]/page.tsx (with your edit form/component inside)
• Ensure the folder name [id] matches the parameter used in router.push(/routing/edit/${rule.id})
🤖 Prompt for AI Agents
In ui/components/routing-rules/RoutingList.tsx around lines 60 to 62, the
handleEditRule function navigates to /routing/edit/${rule.id}, but there is no
corresponding dynamic route to handle this path, causing a 404 error. To fix
this, create a new folder ui/app/routing/edit/[id] and add a page.tsx file
inside it that contains the edit form or component for the routing rule. Ensure
the folder name [id] matches the dynamic segment used in the router.push call.
690115e to
f0a600a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
Makefile (1)
46-61: Consider breaking down long target bodies.The static analysis tool flags the target body length. While functional, consider extracting common logic into variables or helper targets to improve maintainability.
+# Common air command parameters +AIR_PARAMS = -port "$(PORT)" -plugins "$(PLUGINS)" $(if $(PROMETHEUS_LABELS),-prometheus-labels "$(PROMETHEUS_LABELS)") dev: install-ui install-air ## Start complete development environment (UI + API with proxy) @echo "$(GREEN)Starting Bifrost complete development environment...$(NC)" @echo "$(YELLOW)This will start:$(NC)" @echo " 1. UI development server (localhost:3000)" @echo " 2. API server with UI proxy (localhost:$(PORT)/ui)" @echo "$(CYAN)Access everything at: http://localhost:$(PORT)/ui$(NC)" @echo "" @echo "$(YELLOW)Starting UI development server...$(NC)" @cd ui && npm run dev & @sleep 3 @echo "$(YELLOW)Starting API server with UI proxy...$(NC)" - @cd transports/bifrost-http && BIFROST_UI_DEV=true air -c .air.toml -- \ - -port "$(PORT)" \ - -plugins "$(PLUGINS)" \ - $(if $(PROMETHEUS_LABELS),-prometheus-labels "$(PROMETHEUS_LABELS)") + @cd transports/bifrost-http && BIFROST_UI_DEV=true air -c .air.toml -- $(AIR_PARAMS)
♻️ Duplicate comments (17)
ui/package.json (1)
49-49: Consider code-splitting the heavyreactflowdependencyAs noted in previous review,
reactflowadds significant bundle size (~159 KB uncompressed, ~50 KB gzipped). Consider implementing dynamic loading in the RuleEditor component to avoid inflating the initial bundle for routes that don't use the routing rule editor.ui/app/routing/page.tsx (1)
1-5: Add explicit page metadata for SEO and accessibilityNext.js app-router pages benefit from exporting
metadatafor better SEO and tab captions:export const metadata = { title: 'Routing Rules', description: 'Manage API request routing logic', }This improves user experience and maintains consistency with other pages.
ui/app/routing/new/page.tsx (2)
17-51: Remove redundant error handling in the catch block.The
apiService.createRoutingRulealready handles errors and returns them in the tuple format. The try-catch block with a generic error message might hide specific API errors that are already being handled.
18-26: Consider extracting request mapping logic.The manual mapping from
RoutingRuleto the create request object could be extracted into a utility function or the API service could accept the fullRoutingRuleobject directly to reduce code duplication and improve maintainability.ui/components/routing-rules/RoutingList.tsx (4)
92-94: Consider using a modal dialog instead of native confirm.The native
confirm()dialog blocks the UI thread and doesn't match your app's design system. Consider using a proper modal dialog component for better UX.
248-250: Consider using a consistent date format.Using
toLocaleDateString()without options can produce inconsistent formats across different locales and browsers. Consider using a date formatting library or specifying locale and format options.
66-78: Optimize state update logic.The state update logic correctly uses functional updates, but consider using the actual updated rule from the API response instead of manually toggling the enabled state to ensure consistency with the server state.
60-62: Add missing edit route for/routing/edit/:id.There isn't a page component under
ui/app/routing/editor a dynamic segment for handling/routing/edit/${rule.id}, so navigating there will 404. Please add the edit route.#!/bin/bash # Verify if the edit route exists in the routing directory structure fd -t f "page.tsx" ui/app/routing | grep -E "(edit|\\[.*\\])"ui/components/ui/breadcrumb.tsx (1)
52-63: Fix accessibility issue with BreadcrumbPage component.The component has
role="link"but is not focusable, which creates an accessibility barrier for keyboard users.Apply this fix to make the component properly focusable:
<span data-slot="breadcrumb-page" role="link" aria-disabled="true" aria-current="page" + tabIndex={0} className={cn("text-foreground font-normal", className)} {...props} />ui/components/routing-rules/RuleEditor.tsx (5)
632-663: The save function ignores the visual editor stateThe
handleSavefunction creates a hardcoded routing rule instead of converting the actual visual flow (nodes and edges) into a routing rule. This makes the visual editor non-functional.The function should analyze the
nodesandedgesstate to build the routing rule dynamically. Would you like me to implement the logic to convert the visual flow into a proper routing rule structure?
135-379: Reduce code duplication in node componentsAll four node components share nearly identical structure with only minor differences in colors and icons. Consider extracting a base node component to reduce duplication and improve maintainability.
Create a base node component:
interface BaseNodeProps { id: string data: BaseNodeData icon: React.ComponentType<{ className?: string }> iconColor: string showTargetHandle?: boolean showSourceHandle?: boolean } const BaseNode = ({ id, data, icon: Icon, iconColor, showTargetHandle = false, showSourceHandle = true }: BaseNodeProps) => { const handleDelete = () => { if (data.onDelete) { data.onDelete(id) } } return ( <div className="relative"> <div className="absolute -top-8 right-0 z-10"> <Badge variant={data.completed ? 'default' : 'secondary'} className="text-xs"> {data.completed && <Check className="h-3 w-3 mr-1" />} {data.completed ? 'Completed' : 'Draft'} </Badge> </div> <div className="bg-card border-2 border-primary/20 rounded-lg p-4 min-w-[350px] shadow-sm relative"> {showTargetHandle && ( <Handle type="target" position={Position.Top} className="w-3 h-3 bg-primary border-2 border-background" style={{ top: -6 }} /> )} {showSourceHandle && ( <Handle type="source" position={Position.Bottom} className="w-3 h-3 bg-primary border-2 border-background" style={{ bottom: -6 }} /> )} {data.isEditable && ( <Button variant="ghost" size="icon" className="absolute top-2 right-2 h-6 w-6 hover:bg-destructive hover:text-destructive-foreground" onClick={handleDelete} > <X className="h-3 w-3" /> </Button> )} <div className="flex items-start gap-3 pr-8"> <div className={`rounded-lg p-2 mt-1 ${iconColor}`}> <Icon className="h-4 w-4" /> </div> <div className="flex-1"> <div className="flex items-center justify-between mb-1"> <h3 className="font-medium text-sm">{data.name}</h3> <Badge variant="outline" className="text-xs"> {data.service} </Badge> </div> <p className="text-muted-foreground text-xs">{data.description}</p> </div> </div> </div> </div> ) }Then simplify each node component:
const TriggerNode = ({ data, id }: { data: TriggerNodeData; id: string }) => ( <BaseNode id={id} data={data} icon={Play} iconColor="bg-blue-100 dark:bg-blue-900" showSourceHandle={true} showTargetHandle={false} /> )
635-635: Use a more compatible UUID generation method
crypto.randomUUID()is not available in all browsers and environments. Consider letting the backend generate the ID.- id: rule?.id || crypto.randomUUID(), + id: rule?.id || undefined, // Let backend generate ID for new rulesOr use a more compatible approach if client-side ID is required:
- id: rule?.id || crypto.randomUUID(), + id: rule?.id || `rule-${Date.now()}-${Math.random().toString(36).substr(2, 9)}`,
606-609: Extract magic numbers as named constantsThe hardcoded values for node positioning should be extracted as named constants for better maintainability and clarity.
+const NODE_WIDTH = 350 // matches min-w-[350px] in node components +const NODE_CENTER_OFFSET = NODE_WIDTH / 2 +const NODE_TAG_HEIGHT = 60 // accounts for badge above node + const onDrop = useCallback((event: React.DragEvent) => { event.preventDefault() if (!draggedNodeType) return const reactFlowBounds = (event.target as HTMLElement).getBoundingClientRect() const position = { - x: event.clientX - reactFlowBounds.left - 175, // Center the node - y: event.clientY - reactFlowBounds.top - 60, // Account for tag above + x: event.clientX - reactFlowBounds.left - NODE_CENTER_OFFSET, + y: event.clientY - reactFlowBounds.top - NODE_TAG_HEIGHT, }
441-442: Avoid global state for node ID generation.The global
nodeIdvariable could cause issues if multipleRuleEditorinstances are rendered. Consider using component-scoped state or a more robust ID generation approach.-let nodeId = 0 -const getNodeId = () => `node_${nodeId++}` +const getNodeId = () => `node_${Date.now()}_${Math.random().toString(36).substr(2, 9)}`ui/lib/types/routing.ts (3)
48-78: Address naming convention inconsistency.The interfaces use snake_case for some properties which is inconsistent with TypeScript/JavaScript camelCase conventions.
Apply consistent camelCase naming:
export interface RoutingRule { id: string name: string description?: string enabled: boolean priority: number - condition_groups: ConditionGroup[] - group_operator: LogicalOperator // How to combine condition groups + conditionGroups: ConditionGroup[] + groupOperator: LogicalOperator // How to combine condition groups actions: RoutingAction[] - created_at: string - updated_at: string + createdAt: string + updatedAt: string } export interface CreateRoutingRuleRequest { name: string description?: string enabled: boolean priority: number - condition_groups: ConditionGroup[] - group_operator: LogicalOperator + conditionGroups: ConditionGroup[] + groupOperator: LogicalOperator actions: RoutingAction[] }Also consider whether
idshould be included inUpdateRoutingRuleRequestbody, as REST APIs typically pass the ID in the URL path rather than the request body.
37-46: Consider using discriminated unions for better type safety.The action system has optional fields that could lead to runtime errors if required fields are missing for specific action types.
Consider refactoring to a discriminated union pattern:
export type RoutingAction = | { id: string type: "route_to_provider" provider: string priority?: number } | { id: string type: "route_to_model" model: string priority?: number } | { id: string type: "set_header" headers: Record<string, string> priority?: number } | { id: string type: "reject" priority?: number }Also clarify the relationship between action-level
priorityand rule-levelpriorityin documentation.
21-27: Make value field optional for certain operators.Some operators like "exists" and "not_exists" don't require a value, but the interface makes it required.
export interface RoutingCondition { id: string type: ConditionType field: string // header name, query param name, etc. operator: ConditionOperator - value: string + value?: string // Optional for operators like "exists" and "not_exists" }Consider adding validation or documentation to clarify which operators require values.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
ui/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (40)
Makefile(2 hunks)transports/bifrost-http/.air.toml(2 hunks)transports/bifrost-http/main.go(3 hunks)ui/app/docs/page.tsx(2 hunks)ui/app/page.tsx(1 hunks)ui/app/plugins/page.tsx(7 hunks)ui/app/routing/new/page.tsx(1 hunks)ui/app/routing/page.tsx(1 hunks)ui/components/config/core-settings-list.tsx(5 hunks)ui/components/config/mcp-clients-lists.tsx(2 hunks)ui/components/config/provider-form.tsx(3 hunks)ui/components/config/providers-list.tsx(1 hunks)ui/components/logs/empty-state.tsx(1 hunks)ui/components/logs/log-detail-sheet.tsx(2 hunks)ui/components/logs/logs-table.tsx(2 hunks)ui/components/routing-rules/RoutingList.tsx(1 hunks)ui/components/routing-rules/RuleEditor.tsx(1 hunks)ui/components/sidebar.tsx(5 hunks)ui/components/ui/accordion.tsx(3 hunks)ui/components/ui/alert-dialog.tsx(3 hunks)ui/components/ui/alert.tsx(2 hunks)ui/components/ui/badge.tsx(1 hunks)ui/components/ui/breadcrumb.tsx(1 hunks)ui/components/ui/button.tsx(2 hunks)ui/components/ui/command.tsx(3 hunks)ui/components/ui/dialog.tsx(2 hunks)ui/components/ui/dropdown-menu.tsx(4 hunks)ui/components/ui/hover-card.tsx(3 hunks)ui/components/ui/input.tsx(1 hunks)ui/components/ui/popover.tsx(3 hunks)ui/components/ui/select.tsx(4 hunks)ui/components/ui/sidebar.tsx(10 hunks)ui/components/ui/skeleton.tsx(1 hunks)ui/components/ui/tabs.tsx(4 hunks)ui/components/ui/tag-input.tsx(2 hunks)ui/components/ui/textarea.tsx(1 hunks)ui/components/ui/tooltip.tsx(3 hunks)ui/lib/api.ts(2 hunks)ui/lib/types/routing.ts(1 hunks)ui/package.json(1 hunks)
🧠 Learnings (7)
📓 Common learnings
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#84
File: transports/bifrost-http/main.go:2-2
Timestamp: 2025-06-15T16:05:13.489Z
Learning: For the Bifrost project, HTTP transport integration routers for new providers (like Mistral and Ollama) are implemented in separate PRs from the core provider support, following a focused PR strategy.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#158
File: .github/workflows/transports-release.yml:30-43
Timestamp: 2025-07-18T11:12:28.861Z
Learning: Pratham-Mishra04 relies on branch protection rules and mandatory code reviews for security in the bifrost repository, preferring process controls over technical security measures like environment variable isolation for GitHub Actions workflows. All commits are reviewed before merging to main branch.
transports/bifrost-http/.air.toml (3)
Learnt from: Pratham-Mishra04
PR: #103
File: .github/workflows/transport-dependency-update.yml:53-75
Timestamp: 2025-06-20T16:21:18.912Z
Learning: In the bifrost repository's transport dependency update workflow, when updating the core dependency to a new version using go get, the go.mod and go.sum files will always change in normal operation, making the safety check for changes more of a defensive programming practice rather than handling a common scenario.
Learnt from: Pratham-Mishra04
PR: #159
File: ci/npx/package.json:22-23
Timestamp: 2025-07-18T11:07:56.674Z
Learning: In the Bifrost project, Pratham-Mishra04 publishes the npm package by running npm publish from inside the ci/npx directory rather than from the repository root, and prefers not to use a files field to restrict the published package contents.
Learnt from: Pratham-Mishra04
PR: #149
File: docs/contributing/README.md:22-27
Timestamp: 2025-07-08T18:12:13.590Z
Learning: In the Bifrost project, the tests directory structure has tests/core-providers/ and tests/transports-integrations/ as sibling directories. From tests/core-providers/, the correct relative path to reach tests/transports-integrations/ is ../transports-integrations/, not ../../tests/transports-integrations/.
transports/bifrost-http/main.go (6)
Learnt from: Pratham-Mishra04
PR: #149
File: transports/bifrost-http/main.go:259-261
Timestamp: 2025-07-08T18:20:24.086Z
Learning: In transports/bifrost-http/lib/store.go, the LoadFromConfig method already handles missing config files gracefully by initializing defaults and auto-detecting providers from environment variables. It only returns errors for actual parsing/processing failures, not missing files, making log.Fatalf appropriate in main.go since real errors should terminate the program.
Learnt from: Pratham-Mishra04
PR: #103
File: .github/workflows/transport-dependency-update.yml:53-75
Timestamp: 2025-06-20T16:21:18.912Z
Learning: In the bifrost repository's transport dependency update workflow, when updating the core dependency to a new version using go get, the go.mod and go.sum files will always change in normal operation, making the safety check for changes more of a defensive programming practice rather than handling a common scenario.
Learnt from: Pratham-Mishra04
PR: #135
File: docs/core-package.md:105-116
Timestamp: 2025-06-27T17:07:39.462Z
Learning: In Go, when importing a package, the identifier used to access it is determined by the package declaration in the Go source files, not the directory name. For the Bifrost project, the core directory files declare package bifrost, so importing "github.com/maximhq/bifrost/core" automatically makes it accessible as bifrost.Init, bifrost.Ptr, etc., without needing an explicit alias.
Learnt from: Pratham-Mishra04
PR: #81
File: tests/core-providers/openai_test.go:1-2
Timestamp: 2025-06-16T04:29:53.409Z
Learning: In the Bifrost project, the user prefers to use package main for test files in the tests/core-providers directory rather than more descriptive package names like coreproviders_test.
Learnt from: Pratham-Mishra04
PR: #65
File: transports/bifrost-http/integrations/anthropic/types.go:140-146
Timestamp: 2025-06-10T13:51:52.859Z
Learning: In Bifrost core v1.0.9, ImageContent.Type was a pointer type (*string accessed via bifrost.Ptr), but in v1.0.10 it was changed to a value type (ImageContentType). When reviewing code, check the core version being used to determine the correct assignment pattern.
Learnt from: Pratham-Mishra04
PR: #152
File: transports/bifrost-http/plugins/logging/utils.go:378-399
Timestamp: 2025-07-10T13:44:14.518Z
Learning: In the Bifrost logging plugin (transports/bifrost-http/plugins/logging/utils.go), Pratham-Mishra04 prefers not to add error handling for JSON unmarshaling operations, considering logging not very critical and being confident that JSON marshalling won't fail in practice.
ui/app/plugins/page.tsx (2)
Learnt from: Pratham-Mishra04
PR: #159
File: docs/usage/http-transport/README.md:0-0
Timestamp: 2025-07-18T10:52:24.942Z
Learning: In the Bifrost project, the npx wrapper (npx -y bifrost@latest -port 8080) correctly handles single-dash long flags like -port without requiring double dashes or the -- separator. The implementation properly forwards flags to the underlying Go binary.
Learnt from: Pratham-Mishra04
PR: #97
File: transports/Dockerfile:37-42
Timestamp: 2025-06-19T09:06:25.750Z
Learning: In Docker configurations for this project, plugin-specific environment variables (like MAXIM_LOG_REPO_ID) should not be included in the Dockerfile's ENV section. The architectural goal is to keep Docker images plugin-agnostic and externalize all plugin configuration to runtime via docker run -e flags, rather than baking plugin config into the image.
ui/app/routing/new/page.tsx (1)
Learnt from: Pratham-Mishra04
PR: #159
File: ci/npx/bin.js:111-120
Timestamp: 2025-07-18T10:54:58.405Z
Learning: Pratham-Mishra04 considers the error handling in the downloadBinary method of ci/npx/bin.js to be sufficient for the use case. The method handles HTTP response errors with if (!res.ok) check and process.exit(1), and they prefer not to add additional try-catch blocks around the function call for edge cases.
Makefile (1)
Learnt from: Pratham-Mishra04
PR: #159
File: docs/usage/http-transport/README.md:0-0
Timestamp: 2025-07-18T10:52:24.942Z
Learning: In the Bifrost project, the npx wrapper (npx -y bifrost@latest -port 8080) correctly handles single-dash long flags like -port without requiring double dashes or the -- separator. The implementation properly forwards flags to the underlying Go binary.
ui/components/config/provider-form.tsx (1)
Learnt from: Pratham-Mishra04
PR: #85
File: core/providers/anthropic.go:150-156
Timestamp: 2025-06-15T16:07:53.140Z
Learning: In the Bifrost codebase, constructor functions are allowed to mutate input ProviderConfig objects in-place (e.g., setting default BaseURL values and trimming trailing slashes). This pattern is acceptable and doesn't need to be flagged as a code review issue.
🧬 Code Graph Analysis (12)
ui/components/ui/skeleton.tsx (1)
ui/lib/utils.ts (1)
cn(4-6)
ui/components/logs/empty-state.tsx (1)
ui/components/ui/tabs.tsx (2)
Tabs(39-39)TabsList(39-39)
ui/components/ui/tag-input.tsx (1)
ui/lib/utils.ts (1)
cn(4-6)
ui/app/routing/page.tsx (1)
ui/components/routing-rules/RoutingList.tsx (1)
RoutingList(22-278)
ui/components/ui/tabs.tsx (1)
ui/lib/utils.ts (1)
cn(4-6)
ui/app/routing/new/page.tsx (6)
ui/hooks/use-toast.ts (1)
useToast(9-21)ui/lib/types/routing.ts (1)
RoutingRule(48-59)ui/lib/api.ts (1)
apiService(257-257)ui/components/ui/card.tsx (3)
CardHeader(50-50)CardTitle(50-50)CardDescription(50-50)ui/components/ui/breadcrumb.tsx (6)
Breadcrumb(102-102)BreadcrumbList(103-103)BreadcrumbItem(104-104)BreadcrumbLink(105-105)BreadcrumbSeparator(107-107)BreadcrumbPage(106-106)ui/components/routing-rules/RuleEditor.tsx (1)
RuleEditor(444-718)
ui/components/logs/log-detail-sheet.tsx (1)
ui/lib/constants/logs.ts (1)
REQUEST_TYPE_COLORS(33-38)
ui/components/ui/sidebar.tsx (1)
ui/lib/utils.ts (1)
cn(4-6)
ui/components/ui/command.tsx (1)
ui/lib/utils.ts (1)
cn(4-6)
ui/components/routing-rules/RoutingList.tsx (8)
ui/hooks/use-toast.ts (1)
useToast(9-21)ui/lib/types/routing.ts (1)
RoutingRule(48-59)ui/lib/api.ts (1)
apiService(257-257)ui/components/ui/card.tsx (3)
CardHeader(50-50)CardTitle(50-50)CardDescription(50-50)ui/components/ui/button.tsx (1)
Button(71-71)ui/components/ui/badge.tsx (1)
Badge(37-37)ui/components/ui/table.tsx (6)
Table(64-64)TableHeader(64-64)TableRow(64-64)TableHead(64-64)TableBody(64-64)TableCell(64-64)ui/components/ui/switch.tsx (1)
Switch(29-29)
ui/components/ui/breadcrumb.tsx (1)
ui/lib/utils.ts (1)
cn(4-6)
ui/lib/api.ts (1)
ui/lib/types/routing.ts (4)
ListRoutingRulesResponse(75-78)RoutingRule(48-59)CreateRoutingRuleRequest(61-69)UpdateRoutingRuleRequest(71-73)
🪛 checkmake (0.2.2)
Makefile
[warning] 46-46: Target body for "dev" exceeds allowed length of 5 (14).
(maxbodylength)
🪛 Biome (1.9.4)
ui/components/ui/breadcrumb.tsx
[error] 54-61: The HTML element with the interactive role "link" is not focusable.
A non-interactive HTML element that is not focusable may not be reachable for users that rely on keyboard navigation, even with an added role like "link".
Add a tabIndex attribute to make this element focusable.
(lint/a11y/useFocusableInteractive)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#84
File: transports/bifrost-http/main.go:2-2
Timestamp: 2025-06-15T16:05:13.489Z
Learning: For the Bifrost project, HTTP transport integration routers for new providers (like Mistral and Ollama) are implemented in separate PRs from the core provider support, following a focused PR strategy.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#158
File: .github/workflows/transports-release.yml:30-43
Timestamp: 2025-07-18T11:12:28.861Z
Learning: Pratham-Mishra04 relies on branch protection rules and mandatory code reviews for security in the bifrost repository, preferring process controls over technical security measures like environment variable isolation for GitHub Actions workflows. All commits are reviewed before merging to main branch.
transports/bifrost-http/.air.toml (3)
Learnt from: Pratham-Mishra04
PR: #103
File: .github/workflows/transport-dependency-update.yml:53-75
Timestamp: 2025-06-20T16:21:18.912Z
Learning: In the bifrost repository's transport dependency update workflow, when updating the core dependency to a new version using go get, the go.mod and go.sum files will always change in normal operation, making the safety check for changes more of a defensive programming practice rather than handling a common scenario.
Learnt from: Pratham-Mishra04
PR: #159
File: ci/npx/package.json:22-23
Timestamp: 2025-07-18T11:07:56.674Z
Learning: In the Bifrost project, Pratham-Mishra04 publishes the npm package by running npm publish from inside the ci/npx directory rather than from the repository root, and prefers not to use a files field to restrict the published package contents.
Learnt from: Pratham-Mishra04
PR: #149
File: docs/contributing/README.md:22-27
Timestamp: 2025-07-08T18:12:13.590Z
Learning: In the Bifrost project, the tests directory structure has tests/core-providers/ and tests/transports-integrations/ as sibling directories. From tests/core-providers/, the correct relative path to reach tests/transports-integrations/ is ../transports-integrations/, not ../../tests/transports-integrations/.
transports/bifrost-http/main.go (6)
Learnt from: Pratham-Mishra04
PR: #149
File: transports/bifrost-http/main.go:259-261
Timestamp: 2025-07-08T18:20:24.086Z
Learning: In transports/bifrost-http/lib/store.go, the LoadFromConfig method already handles missing config files gracefully by initializing defaults and auto-detecting providers from environment variables. It only returns errors for actual parsing/processing failures, not missing files, making log.Fatalf appropriate in main.go since real errors should terminate the program.
Learnt from: Pratham-Mishra04
PR: #103
File: .github/workflows/transport-dependency-update.yml:53-75
Timestamp: 2025-06-20T16:21:18.912Z
Learning: In the bifrost repository's transport dependency update workflow, when updating the core dependency to a new version using go get, the go.mod and go.sum files will always change in normal operation, making the safety check for changes more of a defensive programming practice rather than handling a common scenario.
Learnt from: Pratham-Mishra04
PR: #135
File: docs/core-package.md:105-116
Timestamp: 2025-06-27T17:07:39.462Z
Learning: In Go, when importing a package, the identifier used to access it is determined by the package declaration in the Go source files, not the directory name. For the Bifrost project, the core directory files declare package bifrost, so importing "github.com/maximhq/bifrost/core" automatically makes it accessible as bifrost.Init, bifrost.Ptr, etc., without needing an explicit alias.
Learnt from: Pratham-Mishra04
PR: #81
File: tests/core-providers/openai_test.go:1-2
Timestamp: 2025-06-16T04:29:53.409Z
Learning: In the Bifrost project, the user prefers to use package main for test files in the tests/core-providers directory rather than more descriptive package names like coreproviders_test.
Learnt from: Pratham-Mishra04
PR: #65
File: transports/bifrost-http/integrations/anthropic/types.go:140-146
Timestamp: 2025-06-10T13:51:52.859Z
Learning: In Bifrost core v1.0.9, ImageContent.Type was a pointer type (*string accessed via bifrost.Ptr), but in v1.0.10 it was changed to a value type (ImageContentType). When reviewing code, check the core version being used to determine the correct assignment pattern.
Learnt from: Pratham-Mishra04
PR: #152
File: transports/bifrost-http/plugins/logging/utils.go:378-399
Timestamp: 2025-07-10T13:44:14.518Z
Learning: In the Bifrost logging plugin (transports/bifrost-http/plugins/logging/utils.go), Pratham-Mishra04 prefers not to add error handling for JSON unmarshaling operations, considering logging not very critical and being confident that JSON marshalling won't fail in practice.
ui/app/plugins/page.tsx (2)
Learnt from: Pratham-Mishra04
PR: #159
File: docs/usage/http-transport/README.md:0-0
Timestamp: 2025-07-18T10:52:24.942Z
Learning: In the Bifrost project, the npx wrapper (npx -y bifrost@latest -port 8080) correctly handles single-dash long flags like -port without requiring double dashes or the -- separator. The implementation properly forwards flags to the underlying Go binary.
Learnt from: Pratham-Mishra04
PR: #97
File: transports/Dockerfile:37-42
Timestamp: 2025-06-19T09:06:25.750Z
Learning: In Docker configurations for this project, plugin-specific environment variables (like MAXIM_LOG_REPO_ID) should not be included in the Dockerfile's ENV section. The architectural goal is to keep Docker images plugin-agnostic and externalize all plugin configuration to runtime via docker run -e flags, rather than baking plugin config into the image.
ui/app/routing/new/page.tsx (1)
Learnt from: Pratham-Mishra04
PR: #159
File: ci/npx/bin.js:111-120
Timestamp: 2025-07-18T10:54:58.405Z
Learning: Pratham-Mishra04 considers the error handling in the downloadBinary method of ci/npx/bin.js to be sufficient for the use case. The method handles HTTP response errors with if (!res.ok) check and process.exit(1), and they prefer not to add additional try-catch blocks around the function call for edge cases.
Makefile (1)
Learnt from: Pratham-Mishra04
PR: #159
File: docs/usage/http-transport/README.md:0-0
Timestamp: 2025-07-18T10:52:24.942Z
Learning: In the Bifrost project, the npx wrapper (npx -y bifrost@latest -port 8080) correctly handles single-dash long flags like -port without requiring double dashes or the -- separator. The implementation properly forwards flags to the underlying Go binary.
ui/components/config/provider-form.tsx (1)
Learnt from: Pratham-Mishra04
PR: #85
File: core/providers/anthropic.go:150-156
Timestamp: 2025-06-15T16:07:53.140Z
Learning: In the Bifrost codebase, constructor functions are allowed to mutate input ProviderConfig objects in-place (e.g., setting default BaseURL values and trimming trailing slashes). This pattern is acceptable and doesn't need to be flagged as a code review issue.
🧬 Code Graph Analysis (12)
ui/components/ui/skeleton.tsx (1)
ui/lib/utils.ts (1)
cn(4-6)
ui/components/logs/empty-state.tsx (1)
ui/components/ui/tabs.tsx (2)
Tabs(39-39)TabsList(39-39)
ui/components/ui/tag-input.tsx (1)
ui/lib/utils.ts (1)
cn(4-6)
ui/app/routing/page.tsx (1)
ui/components/routing-rules/RoutingList.tsx (1)
RoutingList(22-278)
ui/components/ui/tabs.tsx (1)
ui/lib/utils.ts (1)
cn(4-6)
ui/app/routing/new/page.tsx (6)
ui/hooks/use-toast.ts (1)
useToast(9-21)ui/lib/types/routing.ts (1)
RoutingRule(48-59)ui/lib/api.ts (1)
apiService(257-257)ui/components/ui/card.tsx (3)
CardHeader(50-50)CardTitle(50-50)CardDescription(50-50)ui/components/ui/breadcrumb.tsx (6)
Breadcrumb(102-102)BreadcrumbList(103-103)BreadcrumbItem(104-104)BreadcrumbLink(105-105)BreadcrumbSeparator(107-107)BreadcrumbPage(106-106)ui/components/routing-rules/RuleEditor.tsx (1)
RuleEditor(444-718)
ui/components/logs/log-detail-sheet.tsx (1)
ui/lib/constants/logs.ts (1)
REQUEST_TYPE_COLORS(33-38)
ui/components/ui/sidebar.tsx (1)
ui/lib/utils.ts (1)
cn(4-6)
ui/components/ui/command.tsx (1)
ui/lib/utils.ts (1)
cn(4-6)
ui/components/routing-rules/RoutingList.tsx (8)
ui/hooks/use-toast.ts (1)
useToast(9-21)ui/lib/types/routing.ts (1)
RoutingRule(48-59)ui/lib/api.ts (1)
apiService(257-257)ui/components/ui/card.tsx (3)
CardHeader(50-50)CardTitle(50-50)CardDescription(50-50)ui/components/ui/button.tsx (1)
Button(71-71)ui/components/ui/badge.tsx (1)
Badge(37-37)ui/components/ui/table.tsx (6)
Table(64-64)TableHeader(64-64)TableRow(64-64)TableHead(64-64)TableBody(64-64)TableCell(64-64)ui/components/ui/switch.tsx (1)
Switch(29-29)
ui/components/ui/breadcrumb.tsx (1)
ui/lib/utils.ts (1)
cn(4-6)
ui/lib/api.ts (1)
ui/lib/types/routing.ts (4)
ListRoutingRulesResponse(75-78)RoutingRule(48-59)CreateRoutingRuleRequest(61-69)UpdateRoutingRuleRequest(71-73)
🪛 checkmake (0.2.2)
Makefile
[warning] 46-46: Target body for "dev" exceeds allowed length of 5 (14).
(maxbodylength)
🪛 Biome (1.9.4)
ui/components/ui/breadcrumb.tsx
[error] 54-61: The HTML element with the interactive role "link" is not focusable.
A non-interactive HTML element that is not focusable may not be reachable for users that rely on keyboard navigation, even with an added role like "link".
Add a tabIndex attribute to make this element focusable.
(lint/a11y/useFocusableInteractive)
🔇 Additional comments (76)
transports/bifrost-http/main.go (2)
55-55: LGTM - Clean branding additionThe ASCII banner and fmt import addition provides clear identification for the Bifrost HTTP transport during startup. This enhances user experience without affecting functionality.
Also applies to: 237-250
272-272: Good formatting improvementThe added blank line improves code readability after the error handling block.
ui/components/logs/logs-table.tsx (2)
4-9: Good import organizationThe import statements are now better grouped by source, improving code readability and maintainability.
89-89: Consistent styling updateThe border-radius change from
rounded-mdtorounded-smmaintains visual consistency with the UI-wide styling updates in this PR.ui/app/plugins/page.tsx (1)
212-212: Consistent UI styling updateAll the
rounded-mdtorounded-smchanges maintain visual consistency across the plugin page elements. This aligns with the UI-wide styling standardization in this PR.Also applies to: 222-222, 232-232, 281-281, 299-299, 317-317, 341-341
ui/components/ui/textarea.tsx (1)
10-10: Consistent styling updateThe border-radius change from
rounded-mdtorounded-smmaintains visual consistency with the UI-wide styling standardization in this PR.ui/components/ui/input.tsx (1)
14-14: LGTM - Consistent UI styling updateThe border-radius change from
rounded-mdtorounded-smaligns with the broader UI consistency improvements across the codebase.ui/components/config/providers-list.tsx (1)
86-86: LGTM - Consistent table styling updateThe border-radius reduction aligns with the UI consistency improvements being applied across components.
ui/components/ui/skeleton.tsx (1)
4-4: LGTM - Skeleton styling consistencyThe border-radius update maintains visual consistency with other UI components.
ui/components/ui/tag-input.tsx (2)
4-7: LGTM - Clean import organizationThe import reordering improves readability by grouping UI components and utilities logically.
41-41: LGTM - Consistent border-radius stylingThe border-radius update maintains visual consistency with the broader UI styling improvements.
ui/components/ui/badge.tsx (2)
3-3: Import reordering looks good.The React import reordering is a minor cosmetic change that doesn't affect functionality.
8-8: Border radius consistency update approved.The change from
rounded-mdtorounded-smaligns with the broader UI consistency update across components mentioned in the PR summary.transports/bifrost-http/.air.toml (2)
10-10: Removing "ui" from excluded directories is appropriate.This change allows the build system to monitor UI changes, which makes sense given the new routing rules UI components being introduced.
70-71: Adding UI watch directory improves development workflow.The new watch directory for "../../ui" enables hot reload for UI changes during development of the bifrost-http transport, supporting the integrated development of the routing rules feature.
ui/components/ui/button.tsx (2)
3-3: Import reordering approved.The React import reordering maintains consistency without affecting functionality.
9-9: Border radius consistency update looks good.The changes from
rounded-mdtorounded-smin the base class and size variants create a uniform appearance across UI components, aligning with the broader styling update in this PR.Also applies to: 24-25
ui/components/logs/empty-state.tsx (1)
220-221: Tab styling consistency update approved.The border radius changes from
rounded-lg/rounded-t-lgtorounded-sm/rounded-t-smmaintain visual consistency with the broader UI styling update across components.ui/components/ui/dialog.tsx (2)
52-52: Dialog border radius consistency update approved.The change from
rounded-lgtorounded-smmaintains visual consistency with other UI components in this styling update.
91-100: Export formatting improvement looks good.The indented vertical alignment of exports improves code readability without affecting functionality.
ui/components/config/core-settings-list.tsx (2)
6-11: Import organization looks good.The reordering of imports follows standard conventions with external libraries first, followed by internal utilities and components.
149-149: Consistent UI styling update.The change from
rounded-lgtorounded-smacross all form container elements creates a more unified visual appearance and aligns with the broader UI standardization effort throughout the application.Also applies to: 173-173, 190-190, 206-206
ui/components/ui/alert-dialog.tsx (3)
4-4: Import reordering improves organization.Moving React import after external library imports follows standard conventions.
Also applies to: 7-7
41-41: Consistent border-radius styling.The change from
rounded-lgtorounded-smaligns with the UI-wide styling standardization effort.
83-84: Export formatting is cleaner.The single-line export format is more compact while maintaining readability.
ui/components/ui/alert.tsx (3)
2-2: Import organization follows conventions.Moving React import after external dependency imports is the standard practice.
7-7: Border-radius standardization improves UI consistency.The change from
rounded-lgtorounded-smaligns with the broader UI styling updates across components.
41-41: Export reordering is acceptable.The order change doesn't impact functionality and maintains clean organization.
ui/components/ui/tabs.tsx (3)
4-4: Import organization is consistent.The reordering matches the pattern used across other UI components.
16-16: Consistent border-radius styling across tab components.The standardization from
rounded-lg/rounded-mdtorounded-smcreates a unified visual appearance across TabsList and TabsTrigger components.Also applies to: 27-27
39-39: Export reordering maintains functionality.The new order doesn't affect component behavior and follows a logical grouping pattern.
ui/components/ui/hover-card.tsx (3)
4-4: Import reordering follows established pattern.The change aligns with the consistent import organization applied across UI components.
29-29: Border-radius standardization enhances visual consistency.The change from
rounded-mdtorounded-smmaintains the unified styling approach across the UI library.
38-38: Export order adjustment is fine.The reordering doesn't impact functionality and follows a logical component hierarchy.
ui/components/ui/select.tsx (3)
5-5: LGTM: Improved import organization.Moving the React import after external library imports follows common conventions and improves consistency.
34-34: LGTM: Consistent border radius styling.The change from
rounded-mdtorounded-smaligns with the broader UI styling standardization across components.Also applies to: 53-53
137-148: LGTM: Improved export formatting.The consistent indentation, trailing commas, and proper spacing enhance code readability and maintainability.
ui/app/docs/page.tsx (2)
2-2: LGTM: Improved import organization.The import reordering (Button before Card imports, alphabetized icon imports) enhances code organization and consistency.
Also applies to: 5-5
94-94: LGTM: Consistent border radius styling.The change from
rounded-lgtorounded-smon the icon container aligns with the UI-wide styling standardization.ui/components/ui/accordion.tsx (3)
5-5: LGTM: Consistent import organization.Moving the React import after external library imports maintains consistency with other UI components.
23-23: LGTM: Consistent border radius styling.The change from
rounded-mdtorounded-smin the AccordionTrigger aligns with the UI standardization effort.
47-48: LGTM: Export statement formatting.The export reordering and additional spacing improve code organization without affecting functionality.
ui/components/ui/tooltip.tsx (3)
4-4: LGTM: Consistent import organization.Moving the React import after the external library import maintains consistency across UI components.
31-31: LGTM: Consistent border radius styling.The change from
rounded-mdtorounded-smin the TooltipContent maintains visual consistency with other UI components.
43-44: LGTM: Export statement formatting.The export reordering and spacing improvements enhance code organization.
ui/components/ui/popover.tsx (3)
4-4: LGTM: Consistent import organization.Moving the React import after the external library import maintains consistency with the established pattern.
33-33: LGTM: Consistent border radius styling.The change from
rounded-mdtorounded-smin the PopoverContent aligns with the UI standardization across components.
48-49: LGTM: Improved export organization.The export reordering (PopoverAnchor after Popover) creates a more logical grouping while maintaining functionality.
ui/components/config/mcp-clients-lists.tsx (2)
15-24: LGTM - Import formatting improvementThe import statement formatting provides better readability with consistent indentation.
115-115: LGTM - UI consistency updateThe border-radius change from
rounded-mdtorounded-smaligns with the broader UI standardization effort across the codebase.ui/components/ui/dropdown-menu.tsx (3)
5-5: LGTM - Improved import orderingMoving React import after external libraries follows conventional import ordering practices.
28-28: LGTM - Consistent border-radius stylingThe border-radius changes from
rounded-mdtorounded-smmaintain visual consistency across dropdown components.Also applies to: 184-184
193-199: LGTM - Cleaner export formattingThe more compact export statement improves readability while maintaining all exported components.
ui/components/sidebar.tsx (4)
3-3: LGTM - Icon import for routing featureAdding ShuffleIcon import supports the new routing rules navigation item.
61-66: LGTM - Routing rules navigation integrationThe new navigation item properly integrates the routing feature with clear title, appropriate icon, and descriptive text. The
/routingURL should connect to the new routing pages mentioned in the PR objectives.
159-159: LGTM - UI consistency improvementsBorder-radius changes from
rounded-lgtorounded-smalign with the broader UI standardization effort across the codebase.Also applies to: 202-202
174-174: Verify Intentional Removal of WebSocket Connection IndicatorIt looks like the
useWebSockethook is still imported on line 136, but itsisConnectedvalue is never rendered—and no other component in the UI displays connection status. Please confirm that removing the visible connection indicator is deliberate and that there isn’t a replacement elsewhere.• In
ui/components/sidebar.tsx, the hook is called here but unused in the JSX:const { isConnected: isWebSocketConnected } = useWebSocket();• No other TS/TSX files render
isConnectedor a WebSocket status badge.
• Ensure users have another way to monitor live connection health if needed.ui/components/logs/log-detail-sheet.tsx (2)
4-4: LGTM - Improved import organizationGrouping related imports (UI components and constants) together improves code organization and readability.
Also applies to: 6-6, 11-11
103-103: LGTM - Consistent request type stylingThe border-radius change maintains visual consistency with other UI components while preserving the request type color coding functionality.
ui/components/ui/sidebar.tsx (3)
6-6: LGTM - Improved import organizationThe import reordering and grouping (external libraries, UI components, utilities) follows best practices and improves code organization.
Also applies to: 12-17, 20-24, 25-26
247-247: LGTM - Comprehensive UI consistency updateThe systematic change from various rounded classes to
rounded-smcreates consistent visual styling across all sidebar elements (floating variants, labels, actions, buttons, badges, skeletons, and submenus).Also applies to: 408-408, 429-429, 477-477, 564-564, 589-589, 618-618, 623-623, 689-689
702-726: LGTM - Cleaner export formattingThe more compact export statement improves readability while maintaining all exported sidebar components.
Makefile (2)
46-61: LGTM! Good separation of development targets.The restructuring of
dev(complete environment) anddev-http(API-only) targets provides better developer workflow control. The logic is sound and both targets maintain consistent parameter handling.
62-68: LGTM! Clean API-only development target.The new
dev-httptarget is well-designed for API-only development scenarios. The implementation correctly reuses the same air configuration and parameters as the full dev environment.ui/app/routing/new/page.tsx (1)
58-86: LGTM! Clean breadcrumb navigation and component integration.The breadcrumb implementation is well-structured and the integration with the
RuleEditorcomponent through callback props follows React best practices. The navigation flow back to/routingis consistent.ui/components/ui/command.tsx (3)
14-14: LGTM! Consistent border radius styling update.The change from
rounded-mdtorounded-smaligns with the broader UI consistency improvements across the application. This maintains visual harmony with other components.
55-55: LGTM! Input styling consistency maintained.The border radius update on the CommandInput maintains consistency with the parent Command component and follows the design system updates.
5-8: LGTM! Clean import and export organization.The reordering of imports and exports improves code organization without affecting functionality.
Also applies to: 114-114
ui/components/config/provider-form.tsx (3)
14-27: LGTM! Improved import statement formatting.The consistent indentation and organization of import statements enhances code readability without affecting functionality.
464-464: LGTM! Consistent border radius updates.The changes from
rounded-lg/rounded-mdtorounded-smmaintain visual consistency with the broader UI design system updates across the application.Also applies to: 541-541
29-29: LGTM! Clean import organization.The reordering of imports improves code organization and follows consistent patterns.
Also applies to: 36-36
ui/components/routing-rules/RoutingList.tsx (3)
121-146: LGTM! Well-implemented summary functions.The
getConditionSummaryandgetActionSummaryfunctions provide clear, user-friendly descriptions of complex rule structures. The logic handles edge cases appropriately (plural forms, empty arrays) and the action type mapping is comprehensive.
32-54: LGTM! Robust error handling in data fetching.The
loadRulesfunction implements proper error handling with both API-level errors (from the tuple response) and exception-level errors (try-catch). The loading state management and user feedback through toasts are well-implemented.
148-168: LGTM! Good loading state UX.The loading state provides visual feedback with animated placeholders and appropriately disables interactive elements. The consistent layout structure prevents layout shifts when data loads.
ui/lib/api.ts (2)
1-13: LGTM!The import statements are properly organized and the new routing types are correctly imported.
144-189: Well-implemented routing rule API methods!The new methods follow the established patterns in the codebase with proper error handling, consistent return types, and appropriate REST endpoints.
| import type { BifrostMessage, ContentBlock, LogEntry, LogFilters, LogStats, MessageContent, Pagination } from '@/lib/types/logs' | ||
| import { AlertCircle, BarChart, CheckCircle, Clock, Hash } from 'lucide-react' | ||
| import { useCallback, useEffect, useMemo, useState, useRef } from 'react' | ||
| import { useCallback, useEffect, useMemo, useRef, useState } from 'react' |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Import order looks fine – just confirm it aligns with your lint rules
The new sequence (useCallback, useEffect, useMemo, useRef, useState) is alphabetically sorted, which many ESLint configs enforce. If your project’s import-sorting rule differs (e.g., react-hooks/exhaustive-deps preset that prefers useState before useRef), this might trigger a lint error; otherwise, no functional impact.
🤖 Prompt for AI Agents
In ui/app/page.tsx at line 14, verify that the import order of React hooks
matches your project's linting rules. If your ESLint or other lint configuration
requires a specific order (such as placing useState before useRef), reorder the
imports accordingly to comply with those rules; otherwise, no change is needed.
akshaydeo
force-pushed
the
07-14-adds_route_rules
branch
from
f0a600a to
458b077
Compare
2 participants
TL;DR
Added routing rules functionality to the UI, allowing users to create, view, edit, and delete rules that direct requests to specific providers based on conditions.
What changed?
/routingand/routing/new) for managing routing rulesRoutingListcomponent to display and manage existing rules in a table formatRuleEditorwith a visual flow editor using ReactFlow for creating/editing rulesHow to test?
Why make this change?
This feature enhances Bifrost's capabilities by allowing users to create sophisticated routing logic that can direct API requests to different providers based on headers, paths, and other criteria. This provides more flexibility and control over how requests are handled, enabling use cases like: