Skip to content

Fix progress dots mapping, persist variant, clamp restored step #1363

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
alex-nork merged 1 commit into from
Feb 13, 2026
Merged

Fix progress dots mapping, persist variant, clamp restored step #1363

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion clients/macos/vellum-assistant/Features/Onboarding/FirstMeeting/FirstMeetingFlowView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ struct FirstMeetingFlowView: View {
)
.id("\(state.currentStep)-\(observationPhase)")

OnboardingProgressDots(currentStep: state.currentStep)
OnboardingProgressDots(currentStep: state.currentStep, totalSteps: 5)
.padding(.top, VSpacing.xs)
}
.padding(.horizontal, VSpacing.xxl)
Expand Down
25 changes: 16 additions & 9 deletions clients/macos/vellum-assistant/Features/Onboarding/OnboardingProgressDots.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,20 +1,27 @@
import SwiftUI

/// Five cumulative progress dots for the onboarding flow (steps 0-6).
/// Five cumulative progress dots for the onboarding flow.
///
/// By default maps 7 steps (0-6) to 5 dots. Pass a custom `totalSteps`
/// for flows with a different number of steps (e.g. `totalSteps: 5` for
/// the first-meeting flow).
struct OnboardingProgressDots: View {
let currentStep: Int
let totalSteps: Int

private let totalDots = 5

/// Maps onboarding step (0-6) to active dot index (0-4).
init(currentStep: Int, totalSteps: Int = 7) {
self.currentStep = currentStep
self.totalSteps = totalSteps
}

/// Maps the current step to the active dot index (0 ..< totalDots)
/// by evenly distributing `totalSteps` across `totalDots`.
private var activeDotIndex: Int {
switch currentStep {
case 0, 1: return 0
case 2: return 1
case 3: return 2
case 4: return 3
default: return 4
}
guard totalSteps > 1 else { return 0 }
let fraction = Double(currentStep) / Double(totalSteps - 1)
return min(Int(fraction * Double(totalDots - 1) + 0.5), totalDots - 1)
Comment on lines +22 to +24

@devin-ai-integration devin-ai-integration Bot Feb 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Default flow progress dots regression: step 5 no longer lights the last dot

The new generic formula for activeDotIndex changes the default flow's step-to-dot mapping in a way that breaks the previous behavior.

Detailed mapping comparison and impact

The old hand-coded mapping was:

  • Steps 0,1 → dot 0
  • Step 2 → dot 1
  • Step 3 → dot 2
  • Step 4 → dot 3
  • Steps 5,6 (default) → dot 4

The new formula Int(fraction * Double(totalDots - 1) + 0.5) with totalSteps=7 produces:

  • Step 0 → dot 0
  • Steps 1,2 → dot 1
  • Step 3 → dot 2
  • Steps 4,5 → dot 3
  • Step 6 → dot 4

The critical difference is step 5 (ScreenPermissionStepView): it previously mapped to dot 4 (all 5 dots lit), but now maps to dot 3 (only 4 dots lit). This means users on the Screen Permission step see only 4/5 dots filled instead of 5/5, which is a visual regression.

Additionally, step 1 (NamingStepView) now advances the dot from 0 to 1, whereas before steps 0 and 1 shared dot 0. This changes the perceived progress pacing for the default onboarding flow.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

}

var body: some View {
Expand Down
8 changes: 8 additions & 0 deletions clients/macos/vellum-assistant/Features/Onboarding/OnboardingState.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,14 @@ final class OnboardingState {
onboardingVariant = variant
}
firstMeetingCrackProgress = CGFloat(UserDefaults.standard.double(forKey: "onboarding.firstMeetingCrackProgress"))

// Clamp restored step to the variant's maximum to prevent out-of-range
// rendering (e.g. a step saved from the 7-step default flow would be
// invalid for the 5-step first-meeting flow).
let maxStep = onboardingVariant == .firstMeeting ? 4 : 6
if currentStep > maxStep {
currentStep = maxStep
}
}

func advance() {
Expand Down
1 change: 1 addition & 0 deletions clients/macos/vellum-assistant/Features/Onboarding/OnboardingWindow.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ final class OnboardingWindow {
idx + 1 < CommandLine.arguments.count,
CommandLine.arguments[idx + 1] == "first_meeting" {
state.onboardingVariant = .firstMeeting
UserDefaults.standard.set(OnboardingVariant.firstMeeting.rawValue, forKey: "onboarding.variant")
Comment on lines 23 to +24

@chatgpt-codex-connector chatgpt-codex-connector Bot Feb 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Re-clamp restored step after CLI variant override

When --onboarding-variant first_meeting is passed, show() switches state.onboardingVariant after OnboardingState.init() has already clamped currentStep using the previously persisted variant. If a user has onboarding.step saved as 5 or 6 from the default flow, this path leaves currentStep out of range for first-meeting (0...4), and FirstMeetingFlowView falls into its default case (EmptyView) instead of rendering a valid step. This makes the new clamp ineffective for the CLI-switch scenario the commit is targeting.

Useful? React with 👍 / 👎.

Comment on lines 23 to +24

@devin-ai-integration devin-ai-integration Bot Feb 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Step clamping in init() runs before CLI variant override, leaving step out of range

When launching with --onboarding-variant first_meeting after a previous default-flow session saved a step > 4, the step is not properly clamped because the variant override happens after init() has already clamped.

Root cause and reproduction

OnboardingState is initialized at OnboardingWindow declaration time (clients/macos/vellum-assistant/Features/Onboarding/OnboardingWindow.swift:7). The init() at OnboardingState.swift:69-93 reads the variant from UserDefaults and clamps the step accordingly. However, in OnboardingWindow.show() at OnboardingWindow.swift:20-25, the variant is overridden from the command line argument after init() has already completed.

Reproduction:

  1. Run default onboarding, reach step 5, kill the app (saves onboarding.step=5, onboarding.variant=default)
  2. Relaunch with --onboarding-variant first_meeting
  3. OnboardingState.init() reads variant as .default from UserDefaults → clamps with maxStep=6 → currentStep stays at 5
  4. show() sets state.onboardingVariant = .firstMeeting
  5. Now currentStep=5 with onboardingVariant=.firstMeeting — step 5 hits the default: EmptyView() case in FirstMeetingFlowView.swift:82-83, rendering a blank screen

This only occurs on the first launch after switching variants via CLI when a higher step was previously saved. Subsequent launches work correctly because the variant is now persisted.

Suggested change
state.onboardingVariant = .firstMeeting
UserDefaults.standard.set(OnboardingVariant.firstMeeting.rawValue, forKey: "onboarding.variant")
state.onboardingVariant = .firstMeeting
UserDefaults.standard.set(OnboardingVariant.firstMeeting.rawValue, forKey: "onboarding.variant")
let maxStep = 4
if state.currentStep > maxStep {
state.currentStep = maxStep
}
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

}
#endif

Expand Down