Add onboarding setup checklist widget - #3606
Merged
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (6)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds an onboarding checklist widget with OSS API fallbacks, a lazily-loaded confetti helper, client metadata mutation for skip/dismiss persistence (with optimistic updates), layout wiring to render the widget when the DB is connected, and a small Beta badge component. ChangesOnboarding Widget Feature
Sequence DiagramsequenceDiagram
participant App as clientLayout
participant Widget as OnboardingWidget
participant ConfigAPI as getCoreConfig
participant Metadata as updateClientMetadata
participant SCIMQuery as useGetSCIMProvidersQuery
participant Toast as Toast Notifications
App->>Widget: render when is_db_connected
Widget->>SCIMQuery: fetch providers (OSS stub or enterprise)
Widget->>ConfigAPI: fetch core config & related queries
Widget->>Widget: compute steps and previous completion snapshot
Note over Widget: User interacts with checklist
Widget->>Widget: user selects step (set active + navigate)
Widget->>Widget: detect completion edge → fireConfettiFrom(ref)
Widget->>Metadata: POST skip/dismiss patch to /config/metadata
Metadata-->>Widget: success / error
Widget->>Toast: show error on mutation failure
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
|
akshaydeo
marked this pull request as ready for review
May 19, 2026 21:32
Contributor
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
18 tasks
Contributor
Confidence Score: 5/5Safe to merge; the changes are UI-only, no data loss or security boundary is affected, and the core logic for dismissal, confetti, and skip serialisation is sound. All changed code is UI-only and the widget has no server-side write path beyond the metadata blob. The one inconsistency found (auto-restore not checking pending skips) is a minor UX glitch with no data-correctness or security implications. ui/components/onboardingWidget.tsx — the auto-restore effect's pending-skip condition. Important Files Changed
Reviews (9): Last reviewed commit: "onbaording widget ui" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
ui/components/onboardingWidget.tsx (1)
19-19: ⚡ Quick winLazy-load
canvas-confettito avoid paying startup cost for a rare interaction.At Line 19, importing confetti eagerly loads it on every app load even though it is only used on completion transitions.
As per coding guidelines: “Lazy load heavy or rarely-used libraries in React to minimize bundle size”.Proposed change
-import confetti from "canvas-confetti"; +let confettiFn: typeof import("canvas-confetti")["default"] | null = null;-function fireConfettiFrom(el: HTMLElement) { +async function fireConfettiFrom(el: HTMLElement) { const rect = el.getBoundingClientRect(); const originX = (rect.left + rect.width / 2) / window.innerWidth; const originY = (rect.top + rect.height / 2) / window.innerHeight; - confetti({ + if (!confettiFn) { + confettiFn = (await import("canvas-confetti")).default; + } + confettiFn({ particleCount: 36, spread: 60, startVelocity: 28, ticks: 80, origin: { x: originX, y: originY }, colors: ["`#a855f7`", "`#ec4899`", "`#22c55e`", "`#f59e0b`", "`#3b82f6`"], disableForReducedMotion: true, }); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/components/onboardingWidget.tsx` at line 19, Replace the top-level eager import of "confetti" with a dynamic import at the point of use: remove the line importing confetti and instead load it inside the completion transition handler (where "confetti" is invoked) using await import('canvas-confetti') or import('canvas-confetti').then(...), then call the resolved confetti function; ensure the handler (the function that triggers the completion transition / confetti call) handles the promise (async/await or .then) so the library is only fetched when the completion animation runs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ui/components/onboardingWidget.tsx`:
- Around line 308-415: Add stable data-testid attributes to the new interactive
buttons so E2E tests can target them: add data-testid="onboarding-close" to the
close button that calls setClosedForSession, add
data-testid={`onboarding-step-${step.id}`} to the step action button that calls
handleStepClick, add data-testid={`onboarding-skip-${step.id}`} to the per-step
skip button that calls handleSkip, and add data-testid="onboarding-later" and
data-testid="onboarding-skip-all" to the footer buttons that call
handleHideForMe and handleHideForAll respectively (use step.id for per-step
identifiers to keep them unique).
- Around line 237-242: The handleSkip function risks clobbering concurrent skips
because it writes [...skippedIds, stepId] from a potentially stale local
snapshot; change it to perform a merge-on-client or read-modify-write safely:
before calling updateMetadata (the call that writes METADATA_SKIPPED_KEY), fetch
the latest skipped array from the authoritative source (or use an updateMetadata
API that supports a server-side append/merge) and then send the union
(deduplicated) with the new stepId; if the API supports conditional updates or
retries on conflict, use that to retry when the server value changed between
read and write. Ensure you reference handleSkip, METADATA_SKIPPED_KEY and
updateMetadata when applying this change and keep idempotency (no duplicates) in
the merged array.
In `@ui/lib/store/apis/configApi.ts`:
- Around line 100-107: The updateClientMetadata mutation currently only
invalidates the "Config" tag causing a stale getCoreConfig response; change
updateClientMetadata (builder.mutation) to implement an onQueryStarted handler
that performs an optimistic cache update by calling
dispatch(api.util.updateQueryData) for getCoreConfig to patch the metadata field
immediately, store the patchResult (so you can call patchResult.undo() on
errors), await queryFulfilled and if it throws revert using patchResult.undo();
keep invalidatesTags if desired but ensure the optimistic update lives in
updateClientMetadata's onQueryStarted and references the getCoreConfig selector
in the same api slice.
In `@ui/package.json`:
- Around line 47-50: The dependency "`@types/canvas-confetti`" is currently listed
with runtime dependencies but should be a dev-time only type definition; move
the "`@types/canvas-confetti`": "1.9.0" entry from the top-level "dependencies"
section into the "devDependencies" section of package.json (preserve the
version), then update the lockfile / run your package manager install to reflect
the change.
---
Nitpick comments:
In `@ui/components/onboardingWidget.tsx`:
- Line 19: Replace the top-level eager import of "confetti" with a dynamic
import at the point of use: remove the line importing confetti and instead load
it inside the completion transition handler (where "confetti" is invoked) using
await import('canvas-confetti') or import('canvas-confetti').then(...), then
call the resolved confetti function; ensure the handler (the function that
triggers the completion transition / confetti call) handles the promise
(async/await or .then) so the library is only fetched when the completion
animation runs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c69828c6-8036-44c0-83db-adb606b9d557
⛔ Files ignored due to path filters (1)
ui/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
ui/app/_fallbacks/enterprise/lib/store/apis/scimApi.tsui/app/clientLayout.tsxui/components/onboardingWidget.tsxui/lib/store/apis/configApi.tsui/package.json
akshaydeo
force-pushed
the
05-20-onbaording_widget_ui
branch
from
May 20, 2026 04:37
8935ed8 to
e38b6c9
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
ui/lib/store/apis/configApi.ts (1)
131-131:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDrop the tag invalidation here.
After the optimistic patch,
invalidatesTags: ["Config"]can immediately refetchgetCoreConfigand overwrite the local metadata with a stale replica response. In this flow that means skipped/dismissed onboarding state can briefly revert right after the user changes it.♻️ Proposed fix
- invalidatesTags: ["Config"],Verify by comparing this mutation with the established API-layer pattern; the expected result is that optimistic-cache mutations in
ui/lib/store/apis/avoid re-invalidating the same data they just patched.#!/bin/bash set -euo pipefail echo "== updateClientMetadata ==" sed -n '98,132p' ui/lib/store/apis/configApi.ts echo echo "== Nearby optimistic-update patterns in ui/lib/store/apis ==" rg -n -C3 'onQueryStarted|updateQueryData|invalidatesTags' ui/lib/store/apis --type tsBased on learnings: "In ui/lib/store/apis/, optimistically patch the cache with onQueryStarted + routingRulesApi.util.updateQueryData ... instead of using invalidatesTags. This avoids cross-server stale UI in a clustered environment."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/lib/store/apis/configApi.ts` at line 131, Remove the invalidatesTags: ["Config"] from the mutation in configApi (the mutation that patches client metadata, e.g. updateClientMetadata) and instead implement an optimistic cache patch in that mutation's onQueryStarted: use api.util.updateQueryData against the getCoreConfig query to apply the local metadata change and roll it back on error, following the established pattern (onQueryStarted + updateQueryData) used elsewhere in ui/lib/store/apis so the optimistic update is not immediately re-fetched by a tag invalidation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ui/components/onboardingWidget.tsx`:
- Around line 339-345: The "Skip" button is only visible on hover which hides it
from keyboard users; update the button's className for the element using
handleSkip and writingMetadata so it becomes visible when focused (e.g., add
focus:opacity-100 and focus-visible:opacity-100 or equivalent focus utility
classes), ensuring keyboard focus shows the control while preserving the
existing hover and disabled styles.
In `@ui/lib/store/apis/configApi.ts`:
- Around line 99-123: The optimistic updater in updateClientMetadata
(onQueryStarted) currently spreads patch into draft.metadata which preserves
keys with null values; update both configApi.util.updateQueryData calls (the
ones for "getCoreConfig" with {} and with { fromDB: true }) to iterate over
Object.entries(patch) and for each [k,v] do: ensure draft.metadata exists, if v
=== null then delete draft.metadata[k] else set draft.metadata[k] = v, so null
is honored as a deletion rather than being merged into the cache.
In `@ui/package.json`:
- Line 49: Update the pinned canvas-confetti dependency in package.json from
"1.9.3" to "1.9.4": open package.json, locate the "canvas-confetti": "1.9.3"
entry and change its version string to "1.9.4", then run your package manager
(npm/yarn/pnpm) to install and update lockfile; no changes to
`@types/canvas-confetti` are needed.
---
Duplicate comments:
In `@ui/lib/store/apis/configApi.ts`:
- Line 131: Remove the invalidatesTags: ["Config"] from the mutation in
configApi (the mutation that patches client metadata, e.g. updateClientMetadata)
and instead implement an optimistic cache patch in that mutation's
onQueryStarted: use api.util.updateQueryData against the getCoreConfig query to
apply the local metadata change and roll it back on error, following the
established pattern (onQueryStarted + updateQueryData) used elsewhere in
ui/lib/store/apis so the optimistic update is not immediately re-fetched by a
tag invalidation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0c4ce490-117c-4782-b15b-0fa9db6772fd
⛔ Files ignored due to path filters (1)
ui/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
ui/app/_fallbacks/enterprise/lib/store/apis/scimApi.tsui/app/clientLayout.tsxui/components/onboardingWidget.tsxui/lib/store/apis/configApi.tsui/package.json
akshaydeo
force-pushed
the
05-20-onbaording_widget_ui
branch
from
May 20, 2026 04:59
e38b6c9 to
8f2a0ef
Compare
akshaydeo
force-pushed
the
05-20-onboarding_widget_backend_changes
branch
from
May 20, 2026 04:59
99e8bd0 to
a027d50
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
ui/lib/store/apis/configApi.ts (1)
98-123:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHonor delete semantics in the optimistic metadata patch.
Lines 98-99 say
{ key: null }removes a metadata key, but the optimistic updater on Lines 109-122 currently preserves that key with anullvalue. That leaves the widget state briefly out of sync with the server until the refetch lands.🩹 Suggested fix
const patchResults = [ dispatch( configApi.util.updateQueryData("getCoreConfig", {}, (draft) => { - draft.metadata = { - ...(draft.metadata ?? {}), - ...patch, - }; + draft.metadata ??= {}; + for (const [key, value] of Object.entries(patch)) { + if (value === null) { + delete draft.metadata[key]; + } else { + draft.metadata[key] = value; + } + } }), ), dispatch( configApi.util.updateQueryData("getCoreConfig", { fromDB: true }, (draft) => { - draft.metadata = { - ...(draft.metadata ?? {}), - ...patch, - }; + draft.metadata ??= {}; + for (const [key, value] of Object.entries(patch)) { + if (value === null) { + delete draft.metadata[key]; + } else { + draft.metadata[key] = value; + } + } }), ),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/lib/store/apis/configApi.ts` around lines 98 - 123, The optimistic updater in updateClientMetadata (onQueryStarted) currently spreads patch into draft.metadata which preserves keys set to null; change the two configApi.util.updateQueryData updaters (for "getCoreConfig" and "getCoreConfig" with { fromDB: true }) to apply patch keys individually: ensure draft.metadata exists, for each key in patch if patch[key] === null delete draft.metadata[key] else assign draft.metadata[key] = patch[key], so null values remove keys instead of leaving them with null.
🧹 Nitpick comments (1)
ui/components/onboardingWidget.tsx (1)
58-72: ⚡ Quick winSkip checklist queries when the widget is already hidden for this user.
isDismissedForMeis available synchronously from the cookie, but the component still subscribes to the provider/model/SCIM config queries before it returnsnull. That means a user who hid this for a year still pays the checklist fetch cost on every load. Gate these hooks behind a shared skip flag derived from the cookie/session dismissal state.Suggested change
const [closedForSession, setClosedForSession] = useState(false); const [activeStepId, setActiveStepId] = useState<string | null>(null); const [cookies, setCookie] = useCookies([ONBOARDING_DISMISSED_COOKIE]); +const isDismissedForMe = !!cookies[ONBOARDING_DISMISSED_COOKIE]; +const shouldSkipChecklistQueries = closedForSession || isDismissedForMe; const [updateMetadata, { isLoading: writingMetadata }] = useUpdateClientMetadataMutation(); const [fetchCoreConfig] = useLazyGetCoreConfigQuery(); -const { data: bifrostConfig } = useGetCoreConfigQuery({}); -const { data: allKeys } = useGetAllKeysQuery(); +const { data: bifrostConfig } = useGetCoreConfigQuery({}, { skip: shouldSkipChecklistQueries }); +const { data: allKeys } = useGetAllKeysQuery(undefined, { + skip: shouldSkipChecklistQueries, +}); const { data: vksResponse } = useGetVirtualKeysQuery(undefined, { - skip: !IS_ENTERPRISE, + skip: shouldSkipChecklistQueries || !IS_ENTERPRISE, }); const { data: modelConfigsResponse } = useGetModelConfigsQuery(undefined, { - skip: !IS_ENTERPRISE, + skip: shouldSkipChecklistQueries || !IS_ENTERPRISE, }); const { data: scimProviders } = useGetSCIMProvidersQuery(undefined, { - skip: !IS_ENTERPRISE, + skip: shouldSkipChecklistQueries || !IS_ENTERPRISE, }); - -const isDismissedForMe = !!cookies[ONBOARDING_DISMISSED_COOKIE];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/components/onboardingWidget.tsx` around lines 58 - 72, Compute a single synchronous skip flag from the cookie/session dismissal state (using ONBOARDING_DISMISSED_COOKIE and the isDismissedForMe logic after useCookies) and pass that flag into the query hooks' skip options so the component doesn't subscribe when dismissed; specifically, derive something like const skipChecklist = isDismissedForMe || /* other session check */ and use skip: skipChecklist for useGetCoreConfigQuery, useGetAllKeysQuery, useGetVirtualKeysQuery, useGetModelConfigsQuery, and useGetSCIMProvidersQuery (and avoid calling fetchCoreConfig when skipChecklist is true).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@ui/lib/store/apis/configApi.ts`:
- Around line 98-123: The optimistic updater in updateClientMetadata
(onQueryStarted) currently spreads patch into draft.metadata which preserves
keys set to null; change the two configApi.util.updateQueryData updaters (for
"getCoreConfig" and "getCoreConfig" with { fromDB: true }) to apply patch keys
individually: ensure draft.metadata exists, for each key in patch if patch[key]
=== null delete draft.metadata[key] else assign draft.metadata[key] =
patch[key], so null values remove keys instead of leaving them with null.
---
Nitpick comments:
In `@ui/components/onboardingWidget.tsx`:
- Around line 58-72: Compute a single synchronous skip flag from the
cookie/session dismissal state (using ONBOARDING_DISMISSED_COOKIE and the
isDismissedForMe logic after useCookies) and pass that flag into the query
hooks' skip options so the component doesn't subscribe when dismissed;
specifically, derive something like const skipChecklist = isDismissedForMe || /*
other session check */ and use skip: skipChecklist for useGetCoreConfigQuery,
useGetAllKeysQuery, useGetVirtualKeysQuery, useGetModelConfigsQuery, and
useGetSCIMProvidersQuery (and avoid calling fetchCoreConfig when skipChecklist
is true).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fb3bf3ca-9604-497c-ab04-ce7dc6d2bfa6
⛔ Files ignored due to path filters (1)
ui/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
ui/app/_fallbacks/enterprise/lib/store/apis/scimApi.tsui/app/clientLayout.tsxui/components/onboardingWidget.tsxui/lib/store/apis/configApi.tsui/package.json
akshaydeo
force-pushed
the
05-20-onbaording_widget_ui
branch
from
May 20, 2026 05:09
8f2a0ef to
dfc1913
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
ui/components/onboardingWidget.tsx (1)
219-227:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSerialize skip writes; the refetch still doesn't prevent double-skip clobbering.
This narrows the stale-read window, but two quick Skip clicks can still fetch the same pre-update metadata and then overwrite each other;
writingMetadataonly flips after the first fetch completes. You still need a local pending union/queue so every mutation writes the merged skipped-id set.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/components/onboardingWidget.tsx` around lines 219 - 227, handleSkip can still clobber concurrent clicks because successive fetchCoreConfig calls may return the same stale metadata; create a local pending set/queue (e.g., pendingSkippedIds) and on click immediately add stepId to it, then when performing the update (inside handleSkip) fetch latest with fetchCoreConfig, compute the union of latest metadata (latestConfig?.metadata?.[METADATA_SKIPPED_KEY]), the current skippedIds, and pendingSkippedIds (deduplicated), call updateMetadata with that merged set, and only remove the stepId from pending on request completion/failure; also serialize updates by awaiting any in-flight update promise (or use a simple FIFO promise chain) so handleSkip always writes the union of latest+pending to avoid double-skip clobbering.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ui/components/onboardingWidget.tsx`:
- Around line 73-76: The widget currently computes doneCount and renders the
checklist even when dependent query data is unresolved; change the render logic
to gate computing and rendering on the boolean checklistReady (the existing
const checklistReady) so that doneCount and the checklist UI are only
calculated/shown after bifrostConfig, allKeys, and — when IS_ENTERPRISE —
vksResponse, modelConfigsResponse and scimProviders are defined; otherwise
render a loading or error state. Update any other spots that compute or display
progress (the code around doneCount and the checklist render block) to use
checklistReady as the guard so unresolved queries don’t produce a transient or
stale “0 / N” UI.
In `@ui/lib/store/apis/configApi.ts`:
- Around line 5-15: applyMetadataPatch currently shallow-merges only the first
level, causing nested objects (e.g. metadata.onboarding) to be replaced instead
of patched; update applyMetadataPatch to implement JSON Merge Patch semantics:
when patch value is null delete the key, when patch value and target value are
both plain objects (use a plain-object check: typeof === "object" && value !==
null && !Array.isArray(...)) recurse into applyMetadataPatch to merge nested
keys, otherwise set/replace the key; ensure arrays and non-objects are replaced
(not recursed) and return a new object copy (preserve immutability).
---
Duplicate comments:
In `@ui/components/onboardingWidget.tsx`:
- Around line 219-227: handleSkip can still clobber concurrent clicks because
successive fetchCoreConfig calls may return the same stale metadata; create a
local pending set/queue (e.g., pendingSkippedIds) and on click immediately add
stepId to it, then when performing the update (inside handleSkip) fetch latest
with fetchCoreConfig, compute the union of latest metadata
(latestConfig?.metadata?.[METADATA_SKIPPED_KEY]), the current skippedIds, and
pendingSkippedIds (deduplicated), call updateMetadata with that merged set, and
only remove the stepId from pending on request completion/failure; also
serialize updates by awaiting any in-flight update promise (or use a simple FIFO
promise chain) so handleSkip always writes the union of latest+pending to avoid
double-skip clobbering.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d5d5d7aa-b29d-4c1e-a9f9-64c79c015ae9
⛔ Files ignored due to path filters (1)
ui/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
ui/app/_fallbacks/enterprise/lib/store/apis/scimApi.tsui/app/clientLayout.tsxui/components/onboardingWidget.tsxui/lib/store/apis/configApi.tsui/package.json
akshaydeo
force-pushed
the
05-20-onboarding_widget_backend_changes
branch
from
May 20, 2026 05:33
a027d50 to
51e6ecd
Compare
akshaydeo
force-pushed
the
05-20-onbaording_widget_ui
branch
from
May 20, 2026 05:33
dfc1913 to
b28e19e
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
ui/components/onboardingWidget.tsx (1)
73-76:⚠️ Potential issue | 🟠 MajorUse
checklistReadyto suppress the widget until its data is loaded.This still renders the checklist from partial query state, so users can see incorrect progress/completion until the dependent queries resolve. The existing
checklistReadyguard should run beforedoneCountand the early-hide path.Suggested fix
if (closedForSession || isDismissedForAll || isDismissedForMe) { return null; } + if (!checklistReady) { + return null; + } + const isStepDone = (step: Step) => step.complete || skippedIds.includes(step.id); const doneCount = steps.filter(isStepDone).length;Also applies to: 204-209
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/components/onboardingWidget.tsx` around lines 73 - 76, The checklist is computed/rendered from partial query state; move and apply the existing checklistReady guard so the component returns early (hides/suspends rendering) whenever checklistReady is false before any use of doneCount or any early-hide logic; specifically ensure checklistReady is checked at the top of the render/effect path that computes doneCount and before the branch that hides the widget (the "early-hide" path) so doneCount and the checklist JSX are only evaluated when checklistReady is true (update references around checklistReady, doneCount, and the early-hide condition in onboardingWidget.tsx).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@ui/components/onboardingWidget.tsx`:
- Around line 73-76: The checklist is computed/rendered from partial query
state; move and apply the existing checklistReady guard so the component returns
early (hides/suspends rendering) whenever checklistReady is false before any use
of doneCount or any early-hide logic; specifically ensure checklistReady is
checked at the top of the render/effect path that computes doneCount and before
the branch that hides the widget (the "early-hide" path) so doneCount and the
checklist JSX are only evaluated when checklistReady is true (update references
around checklistReady, doneCount, and the early-hide condition in
onboardingWidget.tsx).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7eca4d3f-2ada-4efc-91ce-39e29f85fee8
⛔ Files ignored due to path filters (1)
ui/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
ui/app/_fallbacks/enterprise/lib/store/apis/scimApi.tsui/app/clientLayout.tsxui/components/onboardingWidget.tsxui/lib/store/apis/configApi.tsui/package.json
✅ Files skipped from review due to trivial changes (1)
- ui/package.json
akshaydeo
force-pushed
the
05-20-onboarding_widget_backend_changes
branch
from
May 20, 2026 05:48
51e6ecd to
3da31f2
Compare
akshaydeo
force-pushed
the
05-20-onbaording_widget_ui
branch
from
May 20, 2026 05:48
b28e19e to
9851bce
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
ui/components/onboardingWidget.tsx (1)
200-209:⚠️ Potential issue | 🟠 Major | ⚡ Quick winWidget renders before data is ready, showing transient incorrect progress.
The early-return logic checks dismissal and completion, but does not gate on
checklistReady. When queries are still loading,stepsare computed with undefined data (all falling back to "incomplete"), causing a brief flash of0 / Neven for setups that are already complete.Add a guard before computing
doneCount:if (closedForSession || isDismissedForAll || isDismissedForMe) { return null; } + if (!checklistReady) { + return null; // or a skeleton/loading state + } + const isStepDone = (step: Step) => step.complete || skippedIds.includes(step.id); const doneCount = steps.filter(isStepDone).length;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/components/onboardingWidget.tsx` around lines 200 - 209, The widget is calculating progress before data is ready causing a transient incorrect UI; before computing isStepDone/doneCount add a guard on checklistReady (the boolean used by your queries) and return null (or otherwise avoid rendering) while checklistReady is false so steps, skippedIds and related logic only run when data is loaded; update the flow around closedForSession/isDismissedForAll/isDismissedForMe -> check checklistReady -> then compute isStepDone, doneCount and render.
🧹 Nitpick comments (1)
ui/lib/store/apis/configApi.ts (1)
118-130: Consider also patching{ fromDB: false }for completeness, even though no current callers use it explicitly.While the current optimistic update covers
{}and{ fromDB: true }, any future caller usinggetCoreConfigwith{ fromDB: false }would not have its cache updated, since RTK Query treats it as a distinct cache key. No existing call sites use{ fromDB: false }currently, but adding it would make the update logic more robust:Suggested addition
dispatch( configApi.util.updateQueryData("getCoreConfig", { fromDB: false }, (draft) => { draft.metadata = applyMetadataPatch(draft.metadata, patch); }), ),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/lib/store/apis/configApi.ts` around lines 118 - 130, The optimistic update in onQueryStarted currently patches getCoreConfig for keys {} and { fromDB: true } but misses the distinct cache key { fromDB: false }; add a third dispatch call using configApi.util.updateQueryData("getCoreConfig", { fromDB: false }, ...) that applies applyMetadataPatch to draft.metadata (same pattern as the existing two dispatches) so the { fromDB: false } cache entry is also updated.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@ui/components/onboardingWidget.tsx`:
- Around line 200-209: The widget is calculating progress before data is ready
causing a transient incorrect UI; before computing isStepDone/doneCount add a
guard on checklistReady (the boolean used by your queries) and return null (or
otherwise avoid rendering) while checklistReady is false so steps, skippedIds
and related logic only run when data is loaded; update the flow around
closedForSession/isDismissedForAll/isDismissedForMe -> check checklistReady ->
then compute isStepDone, doneCount and render.
---
Nitpick comments:
In `@ui/lib/store/apis/configApi.ts`:
- Around line 118-130: The optimistic update in onQueryStarted currently patches
getCoreConfig for keys {} and { fromDB: true } but misses the distinct cache key
{ fromDB: false }; add a third dispatch call using
configApi.util.updateQueryData("getCoreConfig", { fromDB: false }, ...) that
applies applyMetadataPatch to draft.metadata (same pattern as the existing two
dispatches) so the { fromDB: false } cache entry is also updated.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ae12f1dc-58f4-4cc5-8d3c-badc30efe5a4
⛔ Files ignored due to path filters (1)
ui/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
ui/app/_fallbacks/enterprise/lib/store/apis/scimApi.tsui/app/clientLayout.tsxui/components/onboardingWidget.tsxui/lib/store/apis/configApi.tsui/package.json
✅ Files skipped from review due to trivial changes (1)
- ui/package.json
akshaydeo
force-pushed
the
05-20-onbaording_widget_ui
branch
from
May 20, 2026 05:54
9851bce to
e7aa894
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ui/components/onboardingWidget.tsx`:
- Around line 328-390: The row-level `skipped`/`done` calculation only reads
persisted skippedIds but doneCount already includes optimistic
pendingSkippedIds, causing UI mismatch during slow writes; update the mapping in
the steps.map render to treat a step as skipped if either skippedIds or
pendingSkippedIds includes step.id (e.g., compute skipped =
skippedIds.includes(step.id) || pendingSkippedIds.includes(step.id)) and then
compute done = step.complete || skipped so the button disabled/visual state
(used by handleStepClick, handleSkip, and the Checkbox) matches the optimistic
doneCount state.
- Around line 62-85: Derive isDismissedForAll immediately after reading
bifrostConfig (useGetCoreConfigQuery) and use that flag in the skip conditions
for the other checklist queries (allKeys, useGetVirtualKeysQuery,
useGetModelConfigsQuery, useGetSCIMProvidersQuery) so they don't run when the
global-dismiss metadata (METADATA_DISMISSED_KEY) is true; specifically, compute
isDismissedForAll = bifrostConfig?.metadata?.[METADATA_DISMISSED_KEY] === true
right after bifrostConfig is available and include it in the skip expressions
(in addition to shouldSkipChecklistQueries/IS_ENTERPRISE) for the listed
queries.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2e54ecf5-d6df-4f4d-942f-1589cc05adf4
⛔ Files ignored due to path filters (1)
ui/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
ui/app/_fallbacks/enterprise/lib/store/apis/scimApi.tsui/app/clientLayout.tsxui/components/onboardingWidget.tsxui/lib/store/apis/configApi.tsui/package.json
akshaydeo
force-pushed
the
05-20-onbaording_widget_ui
branch
from
May 20, 2026 06:49
e7aa894 to
c31d8ca
Compare
akshaydeo
force-pushed
the
05-20-onbaording_widget_ui
branch
from
May 20, 2026 07:00
c31d8ca to
4e74498
Compare
akshaydeo
force-pushed
the
05-20-onboarding_widget_backend_changes
branch
from
May 20, 2026 07:00
3da31f2 to
135a863
Compare
Contributor
Author
Merge activity
|
akshaydeo
changed the base branch from
05-20-onboarding_widget_backend_changes
to
graphite-base/3606
May 20, 2026 07:05
akshaydeo
dismissed
coderabbitai[bot]’s stale review
May 20, 2026 07:05
The base branch was changed.
akshaydeo
added a commit
that referenced
this pull request
May 20, 2026
## Summary
Adds a floating onboarding checklist widget that guides new admins through essential setup steps (CORS restrictions, dashboard auth, inference auth enforcement, provider keys, and enterprise-only steps like virtual keys, model catalog, and SCIM provisioning). The widget appears in the bottom-right corner when the database is connected and disappears automatically once all steps are complete or dismissed.
## Changes
- Added `OnboardingWidget` component that renders a fixed-position checklist card with grouped sections ("Security", "Provider Setup", "Everything Else")
- Steps are dynamically derived from live config/API data so checkboxes reflect real completion state without manual input
- Confetti fires from the checkbox position when a step transitions from incomplete to complete
- When a user clicks a step, the widget scales out and the backdrop lifts so the destination page is fully usable; the widget scales back in once the step completes or is skipped
- Individual steps can be skipped (persisted to `ClientConfig.metadata` so all users see the skip), the widget can be hidden per-user via a 1-year cookie, or dismissed for all users via a metadata flag
- Added `updateClientMetadata` RTK Query mutation (`POST /config/metadata`) to merge-patch the metadata blob for persisting dismissed/skipped state
- Added `useGetSCIMProvidersQuery` OSS stub in the enterprise fallbacks so the SCIM step is always treated as incomplete in OSS builds (the step itself is hidden via `IS_ENTERPRISE`)
- Added `canvas-confetti` and its types as dependencies
## Type of change
- [ ] Bug fix
- [x] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [ ] Core (Go)
- [ ] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [x] UI (React)
- [ ] Docs
## How to test
```sh
cd ui
npm i
npm run build
```
1. Start the app with a connected database (`is_db_connected: true`).
2. Confirm the onboarding widget appears in the bottom-right corner.
3. Complete one of the listed steps (e.g., add a provider key) and verify the corresponding checkbox checks off with confetti.
4. Click a step row and confirm the widget scales out while the backdrop lifts, then scales back in after completing or skipping the step.
5. Click "Skip" on an individual step and confirm it persists across page reloads.
6. Click "I'll do it later" and confirm the widget is hidden for the session (cookie-based).
7. Click "Skip" in the footer and confirm the widget is hidden for all users (metadata-based).
8. Complete all steps and confirm the widget disappears automatically.
9. In an OSS build, confirm the SCIM step is not shown.
## Screenshots/Recordings
_Add before/after screenshots or a short clip of the widget in action._
## Breaking changes
- [ ] Yes
- [x] No
## Related issues
_Link related issues here._
## Security considerations
Dismissal state is stored in `ClientConfig.metadata` and is accessible to any admin who can read the config. No secrets or PII are involved. The per-user dismissal uses a non-sensitive browser cookie scoped to `/`.
## Checklist
- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [ ] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [ ] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
Merged
akhsaul
pushed a commit
to akhsaul/bifrost
that referenced
this pull request
Aug 27, 2026
## Summary
Adds a floating onboarding checklist widget that guides new admins through essential setup steps (CORS restrictions, dashboard auth, inference auth enforcement, provider keys, and enterprise-only steps like virtual keys, model catalog, and SCIM provisioning). The widget appears in the bottom-right corner when the database is connected and disappears automatically once all steps are complete or dismissed.
## Changes
- Added `OnboardingWidget` component that renders a fixed-position checklist card with grouped sections ("Security", "Provider Setup", "Everything Else")
- Steps are dynamically derived from live config/API data so checkboxes reflect real completion state without manual input
- Confetti fires from the checkbox position when a step transitions from incomplete to complete
- When a user clicks a step, the widget scales out and the backdrop lifts so the destination page is fully usable; the widget scales back in once the step completes or is skipped
- Individual steps can be skipped (persisted to `ClientConfig.metadata` so all users see the skip), the widget can be hidden per-user via a 1-year cookie, or dismissed for all users via a metadata flag
- Added `updateClientMetadata` RTK Query mutation (`POST /config/metadata`) to merge-patch the metadata blob for persisting dismissed/skipped state
- Added `useGetSCIMProvidersQuery` OSS stub in the enterprise fallbacks so the SCIM step is always treated as incomplete in OSS builds (the step itself is hidden via `IS_ENTERPRISE`)
- Added `canvas-confetti` and its types as dependencies
## Type of change
- [ ] Bug fix
- [x] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [ ] Core (Go)
- [ ] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [x] UI (React)
- [ ] Docs
## How to test
```sh
cd ui
npm i
npm run build
```
1. Start the app with a connected database (`is_db_connected: true`).
2. Confirm the onboarding widget appears in the bottom-right corner.
3. Complete one of the listed steps (e.g., add a provider key) and verify the corresponding checkbox checks off with confetti.
4. Click a step row and confirm the widget scales out while the backdrop lifts, then scales back in after completing or skipping the step.
5. Click "Skip" on an individual step and confirm it persists across page reloads.
6. Click "I'll do it later" and confirm the widget is hidden for the session (cookie-based).
7. Click "Skip" in the footer and confirm the widget is hidden for all users (metadata-based).
8. Complete all steps and confirm the widget disappears automatically.
9. In an OSS build, confirm the SCIM step is not shown.
## Screenshots/Recordings
_Add before/after screenshots or a short clip of the widget in action._
## Breaking changes
- [ ] Yes
- [x] No
## Related issues
_Link related issues here._
## Security considerations
Dismissal state is stored in `ClientConfig.metadata` and is accessible to any admin who can read the config. No secrets or PII are involved. The per-user dismissal uses a non-sensitive browser cookie scoped to `/`.
## Checklist
- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [ ] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [ ] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
occcat
pushed a commit
to occcat/bifrost
that referenced
this pull request
Sep 2, 2026
## Summary
Adds a floating onboarding checklist widget that guides new admins through essential setup steps (CORS restrictions, dashboard auth, inference auth enforcement, provider keys, and enterprise-only steps like virtual keys, model catalog, and SCIM provisioning). The widget appears in the bottom-right corner when the database is connected and disappears automatically once all steps are complete or dismissed.
## Changes
- Added `OnboardingWidget` component that renders a fixed-position checklist card with grouped sections ("Security", "Provider Setup", "Everything Else")
- Steps are dynamically derived from live config/API data so checkboxes reflect real completion state without manual input
- Confetti fires from the checkbox position when a step transitions from incomplete to complete
- When a user clicks a step, the widget scales out and the backdrop lifts so the destination page is fully usable; the widget scales back in once the step completes or is skipped
- Individual steps can be skipped (persisted to `ClientConfig.metadata` so all users see the skip), the widget can be hidden per-user via a 1-year cookie, or dismissed for all users via a metadata flag
- Added `updateClientMetadata` RTK Query mutation (`POST /config/metadata`) to merge-patch the metadata blob for persisting dismissed/skipped state
- Added `useGetSCIMProvidersQuery` OSS stub in the enterprise fallbacks so the SCIM step is always treated as incomplete in OSS builds (the step itself is hidden via `IS_ENTERPRISE`)
- Added `canvas-confetti` and its types as dependencies
## Type of change
- [ ] Bug fix
- [x] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [ ] Core (Go)
- [ ] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [x] UI (React)
- [ ] Docs
## How to test
```sh
cd ui
npm i
npm run build
```
1. Start the app with a connected database (`is_db_connected: true`).
2. Confirm the onboarding widget appears in the bottom-right corner.
3. Complete one of the listed steps (e.g., add a provider key) and verify the corresponding checkbox checks off with confetti.
4. Click a step row and confirm the widget scales out while the backdrop lifts, then scales back in after completing or skipping the step.
5. Click "Skip" on an individual step and confirm it persists across page reloads.
6. Click "I'll do it later" and confirm the widget is hidden for the session (cookie-based).
7. Click "Skip" in the footer and confirm the widget is hidden for all users (metadata-based).
8. Complete all steps and confirm the widget disappears automatically.
9. In an OSS build, confirm the SCIM step is not shown.
## Screenshots/Recordings
_Add before/after screenshots or a short clip of the widget in action._
## Breaking changes
- [ ] Yes
- [x] No
## Related issues
_Link related issues here._
## Security considerations
Dismissal state is stored in `ClientConfig.metadata` and is accessible to any admin who can read the config. No secrets or PII are involved. The per-user dismissal uses a non-sensitive browser cookie scoped to `/`.
## Checklist
- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [ ] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [ ] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a floating onboarding checklist widget that guides new admins through essential setup steps (CORS restrictions, dashboard auth, inference auth enforcement, provider keys, and enterprise-only steps like virtual keys, model catalog, and SCIM provisioning). The widget appears in the bottom-right corner when the database is connected and disappears automatically once all steps are complete or dismissed.
Changes
OnboardingWidgetcomponent that renders a fixed-position checklist card with grouped sections ("Security", "Provider Setup", "Everything Else")ClientConfig.metadataso all users see the skip), the widget can be hidden per-user via a 1-year cookie, or dismissed for all users via a metadata flagupdateClientMetadataRTK Query mutation (POST /config/metadata) to merge-patch the metadata blob for persisting dismissed/skipped stateuseGetSCIMProvidersQueryOSS stub in the enterprise fallbacks so the SCIM step is always treated as incomplete in OSS builds (the step itself is hidden viaIS_ENTERPRISE)canvas-confettiand its types as dependenciesType of change
Affected areas
How to test
cd ui npm i npm run buildis_db_connected: true).Screenshots/Recordings
Add before/after screenshots or a short clip of the widget in action.
Breaking changes
Related issues
Link related issues here.
Security considerations
Dismissal state is stored in
ClientConfig.metadataand is accessible to any admin who can read the config. No secrets or PII are involved. The per-user dismissal uses a non-sensitive browser cookie scoped to/.Checklist
docs/contributing/README.mdand followed the guidelines