[OPIK-5312] [FE] feat: add loading state to assistant sidebar#5961
[OPIK-5312] [FE] feat: add loading state to assistant sidebar#5961
Conversation
Show a shimmer + spinner while the backend pod starts up (5-30s) instead of rendering nothing. The loader reads the sidebar's stored width from localStorage so there's no layout jump when the iframe takes over.
📋 PR Linter Failed❌ Invalid Title Format. Your PR title must include a ticket/issue number and may optionally include component tags (
Example: ❌ Incomplete Issues Section. You must reference at least one GitHub issue ( |
awkoy
changed the title
![baz-reviewer[bot]](https://avatars.githubusercontent.com/in/933528?s=60&v=4){kind=small}
| if (computeResult && !computeResult.enabled) | ||
| return notReadyResult(null, "disabled"); |
There was a problem hiding this comment.
if (computeResult && !computeResult.enabled) return notReadyResult(null, "disabled"); leaves error as null so toasts are empty — should we return a fallback like Assistant backend is disabled; enable it in settings. instead of null?
notReadyResult("Assistant backend is disabled; enable it in settings.", "disabled")
Finding type: Provide comprehensive error messages | Severity: 🟢 Low
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents:
Before applying, verify this suggestion against the current code. In
apps/opik-frontend/src/plugins/comet/useAssistantBackend.ts around lines 153 to 154, the
branch handling a disabled compute returns notReadyResult(null, "disabled") which yields
an empty error string for the UI. Change that call to return notReadyResult("Assistant
backend is disabled; enable it in settings.", "disabled") so the returned
UseAssistantBackendResult.error contains a descriptive, actionable message for the user.
There was a problem hiding this comment.
Not needed — when phase === "disabled" the component returns null and the error UI is never rendered, so this message would never be seen.
There was a problem hiding this comment.
Got it, that makes sense given the phase === "disabled" behavior—I'll remember this and avoid similar unnecessary error message suggestions after the PR is merged.
| if (computeError) | ||
| return notReadyResult((computeError as Error).message, "error"); |
There was a problem hiding this comment.
if (computeError) return notReadyResult((computeError as Error).message, "error"); forwards computeError.message verbatim — should we fallback to computeError.message ?? "Assistant provisioning failed; please try again." when calling notReadyResult?
if (computeError) return notReadyResult((computeError as Error).message ?? "Assistant provisioning failed; please try again.", "error");
Finding type: Provide comprehensive error messages | Severity: 🟢 Low
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents:
Before applying, verify this suggestion against the current code. In
apps/opik-frontend/src/plugins/comet/useAssistantBackend.ts around lines 155 to 156, the
return currently forwards computeError.message directly which can be undefined for
non-Error shapes. Replace that return with a safe-derived message: compute a message
variable using (computeError as Error)?.message ?? (typeof computeError === 'string' ?
computeError : String(computeError)) ?? 'Assistant provisioning failed; please try
again.' and then call return notReadyResult(message, 'error'); This ensures a non-empty,
user-facing fallback message for all error shapes.
There was a problem hiding this comment.
React Query always wraps errors in an Error object, so .message is always a string here. No fallback needed.
There was a problem hiding this comment.
Got it, since React Query guarantees an Error object, no fallback is needed here—I'll remember this for similar error-handling suggestions after the PR is merged.
| function getStoredSidebarWidth(): number { | ||
| try { | ||
| const parsed = parseInt( | ||
| localStorage.getItem("assistant-sidebar-width") ?? "", | ||
| 10, | ||
| ); | ||
| if (parsed > 0) return parsed; | ||
| } catch { | ||
| /* localStorage unavailable */ | ||
| } | ||
| return LOADER_DEFAULT_WIDTH; | ||
| } |
There was a problem hiding this comment.
getStoredSidebarWidth and getStoredSidebarOpen hardcode the localStorage keys, should we hoist them into constants like const SIDEBAR_WIDTH_KEY = "assistant-sidebar-width" and const SIDEBAR_OPEN_KEY = "assistant-sidebar-open"?
Finding type: Avoid hardcoded configuration values | Severity: 🟢 Low
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents:
Before applying, verify this suggestion against the current code. In
apps/opik-frontend/src/plugins/comet/AssistantSidebar.tsx around lines 26 to 45, the
functions getStoredSidebarWidth and getStoredSidebarOpen use hardcoded localStorage keys
"assistant-sidebar-width" and "assistant-sidebar-open". Hoist these strings into
well-named constants (e.g. const SIDEBAR_WIDTH_KEY = "assistant-sidebar-width" and const
SIDEBAR_OPEN_KEY = "assistant-sidebar-open") declared near the other top-level
constants, then replace the literal occurrences inside getStoredSidebarWidth and
getStoredSidebarOpen with those constants. Also scan the file and replace any other uses
of those same string literals with the new constants to centralize configuration and
avoid duplication.
There was a problem hiding this comment.
Each key is used exactly once in its own dedicated helper function. Extracting constants for single-use strings would be over-abstraction.
There was a problem hiding this comment.
Got it, that makes sense for single-use keys scoped to dedicated helpers—I'll keep this nuance in mind and save it to memory after the PR is merged.
2 participants
Details
Show a shimmer + spinner loader while the assistant backend pod starts up (5-30 seconds) instead of a blank empty space. Adds
phasefield touseAssistantBackendreturn type for contextual loading messages. Loader reads stored sidebar width from localStorage to match the iframe's eventual width, preventing layout jumps.Change checklist
Issues
AI-WATERMARK
AI-WATERMARK: yes
Testing
Documentation
Nothing