packages/console-rust/src/main.rs (1)

33-36: LGTM! Consider logging the bridge port for operational visibility.

The new bridge_port CLI option correctly follows the existing pattern for port arguments and enables configurable bridge connectivity.

However, the startup log at lines 78-81 only displays engine_port and ws_port. Consider adding bridge_port to the log output for better operational visibility:

💡 Suggestion to improve operational logging

     info!(
-        "Connecting to engine at {}:{} (WS: {})",
-        args.engine_host, args.engine_port, args.ws_port
+        "Connecting to engine at {}:{} (WS: {}, Bridge: {})",
+        args.engine_host, args.engine_port, args.ws_port, args.bridge_port
     );

packages/console-frontend/src/hooks/useTraceFilters.ts (1)

215-217: Consider deriving warnings directly instead of syncing via effect.

The useEffect that syncs computedWarnings to validationWarnings state introduces an extra render cycle. Since computedWarnings is already memoized, you could either:

1. Return computedWarnings directly from the hook (renaming as needed)
2. Keep validationWarnings state only if external clearing via clearValidationWarnings is required

Current implementation is functionally correct but adds a minor render overhead.

♻️ Optional: Return computed warnings directly

-  const [validationWarnings, setValidationWarnings] = useState<{
-    durationSwapped?: boolean
-    timeRangeSwapped?: boolean
-  }>({})
+  const [manuallyCleared, setManuallyCleared] = useState(false)

...

-  useEffect(() => {
-    setValidationWarnings(computedWarnings)
-  }, [computedWarnings])

+  // Reset cleared state when filters change
+  useEffect(() => {
+    setManuallyCleared(false)
+  }, [filters])

+  const validationWarnings = manuallyCleared ? {} : computedWarnings

   const clearValidationWarnings = useCallback(() => {
-    setValidationWarnings({})
+    setManuallyCleared(true)
   }, [])

packages/console-frontend/src/routes/streams.tsx (1)

177-181: Update stale comment to reflect composite key structure.

The comment on line 179 states Map<streamName, {groupId, subscriptionId}>, but the actual key is now a composite ${streamName}:${groupId}. This discrepancy could confuse future maintainers.

📝 Suggested comment fix

   const subscriptionsRef = useRef<Map<string, { groupId: string; subscriptionId: string }>>(
     new Map(),
-  ) // Map<streamName, {groupId, subscriptionId}>
+  ) // Map<`${streamName}:${groupId}`, {groupId, subscriptionId}>

packages/console-frontend/src/routes/handlers.tsx (1)

627-636: Consider caching getConfig() result to avoid redundant calls.

Lines 627-628 call getConfig() twice within the same JSX expression. While this works correctly, destructuring once improves readability and slight efficiency.

💡 Optional: Destructure config once in the render scope

