38 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) |
| #4591 | demo-tracker-blocking-onboarding-ui-polish |
Approved |
| #4664 | demo-tracker-blocking-onboarding-chat-path-dialog-polish |
Approved |
| #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.
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 — 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."
This is the same bug as PR #4544 — Bugbot r3234645543 below. Fix is in didTapDismissContextualOnboardingAction on sr-feedback branch, not yet on #4544 branch.
State: 🔴 Fix exists in PR #4855 (sr-feedback), NOT in PR #4544. GitHub thread on #4544 NOT resolved.
🔴 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 added under // MARK: - Chat Path – Subsequent Dialog target NewTabDaxDialogFactory (legacy), not RebrandedNewTabDaxDialogFactory (active under the rebranding flag).
State: 🔴 [OPEN] on GitHub. Rebranded factory coverage may be insufficient.
🔴 PR #4544 — Bugbot: Dead property pendingCompletionDialogMessage never set non-nil (NewTabDaxDialogFactory.swift:192)
Source: Bugbot on PR #4544, NewTabDaxDialogFactory.swift:192
Issue: NewTabDaxDialogFactory.createFinalDialog was updated with chat-path conditional logic (chatPathPhase == .trackerToEOJ && isAIChatEnabled choosing a different message + pixel), but RebrandedNewTabDaxDialogFactory.createFinalDialog was not updated to match — despite its section mark being renamed to "Chat-Path Completion". The .final spec is currently unreachable for chat-path + AI-enabled users (the chat EOJ is driven by presentChatPathOnboardingCompletionIfNeeded instead), so pendingCompletionDialogMessage is always nil.
State: 🔴 [OPEN] on GitHub. The dead branching in NewTabDaxDialogFactory was removed in a later commit; verify RebrandedNewTabDaxDialogFactory is consistent.
🟡 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: Added if contextualOnboardingLogic.chatPathPhase == .trackerToEOJ { delegate?.tabDidRequestNewTab(self) } to didTapDismissContextualOnboardingAction. (Same block was already in didNavigateAwayFromContextualOnboardingDialog.)
State: Fix exists in commit 6c3774e4f8 on sr-feedback branch (PR #4855). NOT present on the #4544 branch itself. Fix will land in main when the full PR stack merges. GitHub thread on PR #4544 is NOT resolved.
🔴 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 has chat-path branching (chatPathPhase == .trackerToEOJ && isAIChatEnabled), but RebrandedNewTabDaxDialogFactory.createFinalDialog does not — both are selected by the onboardingRebranding feature flag. While .final is currently unreachable for chat-path + AI-enabled users, divergence could cause the wrong message + pixel if the flow changes.
State: 🔴 [OPEN] on GitHub. Needs symmetry check between the two factories.
PR #4591 Comments
PR #4591 (
demo-tracker-blocking-onboarding-ui-polish) is Approved. Open threads still need resolving before merge. Threads in GitHub order.
🔴 PR #4591 — Bugbot (SyncSettingsViewController.swift:738): Experiment pixel skips .connect receiver
Source: Bugbot on PR #4591, SyncSettingsViewController.swift:738
Issue: The experiment success pixel fires for .recovery sync path but skips .connect. mallexxx acknowledged: pixel originates in upstream PR #4575, not this PR.
State: 🔴 [OPEN] on GitHub. Acknowledged as upstream issue.
✅ [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: 🔴 [OPEN] on GitHub. Needs verification / fix.
🔴 PR #4591 — Bugbot (MainViewController.swift): Removed performCancel may leave omnibar in editing state
Source: Bugbot on PR #4591, MainViewController.swift
Issue: Removing performCancel call may leave the omnibar stuck in editing state in certain paths.
State: 🔴 [OPEN] on GitHub. Needs verification.
🔴 PR #4591 — Bugbot (HistoryCleaner.swift:74): nil-coalescing
Source: Bugbot on PR #4591, HistoryCleaner.swift:74
State: 🔴 [OPEN] on GitHub. Needs review.
✅ [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
Issue: "Not a big fan of sending this closure in a notification, but I guess this would require some refactoring to change it." Settings deep-link callback dispatched via NotificationCenter with a closure payload.
State: 🔴 [OPEN] on GitHub. Alessandro acknowledged it's a refactor; no fix required to merge.
🔴 PR #4591 — Alessandro (MainViewController.swift): as? (() -> Void) runtime cast
Source: Alessandro review on PR #4591, MainViewController.swift
Issue: as? (() -> Void) cast is only checked at runtime — if the factory's closure signature ever changes, it will silently fail.
State: 🔴 [OPEN] on GitHub. Fixed in commit 482056566b on sr-feedback (wrapped in a typed SettingsDeepLinkCallback struct). Fix is NOT on #4591 branch; will land when stack merges.
🟡 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: A hardcoded return .treatmentB debug shortcut bypasses the feature flag in resolveDuckAIQueryExperimentCohortID(). Must be removed before shipping.
State: 🔴 [OPEN] on GitHub. Same item appears on PR #4855 (Bugbot line 473). Not yet removed on #4668 branch.
🔴 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: 🔴 Still in code on the #4668 branch. NOT fixed. GitHub thread NOT resolved.
🔴 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. See also BrowserChromeManager item below — same root issue.
🔴 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: 🔴 [OPEN] on GitHub. Not yet fixed.
🔴 PR #4668 — Bugbot (BrowserChromeManager.swift:43): New protocol method no callers
Source: Bugbot on PR #4668, BrowserChromeManager.swift:43
Issue: embedInUnifiedInputEditingAreaIfActive added to BrowserChromeDelegate protocol and implemented in BrowserChromeManager, but no caller exists anywhere in the codebase.
State: On sr-feedback branch commit 82bb0f71e3 removed embedDialogInEditingState dead code; the protocol method may now be entirely unreachable. Needs verification on #4668 branch. GitHub thread NOT resolved. → Either add a caller or remove the method and its implementation.
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.
Issue: The current guard guard case .introDialog(isReturningUser: false) = introSteps.first excludes returning users and those eligible to restore data during onboarding. But if the experiment is later remotely enrolled for all users (e.g. post-experiment analysis phase), returning users who are eligible for restore will bypass the guard and still see the Duck.ai experiment step.
Fix: Add !restorePromptHandler.isEligibleForRestorePrompt() as an additional condition to the guard.
State: ✅ Implemented — !restorePromptHandler.isEligibleForRestorePrompt() is the second condition in the guard at OnboardingIntroViewModel.swift:451. Fix is in working tree, NOT committed. GitHub thread NOT resolved.
🟠 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 onboardingDuckAIQueryExperiment entry.
State: Local edit exists on wrong branch (alex/remove-autocomplete-tabs) in privacy-configuration repo, not committed. Should be done together with PRE-SHIP-2 in one PR to that repo.
🔴 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 — update
FeatureFlag.onboardingDuckAIQueryExperimentto reference the new subfeature case; update experiment pixel names accordingly privacy-configurationrepo — add new entryonboardingDuckAIQueryTrackersDemoExperimentwithtargets; set oldonboardingDuckAIQueryExperimentto"state": "disabled"
State: 🔴 Name decided, NOT yet implemented.
🔴 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 — can be filled in once PRE-SHIP-2 is done.
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 🔴 Name decided: onboardingDuckAIQueryTrackersDemoExperiment (Alessandro May 15). Reason: reusing the old name would complete NA analysis immediately (already at sample size). Still needs implementing: new AIChatSubfeature case in BSK, update FeatureFlag, new entry in privacy-configuration/overrides/ios-override.json (disable old one).
Privacy config targets payload 🔴 TBD. "targets": [{ "localeLanguage": "en", "localeCountry": "US" }] must be added to the experiment entry. Local edit exists on wrong branch in privacy-configuration repo. Do together with rename above.
O-N Live Onboarding Experiment Details (task 1214601039604921) 🔴 Name now known (onboardingDuckAIQueryTrackersDemoExperiment). Fill into O-N Live Onboarding Experiment Details and O-J <> O-N Coordination once PRE-SHIP-2 is implemented.
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, local override only)
privacy-configuration/overrides/ios-override.json under aiChat subfeatures:
"onboardingDuckAIQueryExperiment": {
"state": "enabled",
"targets": [{ "localeLanguage": "en", "localeCountry": "US" }],
"cohorts": [
{ "name": "control", "weight": 1 },
{ "name": "treatmentA", "weight": 1 },
{ "name": "treatmentB", "weight": 1 }
]
}
Feature flag:
FeatureFlag.onboardingDuckAIQueryExperimentiniOS/Core/FeatureFlag.swift
Cohort type:FeatureFlag.DuckAIQueryExperimentCohort