40 KiB
iOS: Chat-Path Onboarding — Tracker Blocking Demo
Asana: Ship Review task 1214147157456478
Branch stack:demo-tracker-blocking-onboarding→ui-polish→chat-path-dialog-polish→sr-feedback→uti-flow
Due: 2026-05-15
PR Stack
| PR | Branch | Review status |
|---|---|---|
| #4544 | demo-tracker-blocking-onboarding |
Open — review required (Alessandro + Rachel left comments) |
| #4584 | demo-tracker-blocking-onboarding-eoj-dax-animation |
Merged ✅ 2026-05-18 |
| #4591 | demo-tracker-blocking-onboarding-ui-polish |
Merged ✅ 2026-05-15 |
| #4664 | demo-tracker-blocking-onboarding-chat-path-dialog-polish |
Merged ✅ 2026-05-15 |
| #4855 | demo-tracker-blocking-onboarding-sr-feedback |
Open — no review yet |
| #4668 | demo-tracker-blocking-onboarding-uti-flow |
Open — no review yet |
Open Items
✅ SR — GJ2/ Remove X button from chat-path dialogs
Source: Gary Apr 29
Fix: X (manual dismiss) button removed from "Try visiting a site!", trackers-blocked, and "Oh, before I forget…" dialogs on the chat path.
State: Done. Committed in PR #4664.
✅ SR — GJ4/ Copy: "Next, try visiting a site!" after trying AI
Source: Gary Apr 29
Fix: "Try visiting a site!" dialog title updated to "Next, try visiting a site!" for chat-path context.
State: Done. Committed in PR #4664.
🟠 SR — GJ3/ Hide address bar and toolbar for "Try visiting a site!" step (chat-path only)
Source: Gary Apr 29, confirmed by Gary May 7 and Costas May 8: hide address bar + toolbar for AI-chat flow only so user is forced to choose from presets. Without this: user could type a search (repeating the same dialog) or switch to Duck.ai. Asana task: 1214828145455337 "Try Visiting a Site: Remove Browser Toolbar".
Fix: chromeDelegate?.setBarsHidden(true) on show, setBarsHidden(false) on dismiss. Tracked as PR #4855 — Alessandro (line 477) item below.
State: sr-feedback / uncommitted — in working tree, NOT committed.
✅ SR — Design: "Try a Search or AI chat" dialog — Remove pointer/tail
Source: Asana task 1214846580751519 (Ship Review: Design, May 15). Remove the bubble pointer/tail from the bottom of the "Try a Search or AI chat" dialog container since Dax is not present in this dialog.
State: ✅ Done. tailPosition: nil in RebrandedContextualOnboardingDialogs+TrySearch.swift. Committed in merged PR.
✅ SR — Design: "You've Got This!" dialog — Remove pointer/tail
Source: Asana task 1214828145455345 (Ship Review: Design, May 15). Remove the bubble pointer/tail from the bottom of the "You've Got This!" EOJ dialog container since Dax is not present.
State: ✅ Done. tailPosition: nil in RebrandedContextualOnboardingDialogs+EndOfJourney.swift. In uncommitted working tree on demo-tracker-blocking-onboarding.
🟡 SR — Design: "Try Visiting a Site" screen — Add background
Source: Asana task 1214828145455342 (Ship Review: Design, May 15). After hiding address bar + toolbar (GJ3), the screen is very empty. Gary suggests switching in an existing background for delight. Not a 100% blocker for the experiment.
State: 🟡 Nice-to-have. Not implemented. Not in any PR.
🟠 SR — GJ7/ Exclude sync-restore / returning users from Duck.ai experiment step
Source: Alessandro May 7: if user restored data from sync, Dax dialogs are disabled — enrolling them would break the flow. Alex confirmed addressed in May 13 build comment ("currently commented out in PR").
Fix: Uncommented guard case .introDialog(isReturningUser: false) = introSteps.first in insertExperimentStepIfNeeded(). Tracked as PR #4855 — Bugbot "Returning-user guard commented out" item below.
State: sr-feedback / uncommitted — in working tree, NOT committed.
🟠 SR — GJ8/ Change "Duck.ai" to "Ask AI" on toggle choice screen (onboarding only)
Source: Gary May 6: "Let's also change 'Duck.ai' to 'Ask AI' in the toggle screen in onboarding. Only change it in onboarding though, not the standard toggle."
Fix: DuckAI query toggle label updated to "Ask AI" in OnboardingView+DuckAIExperimentSearchContent.swift and UserText.swift. Tracked as PR #4855 — Alessandro (UserText.swift:2375) item below.
⚠️ Alessandro's review comment says "Ask.ai" (with dot) vs GJ8 "Ask AI" (no dot) — needs product confirmation.
State: sr-feedback / uncommitted — in working tree, NOT committed.
✅ SR — GJ6b/ "You've got this" EOJ fails after tapping "Got it" on tracker blocking step
Source: Gary Apr 29: "This dialogue fails to appear after tapping 'Got it' on the tracker blocking step. As a result, we also skip the subscription upsell dialogue."
Fix: if contextualOnboardingLogic.chatPathPhase == .trackerToEOJ { delegate?.tabDidRequestNewTab(self) } added to didTapDismissContextualOnboardingAction in TabViewController.swift.
State: ✅ Fix landed in alex/demo-tracker-blocking-onboarding via PR #4584 merge (2026-05-18). GitHub thread on #4544 still open — needs resolving.
🔴 SR — GJ1/ Animated toggle thumbnail
Source: Gary Apr 29 + May 6 confirmed in scope. Costas May 8: task 1214627423981798 created for animated assets. If not in time, defer to follow-up project. Alessandro May 10: nice-to-have, defer if needed.
State: 🔴 NOT implemented. Nice-to-have — defer to follow-up if time doesn't allow.
🔴 SR — GJ6a/ UTI: keyboard appears after choosing AI, hiding response and fire button
Source: Gary Apr 29. Pete May 7: UTI experiment participants will be excluded from this experiment; UTI issues can be fixed in follow-up before June 1. Alessandro May 7: follow-up task 1214592943696234.
State: 🔴 NOT implemented here. Scoped to separate follow-up project (UTI + experiment winner work).
🔴 SR — GJ6/ UTI new breakage (visit-site dialog, bar state)
Source: Alex May 13: "new breakage since my last fixes, will do next." Tracked in PR #4668.
State: 🔴 In progress. PR #4668 has open comment threads (see aataraxiaa + Bugbot items below).
➖ SR — GJ5/ Add new Dax brand assets to onboarding
Source: Gary Apr 29.
State: Deferred. Costas/Alessandro agreed: out of scope for this project, addressed in follow-up.
➖ SR — CB1/ Welcome screen copy update
Source: Costas May 8. Alessandro May 10: comes for free once task 1213967368170629 merges.
State: Covered by separate task. No action needed here.
✏️ COMMIT-1 — All working-tree changes on sr-feedback are uncommitted
State: All of the items below marked sr-feedback / uncommitted are local-only on alex/demo-tracker-blocking-onboarding-sr-feedback. None have been committed or pushed.
iOS/DuckDuckGo/MainViewController.swiftiOS/DuckDuckGo/NewTabPageControllerDelegate.swiftiOS/DuckDuckGo/NewTabPageViewController.swiftiOS/DuckDuckGo/OnboardingFlow/LinearOnboarding/OnboardingIntroViewModel.swift— also needs!restorePromptHandler.isEligibleForRestorePrompt()added (see PR #4855 Alessandro line 450)iOS/DuckDuckGo/OnboardingView+DuckAIExperimentSearchContent.swift— label currently "Ask AI", needs correction to "Ask.ai" (with dot)iOS/DuckDuckGo/UserText.swift+ all 26 lproj files
Action: Fix "Ask.ai" copy + add isEligibleForRestorePrompt guard, then commit and push before #4855 can be reviewed.
Color legend: 🟠 fix in working tree, uncommitted — 🟡 fix in another PR in stack, not merged to this one — 🔴 not fixed / blocked — ✅ done
PR #4544 Comments
Threads in GitHub order. Resolved threads grouped into tables.
✅ [Resolved] PR #4544 — Bugbots (outdated): hardcoded cohort / promo override / missing pixels / commented-out redirect
Source: Four Bugbot comments on early commits of PR #4544.
| Thread | Fix | Code state |
|---|---|---|
Hardcoded return .treatmentA in OnboardingIntroViewModel |
Removed in later commit on #4544 | ✅ Fixed (Bugbot commit is outdated) |
shouldDisplay always returns true in OnboardingSubscriptionPromotionHelper |
Reverted in commit f8d5ba2 on #4544 |
✅ Fixed |
Missing pixel definitions in PixelEvent.swift |
Added in commit ec42fd8 on #4544 |
✅ Fixed |
isChatPathSubscriptionPromo commented out, variable unused in RebrandedNewTabDaxDialogFactory |
Removed in commit e682475 on #4544 |
✅ Fixed |
State: All four fixed in code. All four GitHub threads are NOT resolved (outdated Bugbot comments never closed). → Resolve all four threads.
✅ [Resolved] PR #4544 — Bugbot: Chat-path completion dialog shown twice via competing paths
Source: Bugbot on PR #4544, MainViewController.swift:4711
Issue: Two independent paths both call presentChatPathOnboardingCompletionIfNeeded() — tabDidRequestNewTab dispatch and a direct call — potentially showing the completion dialog twice.
State: presentChatPathOnboardingCompletionIfNeeded() has a guard on chatPathPhase == .trackerToEOJ — once the EOJ fires, phase advances and the second call is a no-op. GitHub thread is [Resolved].
✅ [Resolved] PR #4544 — Bugbots + Alessandro (additional resolved threads)
All resolved on GitHub. Fixed in later commits on the #4544 branch.
| Thread | Fix |
|---|---|
DefaultDaxDialogsSettings.chatPathPhase missing isChatFirstPath guard |
Added in later commit |
Chat-path tests miss isChatFirstPath setup |
Fixed in tests |
Debug reset omits clearing isChatFirstPath flag |
Fixed in OnboardingDebugView |
Duplicated chatPathPhase logic across DaxDialogs + ContextualDaxDialogsFactory |
Consolidated into DaxDialogsSettings |
Alessandro: pixels @rachelmcr fyi (PixelEvent.swift:268) |
Informational; no action required |
Alessandro: tests replicating real DaxDialogsSettings logic |
Simplified tests |
TrackerToEOJ phase test also needs browsing dialog flag |
Fixed in tests |
Missing fallback when AI Chat disabled during chat path (MainViewController.swift:4712) |
Fallback to .final added |
isChatPath logic in createFinalDialog is unreachable |
Dead branching removed |
🔴 PR #4544 — Alessandro (DaxDialogs.swift:125): DaxDialogProvider refactoring suggestion
Source: Alessandro May 7 review on PR #4544, DaxDialogs.swift:125
Issue: Alessandro asked whether having a DaxDialogProvider that internally delegates to either DaxDialogs or a new DuckAIDaxDialogs would be a significant change — to separate responsibilities since the chat-path flow doesn't need search dialogs.
State: 🔴 [OPEN] on GitHub. Alessandro's own follow-up (May 10): "No time for this, let's re-assess once the experiment is live." → Deferred to post-experiment follow-up.
🟠 PR #4544 — Alessandro (ContextualOnboardingNewTabDialogFactoryTests.swift:172): Tests are for the legacy factory
Source: Alessandro May 7 review on PR #4544, ContextualOnboardingNewTabDialogFactoryTests.swift:172
Issue: "Just bear in mind that these tests are for the legacy factory." Tests target NewTabDaxDialogFactory, not RebrandedNewTabDaxDialogFactory (active under rebranding flag).
State: 🟠 Partially addressed — createFinalDialog chat-path branching removed (both factories now symmetric), so the asymmetry this comment highlighted is gone. createSubsequentDialog chat-path logic still only tested via legacy factory. GitHub thread still open.
✅ PR #4544 — Bugbot (r3234446893): Dead isChatPath branch in NewTabDaxDialogFactory.createFinalDialog
Source: Bugbot on PR #4544, NewTabDaxDialogFactory.swift:192 (r3234446893)
Issue: NewTabDaxDialogFactory.createFinalDialog had chat-path conditional logic (chatPathPhase == .trackerToEOJ && isAIChatEnabled), but RebrandedNewTabDaxDialogFactory.createFinalDialog did not. .final is unreachable for chat-path + AI-enabled users (EOJ driven by presentChatPathOnboardingCompletionIfNeeded), so both branches were dead.
Fix: Removed isChatPath conditional from NewTabDaxDialogFactory.createFinalDialog — both factories now always use standard EOJ message + daxDialogsEndOfJourneyNewTabUnique pixel. Deleted test testWhenChatPathEOJState_AndFinalDialogAppears_ThenFiresDuckAIExperimentFinalDialogPixel (tested dead code); simplified remaining final-dialog tests.
State: ✅ Fixed and committed (2026-05-18). GitHub thread resolved on GitHub.
✅ PR #4544 — Bugbot (r3234645543) / SR GJ6b/: Chat-path EOJ not shown after tapping tracker dialog CTA
Source: Bugbot comment r3234645543 on PR #4544, TabViewController.swift:4214. Also reported as SR GJ6b/ by Gary Apr 29 — same bug.
Issue: When user taps "Got it" on the tracker-blocked dialog, the code goes through didTapDismissContextualOnboardingAction, which does not call tabDidRequestNewTab. The EOJ new tab is never opened.
Fix: if contextualOnboardingLogic.chatPathPhase == .trackerToEOJ { delegate?.tabDidRequestNewTab(self) } in didTapDismissContextualOnboardingAction (TabViewController.swift:4206).
State: ✅ Fix landed via PR #4584 merge. GitHub thread resolved on GitHub.
✅ PR #4544 — Bugbot: Rebranded factory's final dialog missing chat-path branching (RebrandedNewTabDaxDialogFactory.swift:202)
Source: Bugbot on PR #4544, RebrandedNewTabDaxDialogFactory.swift:202
Issue: NewTabDaxDialogFactory.createFinalDialog had chat-path branching, RebrandedNewTabDaxDialogFactory.createFinalDialog did not. Both factories are selected by the onboardingRebranding feature flag.
Fix: Resolved by removing the dead branch from NewTabDaxDialogFactory (see r3234446893 entry above). Both factories now symmetric.
State: ✅ Fixed and committed (2026-05-18). GitHub thread resolved on GitHub.
✅ PR #4544 — Bugbot (r3257196584): Chat-path subscription promo unreachable after EOJ dismissal
Source: Bugbot r3257196584 on PR #4544, NewTabPageViewController.swift:355-374 and DaxDialogs.swift:407-419.
Issue: In finishDismissal (inside showDuckAIOnboardingCompletionDialog), nextHomeScreenMessageNew() was called while browsingFinalDialogShown was still false. peekNextHomeScreenMessageExperiment() took the chat-path branch (not the finalDaxDialogSeen branch) and returned nil. The else branch then called daxDialogsManager.dismiss() which atomically set browsingFinalDialogShown = true and isDismissed = true, making isEnabled = false. Any subsequent call returned nil unconditionally — subscription promo was unreachable.
Fix: Call setFinalOnboardingDialogSeen() before nextHomeScreenMessageNew() in finishDismissal. Added setFinalOnboardingDialogSeen() to NewTabDialogSpecProvider protocol; added stub to MockNewTabDialogSpecProvider.
State: ✅ Fixed and committed (2026-05-18). GitHub thread still open — needs resolving.
✅ PR #4544 — Bugbot (r3257070868): Unused UserDefaultsPropertyWrapper.Key cases
Source: Bugbot comment r3257070868 on PR #4544, UserDefaultsPropertyWrapper.swift:59-61
Issue: daxChatPathVisitSiteSeen and daxIsChatFirstPath added to the Key enum but never used — DefaultDaxDialogsSettings stores these via KeyValueStoring with its own ChatPathKey constants.
Fix: Removed both dead enum cases from UserDefaultsPropertyWrapper.Key.
State: ✅ Fixed in alex/demo-tracker-blocking-onboarding working tree (2026-05-18). Not yet committed.
PR #4591 Comments
PR #4591 (
demo-tracker-blocking-onboarding-ui-polish) — Merged ✅ 2026-05-15. Threads documented for reference.
✅ PR #4591 — Bugbot (SyncSettingsViewController.swift:738): Experiment pixel skips .connect receiver
Source: Bugbot on PR #4591, SyncSettingsViewController.swift:738
State: ✅ Acknowledged upstream issue (PR #4575). Not in scope. PR merged — thread irrelevant.
✅ [Resolved] PR #4591 — Bugbot (SyncSettingsViewController.swift:567): Missing pixel when shouldShowSyncEnabled
Source: Bugbot on PR #4591, SyncSettingsViewController.swift:567
State: ✅ [Resolved] on GitHub.
✅ PR #4591 — Bugbot (SyncSettingsViewController.swift:588): Missing experiment success pixel in controllerDidCreateSyncAccount
Source: Bugbot on PR #4591, SyncSettingsViewController.swift:588
State: ✅ Acknowledged upstream issue (PR #4575). Not in scope. PR merged — thread irrelevant.
✅ PR #4591 — Bugbot (MainViewController.swift): Removed performCancel may leave omnibar in editing state
Source: Bugbot on PR #4591, MainViewController.swift
State: ✅ Acknowledged upstream issue (PR #4624). Not in scope. PR merged — thread irrelevant.
✅ PR #4591 — Bugbot (HistoryCleaner.swift:74): nil-coalescing
Source: Bugbot on PR #4591, HistoryCleaner.swift:74
State: ✅ Acknowledged upstream issue (PR #4625). Not in scope. PR merged — thread irrelevant.
✅ [Resolved] PR #4591 — Bugbot (NewTabPageViewController.swift): Duplicated editing-state embedding constraint logic
Source: Bugbot on PR #4591, NewTabPageViewController.swift
State: ✅ [Resolved] — embedDialogInEditingState removed in sr-feedback.
✅ PR #4591 — Alessandro (RebrandedNewTabDaxDialogFactory.swift): Closure sent via notification
Source: Alessandro review on PR #4591, RebrandedNewTabDaxDialogFactory.swift
State: ✅ NIP — Alessandro acknowledged refactor not required. PR merged.
✅ PR #4591 — Alessandro (MainViewController.swift): as? (() -> Void) runtime cast
Source: Alessandro review on PR #4591, MainViewController.swift
State: ✅ Fixed in sr-feedback (typed SettingsDeepLinkCallback struct, commit 482056566b). Lands with PR #4855. PR #4591 merged.
🟡 PR #4591 — Alessandro (NewTabPageViewController.swift): GJ3 — hide address bar
Source: Alessandro review on PR #4591, NewTabPageViewController.swift
Issue: Alessandro flagged the GJ3 requirement to hide the address bar for the visit-site step.
State: 🟡 [OPEN] on GitHub thread. Fix implemented in sr-feedback branch (PR #4855) via setBarsHidden. Will land when stack merges.
✅ [Resolved] PR #4591 — Bugbot (RebrandedNewTabDaxDialogFactory.swift): onPresented passes false but sheet was shown
Source: Bugbot on PR #4591, RebrandedNewTabDaxDialogFactory.swift
Issue (Bugbot): onPresented closure calls onDismiss(false) despite the subscription sheet having been shown.
Why it's a false positive: The Bool parameter is activateSearch, not a "was shown" flag. false = "don't activate the search bar after dismissal" (user is heading to the subscription flow). true = "activate search" (user tapped Skip). The legacy NewTabDaxDialogFactory does the same in its proceed path. The onPresented deferral is intentional — avoids an NTP flash by keeping the promo visible until the Settings sheet fully covers it.
Fix: Added inline comment // activateSearch: false — user is navigating to subscription, not returning to search. to make the parameter intent explicit.
State: ✅ False positive. Comment added to suppress future confusion.
✅ [Resolved] PR #4591 — Bugbots (additional resolved threads)
| Thread | State |
|---|---|
Verbose multi-line comments explain standard patterns (NewTabPageViewController.swift) |
[Resolved] |
Missing ?? parent fallback when finding editing controller (NewTabPageViewController.swift) |
[Resolved] — editing state approach removed |
Private method embedDialogInEditingState is never called (NewTabPageViewController.swift) |
[Resolved] — method removed in sr-feedback |
PR #4664 Comments
Threads in GitHub order.
✅ [Resolved] PR #4664 — Bugbot: NTP dialog title changed for all paths, not just chat-path
Source: Bugbot + Alessandro review on PR #4664, RebrandedNewTabDaxDialogFactory.swift
Fix: isChatPath check added — title now conditional on chatPathPhase == .visitSite.
State: ✅ [Resolved] on GitHub. Fixed in PR #4664.
✅ [Resolved] PR #4664 — Bugbot (RebrandedContextualOnboardingDialogs+SubscriptionPromo.swift:101): Dismiss button appears on chat path
Source: Bugbot on PR #4664, RebrandedContextualOnboardingDialogs+SubscriptionPromo.swift:101
Issue: X dismiss button rendered unconditionally on subscription promo dialog; should be hidden on chat path (user should use the dedicated CTA, not escape via X).
Fix: onManualDismiss is (() -> Void)? in OnboardingSubscriptionPromoDialog (line 40, comment: "When nil the X dismiss button is hidden (e.g. chat-path onboarding)"). RebrandedNewTabDaxDialogFactory passes nil when isChatPath. Already in code — Bugbot is outdated.
State: ✅ Already fixed. GitHub thread status unknown.
PR #4668 Comments
Threads in GitHub order.
⭕ PR #4668 — Bugbot (OnboardingIntroViewModel.swift:473): Hardcoded debug return
Source: Bugbot on PR #4668, OnboardingIntroViewModel.swift:473
Issue: Hardcoded return .treatmentA bypasses feature flag in resolveDuckAIQueryExperimentCohortID().
State: ⭕ Removed and pushed on #4668 branch. GitHub thread still open — needs resolving.
⭕ PR #4668 — Bugbot (FeatureFlag.swift:635): Commented-out Config(...) line
Source: Bugbot on PR #4668, FeatureFlag.swift:635
Issue: defaultValue: .enabled override was added to unifiedToggleInput for testing, plus the old // Config(...) line still sits underneath as dead commented-out code. Both need to be removed before merging.
State: ⭕ TODO — still in code on #4668 branch. Deferred until main PR (#4544) is done.
🔴 PR #4668 — Bugbot (MainViewController.swift:3457): Protocol method embedInUnifiedInputEditingAreaIfActive never called
Source: Bugbot on PR #4668, MainViewController.swift:3457
Issue: Method added to BrowserChromeDelegate protocol and implemented, but no caller exists.
State: 🔴 [OPEN] on GitHub. Deferred until #4544 is done — see BrowserChromeManager item below for full detail.
⭕ PR #4668 — aataraxiaa (FeatureFlag.swift:635): Commented-out config line
Source: aataraxiaa review comment on PR #4668, FeatureFlag.swift:635
Issue: Same commented-out Config(...) dead code flagged by Bugbot above. aataraxiaa also requests it be removed.
State: ⭕ TODO — GitHub thread [OPEN]. Deferred until #4544 is done.
🔴 PR #4668 — Bugbot (BrowserChromeManager.swift:43) r3234513817: New protocol method no callers
Source: Bugbot on PR #4668, BrowserChromeManager.swift:43
Issue: embedInUnifiedInputEditingAreaIfActive added to BrowserChromeDelegate protocol and implemented, but no caller exists. The NTP's embedDialogInEditingState and showDuckAIOnboardingCompletionDialogInUTI both embed by casting parent as? MainViewController directly instead of going through this protocol method. Additionally the implementations differ: the protocol method pins to navigationBarContainer.bottomAnchor, while actual NTP embedding pins to coordinator.viewController.view.bottomAnchor. So simply adding a caller is not enough — implementations must be aligned first.
State: 🔴 [OPEN] on GitHub. Deferred until #4544 is done. Fix = align implementations + wire caller, or remove the protocol method entirely.
PR #4855 Comments
Threads in GitHub order.
🟠 [Resolved] PR #4855 — Bugbot: Nav bar not restored when NTP dismissed during visit-site dialog
Source: Bugbot on PR #4855, NewTabPageViewController.swift:281
Fix: dismiss() now checks didHideBarsForChatPathVisitSiteDialog and calls setBarsHidden(false) before removing the view. Previously only dismissHostingController() restored the bars, but dismiss() (called when the NTP is removed via removeHomeScreen()) did not.
Note on animation: dismiss() uses animated: false while dismissHostingController() uses animated: true. This is intentional — dismiss() is the teardown path (e.g. user switches tabs), where the NTP view is being removed anyway, so animating bars in is pointless. dismissHostingController() is the success path (user picks a site), where the bars slide in as part of the navigation transition.
State: sr-feedback / uncommitted — fix is in working tree, NOT committed. GitHub thread NOT resolved.
🟠 PR #4855 — Bugbot: Guard early return aborts entire dialog presentation (NewTabPageViewController.swift:473)
Source: Bugbot on PR #4855, NewTabPageViewController.swift:473
Issue: guard (parent as? MainViewController)?.currentTab?.isLoading != true else { return } was a standalone guard that aborted the whole function. When the tab was loading, the guard returned early before any dialog or bar logic ran, leaving an orphaned hostingController and a blank NTP.
Fix: Removed the loading check entirely. The standalone guard ((parent as? MainViewController)?.currentTab?.isLoading) was deleted — no delegate method was introduced. The if block now only checks spec == .subsequent and chatPathPhase == .visitSite. Dialog always presents; no loading condition gates it.
State: sr-feedback / uncommitted — fix is in working tree, NOT committed. GitHub thread NOT resolved.
🟠 PR #4855 — Bugbot: Hardcoded return .treatmentB bypasses feature flag in OnboardingIntroViewModel
Source: Bugbot on PR #4855, OnboardingIntroViewModel.swift:473
Fix: The original return .treatmentB Bugbot flagged was already removed in an earlier commit. The override was subsequently restored to return .treatmentA (with // TODO: Remove this before shipping) for development testing purposes. The GitHub thread predates that restore and refers to .treatmentB; the .treatmentA override is intentional and tracked under PRE-SHIP-2 below.
State: sr-feedback / uncommitted — override present in working tree, NOT committed. GitHub thread NOT resolved (refers to outdated .treatmentB).
🟠 PR #4855 — Bugbot: Returning-user guard commented out in insertExperimentStepIfNeeded
Source: Bugbot on PR #4855, OnboardingIntroViewModel.swift:451. Also: Alessandro May 7 review on #4544, Asana task 1214682678629767.
Fix: Uncommented guard case .introDialog(isReturningUser: false) = introSteps.first in insertExperimentStepIfNeeded(). Returning users (including sync-restore users) are now excluded from the Duck.ai query experiment step. The isReturningUser value comes from OnboardingManager.isNewUser (line 250 in OnboardingIntroViewModel: introDialog(isReturningUser: !isNewUser)).
State: sr-feedback / uncommitted — fix is in working tree, NOT committed. GitHub thread NOT resolved.
🟠 PR #4855 — Alessandro (line 280): Use NewTabPageControllerDelegate instead of (parent as? MainViewController) cast
Source: #4855 review comment, line 280
Fix: The loading check was the only reason for the (parent as? MainViewController) cast, so the cast was removed by eliminating the loading check entirely (see "Guard early return" item above). No delegate method was added. Zero parent casts remain in NewTabPageViewController.
State: sr-feedback / uncommitted — fix is in working tree, NOT committed. GitHub thread NOT resolved.
🟠 PR #4855 — Alessandro (line 477): Hide both bars via setBarsHidden (Costas preference)
Source: #4855 review comment, line 477
Fix: Replaced chromeDelegate?.setNavigationBarHidden(true/false) + (parent as? MainViewController)?.setChatPathVisitSiteControlsLocked(true/false) with chromeDelegate?.setBarsHidden(true/false, animated: false, customAnimationDuration: nil) in NewTabPageViewController — both the show path and dismiss() / dismissHostingController(). This removes all (parent as?) casts from NewTabPageViewController.
Why setChatPathVisitSiteControlsLocked calls were removed: Alessandro's review requested setBarsHidden via chromeDelegate (no parent cast). setBarsHidden hides both address bar and toolbar in one call, making setChatPathVisitSiteControlsLocked unreachable. The method definition still sits in MainViewController+DuckAIExperiment.swift:166 as dead code — see DEAD-1 below.
State: sr-feedback / uncommitted — fix is in working tree, NOT committed. GitHub thread NOT resolved.
🟠 [Resolved] PR #4855 — Alessandro (UserText.swift:2375): Revert searchAndDuckAIOption copy
Source: #4855 review comment, UserText.swift:2375
Fix: Reverted searchAndDuckAIOption back to "Toggle between\nSearch and Duck.ai" in UserText.swift and all 26 lproj files. (Was incorrectly changed to "Ask AI" — that rename applies only to the DuckAI query toggle label, not the Search Experience screen.)
State: GitHub thread [Resolved]. Fix is in working tree, NOT committed.
✅ PR #4855 — Alessandro (line 450): Add !restorePromptHandler.isEligibleForRestorePrompt() to the returning-user guard
Source: Alessandro May 15 review on PR #4855, OnboardingIntroViewModel.swift:450. Also referenced in Alessandro's May 15 Asana comment.
Fix: !restorePromptHandler.isEligibleForRestorePrompt() is the second condition in the guard at OnboardingIntroViewModel.swift:451.
State: ✅ Committed in alex/demo-tracker-blocking-onboarding via PR #4584 merge (commit 4e626d3bdc, 2026-05-18). GitHub thread on PR #4855 still open — needs resolving.
🟠 PR #4855 — Legacy factory: dismiss button shown unconditionally on visit-site dialog
Source: Observed during SR feedback pass. Parity issue with RebrandedNewTabDaxDialogFactory, which already suppresses the X button on chat path.
Issue: NewTabDaxDialogFactory.createSubsequentDialog always passed manualDismissAction as a non-optional closure to OnboardingTryVisitingSiteDialog, so DaxDialogView always rendered the X dismiss button — including on the chat path where the user must pick a site and should not be able to escape.
Fix: manualDismissAction changed to (() -> Void)? = isChatPath ? nil : { … }. OnboardingTryVisitingSiteDialog.onManualDismiss made (() -> Void)? so DaxDialogView's ifLet(onManualDismiss) skips the button when nil. ContextualDaxDialogsFactory (in-browser overlay, always correct to show dismiss) unaffected.
State: 🟠 Fix in working tree, NOT committed.
✅ DEAD-1 — setChatPathVisitSiteControlsLocked in MainViewController+DuckAIExperiment.swift deleted
Source: Callers removed as part of PR #4855 Alessandro (line 477) fix above.
State: Method deleted. Not committed (tracked in MainViewController+DuckAIExperiment.swift working-tree change).
✅ GJ8 / Copy: "Ask.ai" confirmed (with dot)
Source: GJ8 said "Ask AI". Alessandro's May 14 Asana comment explicitly says "Ask.ai" (with dot) — resolves the conflict.
State: ✅ Implemented — UserText.DuckAIQueryExperiment.toggleAILabel is "Ask.ai" (with dot) in working tree. Not committed.
✅ PRE-SHIP-1 — Privacy config: add US/EN targeting
Source: Task 1214682678629768. Without targets, experiment enrolls all locales worldwide.
What: In privacy-configuration/overrides/ios-override.json, add "targets": [{ "localeLanguage": "en", "localeCountry": "US" }] to the new onboardingDuckAIQueryTrackersDemoExperiment entry.
State: ✅ Done — targets included in the new experiment entry in privacy-config PR. Done together with PRE-SHIP-2.
✅ PRE-SHIP-2 — Privacy config: rename the experiment
Source: Task 1214682678629768. Internal testers already enrolled under onboardingDuckAIQueryExperiment; reusing it would complete NA analysis immediately (already at sample size).
Name decided: Alessandro May 15: use onboardingDuckAIQueryTrackersDemoExperiment as the new AIChatSubfeature case.
What (three parts):
- BSK — new case
onboardingDuckAIQueryTrackersDemoExperimentinAIChatSubfeatureenum ✅ - App — new
FeatureFlag.onboardingDuckAIQueryTrackersDemoExperimentadded; all flag references (MainViewController+DuckAIExperiment.swift,MainViewController.swift,OnboardingIntroViewModel.swift,OnboardingPixelReporter.swift, tests) updated to new name ✅ privacy-configurationrepo — new entryonboardingDuckAIQueryTrackersDemoExperimentwithtargetsadded; oldonboardingDuckAIQueryExperimentset to"state": "disabled"✅
State: ✅ Fully implemented. Privacy-config PR merged. App code updated on alex/demo-tracker-blocking-onboarding.
🔴 PRE-SHIP-3 — Tell UTI team the final experiment name
Source: Pete Apr 30 comment. Pete's team needs the name to exclude enrolled users from the UTI feature rollout.
What: Fill onboardingDuckAIQueryTrackersDemoExperiment into O-J <> O-N Coordination and O-N Live Onboarding Experiment Details.
State: 🔴 Name now known and finalized — needs filling into both Asana tasks.
Status Summary (mirrors deleted Asana comment, updated to current state)
These were the TBD items called out in the ship review. Status reflects current state as of May 14.
GJ6c/ ✅ No longer required. The address bar will be hidden for the "Try visiting a site" step (GJ3 / setBarsHidden). Costas confirmed a search performed after the trackers-blocked dialog is acceptable, so no additional guard is needed here.
GJ7/ Sync-restore exclusion 🟠 Implemented, not yet committed. guard case .introDialog(isReturningUser: false) = introSteps.first is now active in insertExperimentStepIfNeeded(). Users who selected "Restore My Stuff" (sync restore) will not be enrolled in the experiment. Working tree only — needs committing to sr-feedback.
GJ3/ Hide address bar + toolbar 🟠 Implemented via chromeDelegate?.setBarsHidden(true/false), not yet committed. Replaces earlier setNavigationBarHidden + setChatPathVisitSiteControlsLocked approach.
GJ8/ "Ask.ai" toggle label 🟠 Copy confirmed as "Ask.ai" (with dot) by Alessandro May 14. Currently implemented as "Ask AI" (no dot) in working tree — needs correction to "Ask.ai" before committing.
Experiment rename in privacy config ✅ Done. onboardingDuckAIQueryTrackersDemoExperiment implemented in BSK, app code updated, privacy-config PR merged with new entry + old disabled.
Privacy config targets payload ✅ Done. "targets": [{ "localeLanguage": "en", "localeCountry": "US" }] added to onboardingDuckAIQueryTrackersDemoExperiment in privacy-config PR.
O-N Live Onboarding Experiment Details (task 1214601039604921) 🔴 Still needs filling: add onboardingDuckAIQueryTrackersDemoExperiment into O-N Live Onboarding Experiment Details and O-J <> O-N Coordination.
Alessandro's May 15 QA — Tested PR #4855 against test cases in task 1214683268207880. Test run: task 1214794564106677. Left notes in test run and new comments in PR (see PR #4855 — Alessandro line 450 item above).
Merge approved PRs — Alessandro recommends merging #4591 and #4664 (both approved) into the main feature branch (#4544) to avoid propagating main-branch merges through each PR separately.
Experiment Config (current, privacy-config PR merged)
privacy-configuration/overrides/ios-override.json under aiChat subfeatures:
"onboardingDuckAIQueryExperiment": {
"state": "disabled"
},
"onboardingDuckAIQueryTrackersDemoExperiment": {
"state": "enabled",
"targets": [{ "localeLanguage": "en", "localeCountry": "US" }],
"cohorts": [
{ "name": "control", "weight": 1 },
{ "name": "treatmentA", "weight": 1 }
]
}
Feature flag:
FeatureFlag.onboardingDuckAIQueryTrackersDemoExperimentiniOS/Core/FeatureFlag.swift
Cohort type:FeatureFlag.DuckAIQueryExperimentCohort