Since this is inside an IIFE that already has access to trigger and other computed values, you could destructure once at the start of that block:

 {(() => {
   const trigger = getAssociatedTrigger(selectedFunction)
   const apiPath = getApiPath(selectedFunction)
+  const { engineHost, enginePort } = getConfig()
+  const baseUrl = `${window.location.protocol}//${engineHost}:${enginePort}`

   return (
     <>
       {/* ... */}
       <code className="flex-1 text-xs font-mono bg-black/40 text-cyan-400 px-3 py-2 rounded border border-cyan-500/20">
-        {getHttpMethod(selectedFunction)} {window.location.protocol}{'//'}
-        {getConfig().engineHost}:{getConfig().enginePort}/{apiPath}
+        {getHttpMethod(selectedFunction)} {baseUrl}/{apiPath}
       </code>
       <button
         onClick={() => {
-          const { engineHost, enginePort } = getConfig()
           copyToClipboard(
-            `${window.location.protocol}//${engineHost}:${enginePort}/${apiPath}`,
+            `${baseUrl}/${apiPath}`,
             'endpoint',
           )
         }}

packages/console-frontend/src/components/traces/TraceFilters.tsx (3)

63-76: Consider stabilizing the useClickOutside callback with useCallback.

The onClickOutside callback passed to useClickOutside is recreated on every render (e.g., () => openPopover === 'status' && setOpenPopover(null)). This causes the effect to re-run on each render since it's in the dependency array.

💡 Stabilize with useCallback or move condition inside the hook

Option 1: Move the condition inside the hook so consumers pass a stable setter:

-function useClickOutside(ref: React.RefObject<HTMLElement | null>, onClickOutside: () => void) {
+function useClickOutside(
+  ref: React.RefObject<HTMLElement | null>,
+  onClickOutside: () => void,
+  enabled: boolean = true
+) {
   useEffect(() => {
     function handleClickOutside(event: MouseEvent) {
-      if (ref.current && !ref.current.contains(event.target as Node)) {
+      if (enabled && ref.current && !ref.current.contains(event.target as Node)) {
         onClickOutside()
       }
     }
     // ...
-  }, [ref, onClickOutside])
+  }, [ref, onClickOutside, enabled])
 }

// Usage:
-useClickOutside(statusRef, () => openPopover === 'status' && setOpenPopover(null))
+useClickOutside(statusRef, () => setOpenPopover(null), openPopover === 'status')

Option 2: Wrap callbacks in useCallback where they're defined.

---

78-83: formatDuration is duplicated in traces.tsx.

This same function exists in packages/console-frontend/src/routes/traces.tsx (lines 73-78). Consider extracting to a shared utility module (e.g., @/lib/formatters or @/lib/traceTransform).

---

227-255: Consider memoizing activeFilters array.

The activeFilters array is reconstructed on every render. While the array is small, wrapping it in useMemo with appropriate dependencies would prevent unnecessary recalculations during frequent re-renders.

packages/console-frontend/src/routes/traces.tsx (1)

110-111: Consider reducing the default limit or implementing lazy loading.

A default limit of 10,000 traces could cause performance issues with memory usage and rendering time on the client. Consider:

• A lower default (e.g., 1000) with load-more functionality
• Server-side pagination
• Virtual scrolling for large lists

packages/console-frontend/src/api/config.ts (1)

27-37: getDevtoolsApi and getManagementApi return identical URLs despite different usage contexts.

Both functions return ${protocol}//${c.engineHost}:${c.enginePort}/_console. While they're used across different modules (getDevtoolsApi more extensively, getManagementApi for specific operations), they produce the same result. If this semantic distinction is intentional, add a comment explaining the purpose. Otherwise, one could delegate to the other to avoid duplication.

packages/console-frontend/src/api/utils.ts (1)

29-30: Consider including full URL in error message for easier debugging.

Since the base URL is now dynamic, logging only the path may make debugging harder. Including the full URL would provide better context.

🔧 Proposed improvement

+  const url = `${getDevtoolsApi()}${devtoolsPath}`
+  const res = await fetch(url, options)
-  const res = await fetch(`${getDevtoolsApi()}${devtoolsPath}`, options)
   if (!res.ok) {
-    throw new Error(`Failed to fetch from ${devtoolsPath}: ${res.status}`)
+    throw new Error(`Failed to fetch from ${url}: ${res.status}`)
   }

packages/console-frontend/src/components/traces/TraceFilters.tsx (2)

63-76: useClickOutside re-registers listeners on every render due to inline callbacks.

The hook includes onClickOutside in its dependency array. Since the callbacks passed on lines 121-126 are inline arrow functions, they're recreated on each render, causing the effect to repeatedly add/remove event listeners.

Consider either memoizing the callbacks or restructuring the hook to accept the popover type and close action separately:

♻️ Option 1: Restructure hook to avoid inline callbacks

-function useClickOutside(ref: React.RefObject<HTMLElement | null>, onClickOutside: () => void) {
+function useClickOutside(
+  ref: React.RefObject<HTMLElement | null>,
+  isActive: boolean,
+  onClickOutside: () => void
+) {
   useEffect(() => {
+    if (!isActive) return
+
     function handleClickOutside(event: MouseEvent) {
       if (ref.current && !ref.current.contains(event.target as Node)) {
         onClickOutside()
       }
     }

     document.addEventListener('mousedown', handleClickOutside)
     return () => {
       document.removeEventListener('mousedown', handleClickOutside)
     }
-  }, [ref, onClickOutside])
+  }, [ref, isActive, onClickOutside])
 }

Then use a stable callback:

const closePopover = useCallback(() => setOpenPopover(null), [])

useClickOutside(statusRef, openPopover === 'status', closePopover)
useClickOutside(timeRangeRef, openPopover === 'timeRange', closePopover)
// ...

Also applies to: 121-126

---

338-353: Popovers lack keyboard accessibility (Escape to close).

Users cannot close open popovers with the Escape key, which is a common accessibility expectation. Consider adding a keydown handler at the popover container level.

♿ Add Escape key handling

+  useEffect(() => {
+    const handleEscape = (e: KeyboardEvent) => {
+      if (e.key === 'Escape' && openPopover) {
+        setOpenPopover(null)
+      }
+    }
+    document.addEventListener('keydown', handleEscape)
+    return () => document.removeEventListener('keydown', handleEscape)
+  }, [openPopover])

Add this effect near the other useEffect hooks (around line 111).