Failed to authenticate. API Error: 401 Invalid authentication credentials
[2026-05-14] vault sync
This commit is contained in:
Alexey Martemyanov
@@ -8,220 +8,346 @@
|
||||
|
||||
## PR Stack
|
||||
|
||||
| PR | Branch | Base | Review status |
|
||||
|---|---|---|---|
|
||||
| [#4544](https://github.com/duckduckgo/apple-browsers/pull/4544) | `demo-tracker-blocking-onboarding` | `main` | Open — review required (Alessandro + Rachel left comments) |
|
||||
| [#4591](https://github.com/duckduckgo/apple-browsers/pull/4591) | `demo-tracker-blocking-onboarding-ui-polish` | #4544 | Approved |
|
||||
| [#4664](https://github.com/duckduckgo/apple-browsers/pull/4664) | `demo-tracker-blocking-onboarding-chat-path-dialog-polish` | #4591 | Approved |
|
||||
| [#4855](https://github.com/duckduckgo/apple-browsers/pull/4855) | `demo-tracker-blocking-onboarding-sr-feedback` | #4664 | Open — no review yet. Has uncommitted local changes (see below). |
|
||||
| [#4668](https://github.com/duckduckgo/apple-browsers/pull/4668) | `demo-tracker-blocking-onboarding-uti-flow` | ? | Open — no review yet. Has open Bugbot + reviewer comments (see below). |
|
||||
|
||||
---
|
||||
|
||||
## Uncommitted Local Changes — PR #4855 branch (`sr-feedback`)
|
||||
|
||||
All of the following are in the working tree but **NOT committed or pushed**. They must be committed before #4855 can be reviewed or merged.
|
||||
|
||||
### 1. `NewTabPageViewController.swift` — setBarsHidden + delegate pattern + loading guard
|
||||
|
||||
**What changed:**
|
||||
- Replaced `(parent as? MainViewController)?.setChatPathVisitSiteControlsLocked(true/false)` + `chromeDelegate?.setNavigationBarHidden(true/false)` with a single `chromeDelegate?.setBarsHidden(true/false, animated: false, customAnimationDuration: nil)` call.
|
||||
- Removed all `(parent as? MainViewController)?` casts from `NewTabPageViewController`.
|
||||
- Loading guard (`guard ... isLoading != true else { return }`) was a standalone guard that would abort dialog presentation entirely, leaving an orphaned `hostingController` reference. Folded into the outer `if` condition using the new delegate method instead.
|
||||
- `dismiss()` now calls `setBarsHidden(false)` when `didHideBarsForChatPathVisitSiteDialog` is true.
|
||||
|
||||
**Why `setChatPathVisitSiteControlsLocked` calls were removed:**
|
||||
Alessandro's review comment on [#4855 line 280](https://github.com/duckduckgo/apple-browsers/pull/4855) asked to use `NewTabPageControllerDelegate` instead of casting `(parent as? MainViewController)`. A separate review comment on [#4855 line 477](https://github.com/duckduckgo/apple-browsers/pull/4855) noted Costas's preference to hide both address bar and toolbar via `setBarsHidden(false)`. Using `setBarsHidden` via `chromeDelegate` removes the need for the cast entirely, making `setChatPathVisitSiteControlsLocked` unreachable. The method definition still exists in `MainViewController+DuckAIExperiment.swift:166` as dead code — needs to be deleted.
|
||||
|
||||
**Source:** Alessandro review comments on PR #4855 ([line 280](https://github.com/duckduckgo/apple-browsers/pull/4855), [line 473](https://github.com/duckduckgo/apple-browsers/pull/4855), [line 477](https://github.com/duckduckgo/apple-browsers/pull/4855)) + Costas's preference cited therein + Bugbot #4855 "Guard early return aborts entire dialog" + Bugbot #4855 "Navigation bar not restored when NTP dismissed".
|
||||
|
||||
**Current state:** ✏️ Uncommitted local change on `sr-feedback` branch.
|
||||
|
||||
### 2. `NewTabPageControllerDelegate.swift` + `MainViewController.swift` — new delegate method
|
||||
|
||||
**What changed:** Added `newTabPageControllerCurrentTabIsLoading(_ controller: NewTabPageViewController) -> Bool` to the `NewTabPageControllerDelegate` protocol. Implemented in `MainViewController.swift` to return `currentTab?.isLoading == true`.
|
||||
|
||||
**Source:** Alessandro review comment on [#4855 line 473](https://github.com/duckduckgo/apple-browsers/pull/4855): "should we add `(parent as? MainViewController)?.currentTab?.isLoading != true` as condition of the if instead of having the guard..." Also addresses Bugbot #4855 "Guard early return aborts entire dialog presentation" and removes the `(parent as?)` cast as requested on [#4855 line 280](https://github.com/duckduckgo/apple-browsers/pull/4855).
|
||||
|
||||
**Current state:** ✏️ Uncommitted local change on `sr-feedback` branch.
|
||||
|
||||
### 3. `OnboardingIntroViewModel.swift` — returning-user guard + remove hardcoded cohort override
|
||||
|
||||
**What changed:**
|
||||
- Uncommented `guard case .introDialog(isReturningUser: false) = introSteps.first` in `insertExperimentStepIfNeeded()`. This ensures sync-restore / returning users are excluded from the Duck.ai query experiment step.
|
||||
- Removed hardcoded `return .treatmentB` override from `resolveDuckAIQueryExperimentCohortID()` (was left in for local testing).
|
||||
|
||||
**Source (returning-user guard):** [Alessandro May 7 review on PR #4544](https://github.com/duckduckgo/apple-browsers/pull/4544), Asana task [1214682678629767](https://app.asana.com/1/137249556945/task/1214682678629767), Bugbot on #4855 "Returning-user guard commented out despite PR intent". The guard was temporarily commented out during development; it must be active before shipping.
|
||||
|
||||
**Source (hardcoded override removal):** Bugbot on #4855 "Hardcoded debug return bypasses feature flag entirely". This was a local testing override (`return .treatmentB`) that must not ship.
|
||||
|
||||
**Current state:** ✏️ Uncommitted local change on `sr-feedback` branch.
|
||||
|
||||
### 4. `UserText.swift` + all 26 lproj files — `searchAndDuckAIOption` string reverted + "Ask AI" toggle label
|
||||
|
||||
**What changed:**
|
||||
- `searchAndDuckAIOption` reverted to `"Toggle between\nSearch and Duck.ai"` (was changed to "Toggle between\nSearch and Ask AI" in a prior commit).
|
||||
- DuckAIQuery screen toggle label changed from "Duck.ai" to "Ask AI" in `OnboardingView+DuckAIExperimentSearchContent.swift`.
|
||||
|
||||
**Source (revert):** Alessandro review comment on [#4855 UserText.swift:2375](https://github.com/duckduckgo/apple-browsers/pull/4855): *"the rename of 'Duck.ai' to 'Ask AI' is for the toggle we show in the Duck Ai Query screen. The copy for this screen should be reverted."* — meaning the Search Experience screen copy stays as "Duck.ai", only the DuckAI query toggle label becomes "Ask AI".
|
||||
|
||||
**Source (Ask AI label):** [GJ8 ship review](https://app.asana.com/1/137249556945/task/1214147157456478): *"change 'Duck.ai' to 'Ask AI' in the toggle screen in onboarding only."*
|
||||
|
||||
⚠️ **Unresolved copy question:** Alessandro's comment says "Ask.ai" (with dot). Gary's GJ8 says "Ask AI" (no dot). Needs explicit confirmation from product/design before shipping.
|
||||
|
||||
**Current state:** ✏️ Uncommitted local change on `sr-feedback` branch.
|
||||
|
||||
### 5. Dead code: `setChatPathVisitSiteControlsLocked` in `MainViewController+DuckAIExperiment.swift:166`
|
||||
|
||||
**What:** The method has no callers now that `NewTabPageViewController` uses `setBarsHidden` via the chrome delegate instead. Should be deleted.
|
||||
|
||||
**Source:** Removal of callers in item #1 above.
|
||||
|
||||
**Current state:** ✏️ Still in code, needs deletion. Uncommitted.
|
||||
|
||||
---
|
||||
|
||||
## Open GitHub Comment Threads — PR #4544
|
||||
|
||||
These Bugbot threads were on PR #4544 and are **not resolved on GitHub**. Status of each:
|
||||
|
||||
### [Bugbot] Hardcoded return forces all users into treatment group (OnboardingIntroViewModel.swift)
|
||||
**Issue:** `return .treatmentA` hardcoded override in `resolveDuckAIQueryExperimentCohortID()`.
|
||||
**Code state:** The `.treatmentA` override was removed on the `#4544` branch (replaced with `.treatmentB` for testing, then also removed — see uncommitted change #3 above). The `#4544` branch tip no longer has `.treatmentA` hardcoded.
|
||||
**Thread state:** 🔴 **Not resolved on GitHub.** Bugbot comment was on an old commit — outdated. → Resolve thread.
|
||||
|
||||
### [Bugbot] Subscription promo override always returns true (OnboardingSubscriptionPromotionHelper.swift)
|
||||
**Issue:** `shouldDisplay` was hardcoded to `return true` for testing.
|
||||
**Code state:** Reverted in commit [`f8d5ba2`](https://github.com/duckduckgo/apple-browsers/commit/f8d5ba2) on #4544 branch.
|
||||
**Thread state:** 🔴 **Not resolved on GitHub.** Fix is in the branch. → Resolve thread.
|
||||
|
||||
### [Bugbot] Chat-path subscription redirect commented out, variable unused (RebrandedNewTabDaxDialogFactory.swift)
|
||||
**Issue:** `isChatPathSubscriptionPromo` was commented out, variable unused.
|
||||
**Code state:** Dead code removed in commit [`e682475`](https://github.com/duckduckgo/apple-browsers/commit/e682475) on #4544 branch.
|
||||
**Thread state:** 🔴 **Not resolved on GitHub.** Fix is in the branch. → Resolve thread.
|
||||
|
||||
### [Bugbot] Missing pixel definitions (PixelEvent.swift:2120)
|
||||
**Issue:** `onboardingChatPathTryVisitSiteUnique` and `onboardingChatPathTrackersBlockedUnique` missing from `PixelEvent`.
|
||||
**Code state:** Both defined in commit [`ec42fd8`](https://github.com/duckduckgo/apple-browsers/commit/ec42fd8) on #4544 branch (lines 268–269, 2119–2120 of PixelEvent.swift).
|
||||
**Thread state:** 🔴 **Not resolved on GitHub.** Fix is in the branch. → Resolve thread.
|
||||
|
||||
### [Bugbot] Chat-path completion dialog shown twice (MainViewController.swift:4711)
|
||||
**Issue:** Two competing paths (`tabDidRequestNewTab` dispatch + direct call) could both invoke `presentChatPathOnboardingCompletionIfNeeded()`.
|
||||
**Code state:** `presentChatPathOnboardingCompletionIfNeeded()` is guarded by `chatPathPhase == .trackerToEOJ`. Whether the double-call race was fully resolved needs verification.
|
||||
**Thread state:** 🔴 **Not resolved on GitHub.** → Verify code, then resolve thread.
|
||||
|
||||
### [Bugbot r3234645543] Chat-path EOJ not shown after tapping tracker dialog CTA (TabViewController.swift:4214)
|
||||
**Issue:** `didTapDismissContextualOnboardingAction` doesn't call `tabDidRequestNewTab` for chat path, so tapping "Got it" on tracker dialog doesn't open a new tab for the EOJ.
|
||||
**Code state:** 🔴 **NOT fixed on the #4544 branch.** The fix (`if chatPathPhase == .trackerToEOJ { tabDidRequestNewTab }` in `didTapDismissContextualOnboardingAction`) exists only on the `sr-feedback` branch (commit `6c3774e4f8`). It will be present when the full PR stack merges, but it is not in #4544 itself.
|
||||
**Thread state:** 🔴 **Not resolved on GitHub.** Fix is not in this PR — it's in #4855. Note this in the thread or backport the fix to #4544.
|
||||
|
||||
---
|
||||
|
||||
## Open GitHub Comment Threads — PR #4664
|
||||
|
||||
### [Alessandro + Bugbot] Title changes for all paths, not just chat path (RebrandedNewTabDaxDialogFactory.swift)
|
||||
**Issue:** `createSubsequentDialog` title was changed for all flows, not gated on `isChatPath`.
|
||||
**Code state:** Fixed — `isChatPath` check added (`daxDialogsFlowCoordinator.chatPathPhase == .visitSite`). The rebranded factory now uses the chat-path title conditionally.
|
||||
**Thread state:** Likely resolved by the fix but confirm on GitHub. → Check and resolve thread.
|
||||
|
||||
### [Bugbot] Dismiss button appears on standard path too (RebrandedContextualOnboardingDialogs+SubscriptionPromo.swift:101)
|
||||
**Issue:** Dismiss button added for chat path appeared on standard path too.
|
||||
**Code state:** Needs verification on #4664 branch.
|
||||
**Thread state:** 🔴 **Not confirmed resolved.** → Verify code.
|
||||
|
||||
---
|
||||
|
||||
## Open GitHub Comment Threads — PR #4668
|
||||
|
||||
### [aataraxiaa] Commented-out `Config(...)` line in FeatureFlag.swift:635
|
||||
**Issue:** `// Config(source: .remoteReleasable(.subfeature(AIChatSubfeature.unifiedToggleInput)))` — a dead commented-out duplicate still in code.
|
||||
**Source:** [aataraxiaa review comment](https://github.com/duckduckgo/apple-browsers/pull/4668): *"Obviously you just changed this for testing, but just a reminder to change it back before we merge."*
|
||||
**Code state:** 🔴 **Still in code** on `#4668` branch. The `defaultValue: .enabled` override + commented-out original are both present.
|
||||
**Thread state:** 🔴 **Not resolved.** → Remove the commented-out line and the `defaultValue: .enabled` before merging.
|
||||
|
||||
### [Bugbot] Hardcoded debug return bypasses cohort resolution (OnboardingIntroViewModel.swift:473)
|
||||
**Issue:** Same `return .treatmentB` override as above.
|
||||
**Code state:** Removed in the uncommitted local change on `sr-feedback` branch. Will be present once committed and the stack is built. Not yet fixed on #4668 branch itself.
|
||||
**Thread state:** 🔴 **Not resolved.** → Will be resolved when sr-feedback is merged down or the fix is backported.
|
||||
|
||||
### [Bugbot] Protocol method `embedInUnifiedInputEditingAreaIfActive` never called (MainViewController.swift + BrowserChromeManager.swift)
|
||||
**Issue:** Method added to `BrowserChromeDelegate` protocol and implemented in `BrowserChromeManager`, but never called anywhere.
|
||||
**Code state:** On the `sr-feedback` branch, commit `82bb0f71e3` removed the `embedDialogInEditingState` dead code path. The method may now be truly unreachable. Needs verification on #4668 branch.
|
||||
**Thread state:** 🔴 **Not resolved.** → Verify, then either add a caller or delete the method and its implementations.
|
||||
|
||||
---
|
||||
|
||||
## Open GitHub Comment Threads — PR #4855
|
||||
|
||||
All four Alessandro review comments and all Bugbot comments are addressed by the **uncommitted local changes** listed above. The GitHub threads are **not yet resolved** because the changes haven't been committed/pushed.
|
||||
|
||||
| Comment | Fix location | Thread resolved? |
|
||||
| PR | Branch | Review status |
|
||||
|---|---|---|
|
||||
| Use `NewTabPageControllerDelegate` instead of `(parent as?)` cast (line 280) | Uncommitted — `NewTabPageControllerDelegate.swift` | 🔴 No |
|
||||
| Hide both bars via `setBarsHidden` per Costas (line 477) | Uncommitted — `NewTabPageViewController.swift` | 🔴 No |
|
||||
| Fold loading check into `if` condition (line 473) | Uncommitted — `NewTabPageViewController.swift` | 🔴 No |
|
||||
| Revert `searchAndDuckAIOption` copy (UserText.swift:2375) | Uncommitted — `UserText.swift` + lproj files | 🔴 No |
|
||||
| Bugbot: Navigation bar not restored when NTP dismissed (line 281) | Uncommitted — `NewTabPageViewController.swift` `dismiss()` | 🔴 No |
|
||||
| Bugbot: Guard early return aborts dialog presentation (line 473) | Uncommitted — guard folded into `if` | 🔴 No |
|
||||
| Bugbot: Hardcoded debug return bypasses feature flag (OnboardingIntroViewModel:473) | Uncommitted — hardcoded return removed | 🔴 No |
|
||||
| Bugbot: Returning-user guard commented out (OnboardingIntroViewModel:451) | Uncommitted — guard uncommented | 🔴 No |
|
||||
|
||||
→ **Commit and push all local changes first. Then resolve these threads.**
|
||||
| [#4544](https://github.com/duckduckgo/apple-browsers/pull/4544) | `demo-tracker-blocking-onboarding` | Open — review required (Alessandro + Rachel left comments) |
|
||||
| [#4591](https://github.com/duckduckgo/apple-browsers/pull/4591) | `demo-tracker-blocking-onboarding-ui-polish` | Approved |
|
||||
| [#4664](https://github.com/duckduckgo/apple-browsers/pull/4664) | `demo-tracker-blocking-onboarding-chat-path-dialog-polish` | Approved |
|
||||
| [#4855](https://github.com/duckduckgo/apple-browsers/pull/4855) | `demo-tracker-blocking-onboarding-sr-feedback` | Open — no review yet |
|
||||
| [#4668](https://github.com/duckduckgo/apple-browsers/pull/4668) | `demo-tracker-blocking-onboarding-uti-flow` | Open — no review yet |
|
||||
|
||||
---
|
||||
|
||||
## Pre-Ship Blockers
|
||||
|
||||
### 1. Commit and push local changes on `sr-feedback`
|
||||
|
||||
All changes in the "Uncommitted Local Changes" section above must be committed to `alex/demo-tracker-blocking-onboarding-sr-feedback` and pushed before PR #4855 can be reviewed. Then the eight GitHub threads above can be resolved.
|
||||
|
||||
**Current state:** 🔴 Not done.
|
||||
|
||||
### 2. Tell UTI team the experiment name
|
||||
|
||||
**Source:** [Pete Apr 30 comment](https://app.asana.com/1/137249556945/task/1214147157456478/1214600831548835): Pete's team will exclude users enrolled in this experiment from the UTI feature rollout, to avoid breaking their onboarding. They need the final experiment name.
|
||||
|
||||
**Current state:** 🔴 Blocked — experiment name not final (see #4 below). Once known: fill into [O-J <> O-N Coordination](https://app.asana.com/1/137249556945/project/1214157224317277/task/1214288645859692) and [O-N Live Onboarding Experiment Details](https://app.asana.com/1/137249556945/task/1214601039604921).
|
||||
|
||||
### 3. Privacy config: add US/EN targeting to override
|
||||
|
||||
**Source:** [Task 1214682678629768](https://app.asana.com/1/137249556945/task/1214682678629768): experiment must be limited to US English users. Without this, all locales worldwide would be enrolled.
|
||||
|
||||
**What:** In `privacy-configuration/overrides/ios-override.json`, add `"targets": [{ "localeLanguage": "en", "localeCountry": "US" }]` to the experiment entry.
|
||||
|
||||
**Current state:** 🔴 Local edit exists on wrong branch (`alex/remove-autocomplete-tabs`) in the `privacy-configuration` repo, not committed.
|
||||
|
||||
### 4. Privacy config: rename the experiment
|
||||
|
||||
**Source:** [Task 1214682678629768](https://app.asana.com/1/137249556945/task/1214682678629768): some internal users were enrolled under `onboardingDuckAIQueryExperiment` during testing. A new name gives production users a clean, separate enrollment.
|
||||
|
||||
**What — three-part change:**
|
||||
1. **BSK** (`SharedPackages/BrowserServicesKit`): add a new case to `AIChatSubfeature` enum with the new name string.
|
||||
2. **App** (`iOS/Core/FeatureFlag.swift`): update `FeatureFlag.onboardingDuckAIQueryExperiment` to reference the new subfeature case.
|
||||
3. **`privacy-configuration` repo** (`overrides/ios-override.json`): add new experiment entry with `targets`, and set the old `onboardingDuckAIQueryExperiment` entry to `"state": "disabled"`.
|
||||
|
||||
Do this in the same PR as item #3 (both touch the same file in the same repo).
|
||||
|
||||
**Current state:** 🔴 Blocked — new experiment name not decided.
|
||||
|
||||
### 5. Fix commented-out feature flag line in PR #4668
|
||||
|
||||
**Source:** aataraxiaa review comment on #4668 (see Open Threads above).
|
||||
|
||||
**What:** Remove `defaultValue: .enabled` and the `// Config(...)` dead line from `FeatureFlag.swift:635` for `unifiedToggleInput`.
|
||||
|
||||
**Current state:** 🔴 Still in code on `#4668` branch. Not fixed.
|
||||
|
||||
### 6. Clarify "Ask AI" vs "Ask.ai" copy
|
||||
|
||||
**Source:** Conflict between GJ8 ("Ask AI") and Alessandro's review comment ("Ask.ai").
|
||||
|
||||
**Current state:** 🔴 Implemented as "Ask AI" (no dot) per GJ8. Awaiting explicit product/design confirmation.
|
||||
## Open Items
|
||||
|
||||
---
|
||||
|
||||
## Experiment Config (current, local override)
|
||||
### ✅ SR — GJ2/ Remove X button from chat-path dialogs
|
||||
|
||||
**Source:** [Gary Apr 29](https://app.asana.com/1/137249556945/task/1214147157456478/comment/1214409112744728)
|
||||
|
||||
**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](https://app.asana.com/1/137249556945/task/1214147157456478/comment/1214409112744728)
|
||||
|
||||
**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](https://app.asana.com/1/137249556945/task/1214147157456478/comment/1214409112744728), confirmed by [Gary May 7](https://app.asana.com/1/137249556945/task/1214147157456478/comment/1214615822543276) and [Costas May 8](https://app.asana.com/1/137249556945/task/1214147157456478/comment/1214627423981810): 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 above.
|
||||
|
||||
**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](https://app.asana.com/1/137249556945/task/1214147157456478/comment/1214595287457513): 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 above.
|
||||
|
||||
**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](https://app.asana.com/1/137249556945/task/1214147157456478/comment/1214592278234254): "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 above.
|
||||
|
||||
⚠️ 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](https://app.asana.com/1/137249556945/task/1214147157456478/comment/1214409112744728): "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](https://app.asana.com/1/137249556945/task/1214147157456478/comment/1214592278234254) confirmed in scope. [Costas May 8](https://app.asana.com/1/137249556945/task/1214147157456478/comment/1214627423981810): task [1214627423981798](https://app.asana.com/1/137249556945/project/1212535659026889/task/1214627423981798) created for animated assets. If not in time, defer to follow-up project. [Alessandro May 10](https://app.asana.com/1/137249556945/task/1214147157456478/comment/1214683268207876): 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](https://app.asana.com/1/137249556945/task/1214147157456478/comment/1214409112744728). [Pete May 7](https://app.asana.com/1/137249556945/task/1214147157456478/comment/1214600831548835): UTI experiment participants will be excluded from this experiment; UTI issues can be fixed in follow-up before June 1. [Alessandro May 7](https://app.asana.com/1/137249556945/task/1214147157456478/comment/1214595287457513): follow-up task [1214592943696234](https://app.asana.com/1/137249556945/project/1212535659026889/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](https://app.asana.com/1/137249556945/task/1214147157456478/comment/1214741014878191): "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 above).
|
||||
|
||||
---
|
||||
|
||||
### ➖ SR — GJ5/ Add new Dax brand assets to onboarding
|
||||
|
||||
**Source:** [Gary Apr 29](https://app.asana.com/1/137249556945/task/1214147157456478/comment/1214409112744728).
|
||||
|
||||
**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](https://app.asana.com/1/137249556945/task/1214147157456478/comment/1214627423981810). [Alessandro May 10](https://app.asana.com/1/137249556945/task/1214147157456478/comment/1214683268207876): comes for free once task [1213967368170629](https://app.asana.com/1/137249556945/project/1142021229838617/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.swift`
|
||||
- `iOS/DuckDuckGo/NewTabPageControllerDelegate.swift`
|
||||
- `iOS/DuckDuckGo/NewTabPageViewController.swift`
|
||||
- `iOS/DuckDuckGo/OnboardingFlow/LinearOnboarding/OnboardingIntroViewModel.swift`
|
||||
- `iOS/DuckDuckGo/OnboardingView+DuckAIExperimentSearchContent.swift`
|
||||
- `iOS/DuckDuckGo/UserText.swift` + all 26 lproj files
|
||||
|
||||
**Action:** Commit and push before #4855 can be reviewed.
|
||||
|
||||
---
|
||||
|
||||
### 🟠 PR #4855 — Alessandro (line 280): Use `NewTabPageControllerDelegate` instead of `(parent as? MainViewController)` cast
|
||||
|
||||
**Source:** [#4855 review comment, line 280](https://github.com/duckduckgo/apple-browsers/pull/4855)
|
||||
|
||||
**Fix:** Added `newTabPageControllerCurrentTabIsLoading(_ :) -> Bool` to `NewTabPageControllerDelegate` protocol; implemented in `MainViewController.swift`; removed the `(parent as? MainViewController)?` cast from `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](https://github.com/duckduckgo/apple-browsers/pull/4855/files#r3238549269)
|
||||
|
||||
**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.
|
||||
|
||||
---
|
||||
|
||||
### 🟠 PR #4855 — Alessandro (line 473): Fold loading check into `if` condition
|
||||
|
||||
**Source:** [#4855 review comment, line 473](https://github.com/duckduckgo/apple-browsers/pull/4855)
|
||||
|
||||
**Fix:** Replaced `guard (parent as? MainViewController)?.currentTab?.isLoading != true else { return }` (which was a standalone guard that aborted the whole function, leaving an orphaned `hostingController`) with `delegate?.newTabPageControllerCurrentTabIsLoading(self) != true` as a third condition in the outer `if`. The dialog still presents; only the bar-hiding is skipped when loading.
|
||||
|
||||
**State:** `sr-feedback / uncommitted` — fix is in working tree, NOT committed. GitHub thread NOT resolved.
|
||||
|
||||
---
|
||||
|
||||
### 🟠 PR #4855 — Alessandro (UserText.swift:2375): Revert `searchAndDuckAIOption` copy
|
||||
|
||||
**Source:** [#4855 review comment, UserText.swift:2375](https://github.com/duckduckgo/apple-browsers/pull/4855)
|
||||
|
||||
**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:** `sr-feedback / uncommitted` — fix is in working tree, NOT committed. GitHub thread NOT 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.
|
||||
|
||||
**State:** `sr-feedback / uncommitted` — fix is in working tree, NOT committed. GitHub thread NOT resolved.
|
||||
|
||||
---
|
||||
|
||||
### 🟠 PR #4855 — Bugbot: Guard early return leaves orphaned `hostingController` and blank NTP
|
||||
|
||||
**Source:** Bugbot on PR #4855, `NewTabPageViewController.swift:473`
|
||||
|
||||
**Fix:** Same as the Alessandro (line 473) item above — guard replaced with `if` condition using delegate method.
|
||||
|
||||
**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:** Removed `return .treatmentB` (and its `// TODO: Remove this` comment) from `resolveDuckAIQueryExperimentCohortID()`. The method now always goes through `featureFlagger.resolveCohort()`.
|
||||
|
||||
**State:** `sr-feedback / uncommitted` — fix is in working tree, NOT committed. GitHub thread NOT resolved.
|
||||
|
||||
---
|
||||
|
||||
### 🟠 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](https://github.com/duckduckgo/apple-browsers/pull/4544), Asana task [1214682678629767](https://app.asana.com/1/137249556945/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 #4544 — Bugbot (r3234645543) / SR GJ6b/: Chat-path EOJ not shown after tapping tracker dialog CTA
|
||||
|
||||
**Source:** [Bugbot comment r3234645543 on PR #4544](https://github.com/duckduckgo/apple-browsers/pull/4544#discussion_r3234645543), `TabViewController.swift:4214`. Also reported as SR GJ6b/ by [Gary Apr 29](https://app.asana.com/1/137249556945/task/1214147157456478/comment/1214409112744728) — 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: 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. **Needs manual verification.** GitHub thread NOT resolved.
|
||||
|
||||
---
|
||||
|
||||
### 🔴 PR #4544 — Bugbots (outdated): hardcoded cohort / promo override / missing pixels / commented-out redirect
|
||||
|
||||
**Source:** Four Bugbot comments on early commits of PR #4544.
|
||||
|
||||
| Comment | 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 are fixed in code. All four GitHub threads are **NOT resolved** (outdated Bugbot comments never closed). → Resolve all four threads.
|
||||
|
||||
---
|
||||
|
||||
### 🔴 PR #4668 — aataraxiaa: Commented-out `Config(...)` line in `FeatureFlag.swift:635`
|
||||
|
||||
**Source:** [aataraxiaa review comment on PR #4668](https://github.com/duckduckgo/apple-browsers/pull/4668), `FeatureFlag.swift:635`
|
||||
|
||||
**Issue:** `defaultValue: .enabled` override was added to `unifiedToggleInput` for testing, plus the old `// Config(...)` line is still sitting underneath it 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: `embedInUnifiedInputEditingAreaIfActive` added to protocol but never called
|
||||
|
||||
**Source:** Bugbot on PR #4668, `BrowserChromeManager.swift:43` and `MainViewController.swift:3457`
|
||||
|
||||
**Issue:** Method added to `BrowserChromeDelegate` protocol and implemented in `BrowserChromeManager`, but no caller exists anywhere.
|
||||
|
||||
**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.
|
||||
|
||||
---
|
||||
|
||||
### 🟠 DEAD-1 — `setChatPathVisitSiteControlsLocked` in `MainViewController+DuckAIExperiment.swift:166` is dead code
|
||||
|
||||
**Source:** Callers removed as part of PR #4855 Alessandro (line 477) fix above.
|
||||
|
||||
**State:** Method definition still exists in `MainViewController+DuckAIExperiment.swift:166`, no callers. Needs deletion. Not committed.
|
||||
|
||||
---
|
||||
|
||||
### ⚠️ GJ8 / Copy: "Ask AI" vs "Ask.ai"
|
||||
|
||||
**Source:** [GJ8 ship review](https://app.asana.com/1/137249556945/task/1214147157456478) says *"Ask AI"* (no dot). [Alessandro's #4855 review comment](https://github.com/duckduckgo/apple-browsers/pull/4855) says *"Ask.ai"* (with dot).
|
||||
|
||||
**State:** Implemented as "Ask AI" (no dot) per GJ8. ⚠️ Needs explicit confirmation from product/design before shipping.
|
||||
|
||||
---
|
||||
|
||||
### 🔴 PRE-SHIP-1 — Privacy config: add US/EN targeting
|
||||
|
||||
**Source:** [Task 1214682678629768](https://app.asana.com/1/137249556945/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](https://app.asana.com/1/137249556945/task/1214682678629768). Internal testers already enrolled under `onboardingDuckAIQueryExperiment`; new name ensures clean production enrollment.
|
||||
|
||||
**What (three parts):**
|
||||
1. **BSK** — new case in `AIChatSubfeature` enum with the new name string
|
||||
2. **App** — update `FeatureFlag.onboardingDuckAIQueryExperiment` to reference the new subfeature case
|
||||
3. **`privacy-configuration` repo** — add new experiment entry with `targets`; set old entry to `"state": "disabled"`
|
||||
|
||||
**State:** 🔴 Blocked — new experiment name not decided.
|
||||
|
||||
---
|
||||
|
||||
### 🔴 PRE-SHIP-3 — Tell UTI team the final experiment name
|
||||
|
||||
**Source:** [Pete Apr 30 comment](https://app.asana.com/1/137249556945/task/1214147157456478/1214600831548835). Pete's team needs the name to exclude enrolled users from the UTI feature rollout.
|
||||
|
||||
**What:** Fill the name into [O-J <> O-N Coordination](https://app.asana.com/1/137249556945/project/1214157224317277/task/1214288645859692) and [O-N Live Onboarding Experiment Details](https://app.asana.com/1/137249556945/task/1214601039604921).
|
||||
|
||||
**State:** 🔴 Blocked on PRE-SHIP-2 (name not decided).
|
||||
|
||||
---
|
||||
|
||||
## 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** 🟠 Implemented in `OnboardingView+DuckAIExperimentSearchContent.swift`, not yet committed. ⚠️ Pending copy confirmation ("Ask AI" vs "Ask.ai").
|
||||
|
||||
**Experiment rename in privacy config** 🔴 TBD. Need to create a new experiment name, add new `AIChatSubfeature` case in BSK, update `FeatureFlag`, and add new entry in `privacy-configuration/overrides/ios-override.json` (disabling old one). Blocked — new name not decided.
|
||||
|
||||
**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](https://app.asana.com/1/137249556945/task/1214601039604921))** 🔴 TBD. Provide Pete with final experiment name once rename is decided. Also fill into [O-J <> O-N Coordination](https://app.asana.com/1/137249556945/project/1214157224317277/task/1214288645859692).
|
||||
|
||||
---
|
||||
|
||||
## Experiment Config (current, local override only)
|
||||
|
||||
`privacy-configuration/overrides/ios-override.json` under `aiChat` subfeatures:
|
||||
|
||||
|
||||
Reference in New Issue
Block a user