From 918a31286ffd016eb265d6ba99bac853c0bda5c1 Mon Sep 17 00:00:00 2001 From: Alexey Martemyanov Date: Fri, 15 May 2026 15:57:57 +0600 Subject: [PATCH] =?UTF-8?q?2026-05-16=20media-toolbox:=20session=209=20?= =?UTF-8?q?=E2=80=94=20cyr=E2=86=92lat=20fix,=20year-=D0=B3=20fix,=20genre?= =?UTF-8?q?=20routing?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- family/projects/media-toolbox-kraken.md | 9 + .../chat-path-onboarding-tracker-blocking.md | 611 ++++++++++-------- 2 files changed, 350 insertions(+), 270 deletions(-) diff --git a/family/projects/media-toolbox-kraken.md b/family/projects/media-toolbox-kraken.md index 9ec7576b..9ea85aca 100644 --- a/family/projects/media-toolbox-kraken.md +++ b/family/projects/media-toolbox-kraken.md @@ -168,6 +168,14 @@ media-pipeline (~/Developer/media_files_db, ветка kraken-agent) `ni4ego_horoshego`, `o_4em_govoryat_muzh4iny` — цифра `4` = `ч` в русском warez-транслите. Стандартная транслитерация не покрывает. **Фикс:** `String.decodeLeetSpeak()` в `StringExtensions.swift` + дополнительный variant в `NameTokenization.variants`. +### Баг 10: Год `2023г` не парсился как год +`Миграция. 2023г.mkv` — кириллическая `г` (год) является word char, поэтому `\b` после `2023` не срабатывает → `qYear=nil` → TMDb ищет без фильтра года → 0 результатов. +**Фикс:** В `TitleParser.cleanupTitleAndYear` добавлен паттерн `(19|20)\d\d)г\b` до стандартного `\b` regex. Коммит `770e51e`. + +### Баг 11: Cyr→Lat query variant — мусорные запросы +`NameTokenization` генерировал `transliteratedToLatin` variant для кириллических имён → `Миграция` → `Migratsiya` → TMDb/KP не знают транслит, 0 результатов, только занимают cache-слоты. Оригинальный Go-sync делал только Latin→Cyrillic. +**Фикс:** Убран `transliteratedToLatin` из query variants в `NameTokenization.swift`. Коммит `10d64da`. + ### Баг 6: resolve-manual NFO path для single-файлов `resolve-manual` писал NFO внутрь файла вместо рядом. **Фикс:** Detect `isDirectory`, для файлов → `deletingPathExtension().appendingPathExtension("nfo")`. Коммит `c0fdc59` @@ -245,3 +253,4 @@ docker run --rm ... media-pipeline:kraken \ - **2026-05-14 сессия 6:** dry_run=false, sync запущен → 401 resolved, 32 unresolved. `apply` создаёт canonical папки + симлинки (оригиналы не трогает). 716 NFO написано. Валидация торрентов HTPC. - **2026-05-15 сессия 7:** Торренты: 467 скопированы с HTPC, 323 добавлены в Transmission (294 итого после удаления 50 игр/нон-медиа). rsync: movies + мультфильмы + BBC документалки. Фиксы в коде: (1) best-effort collapse при исчерпании tierов, (2) дедупликация dir+file регионов в RegionPartitioner, (3) strip .aka./full.screen из TitleParser, (4) scanner пропускает canonical dirs по маркеру `.media-pipeline-canonical`, (5) Roman numerals в транслитерации (Ампир V). Финал: 19 unresolved. - **2026-05-16 сессия 8:** Обнаружен корневой баг: KP API переехал `api.kinopoisk.dev` → `api.poiskkino.dev` (301 redirect, Swift теряет X-API-KEY). Исправлен URL. Удалены 359 невалидных KP-кеш записей. Добавлен leet-speak decode (`4`→`ch`: ni4ego→nichego, muzh4iny→muzhchiny). Починен кеш: KP пустые `docs:[]` больше не кешируются (были причиной повторных промахов после rate-limit/redirect). **Текущий unresolved: 22. Завтра KP лимит сбросится → ожидаем ≤5.** +- **2026-05-16 сессия 9:** Добавлен жанровый роутинг (cartoons/documentaries — только по genres из провайдера, не по source path). Фикс `nfoMeta priority` (provider genres > local empty NFO). Фикс парсинга года `2023г` (русский суффикс). TMDb genres передаются из config. **Убран Cyr→Lat query variant** (не соответствует оригинальному Go-sync, TMDb не ищет по транслиту). Порядок провайдеров подтверждён: TMDb → KP → IMDb (short-circuit после каждого). KP — настоящий fallback (40 запросов vs 8428 TMDb). Текущий unresolved: 30 (ждём сброса KP rate limit). diff --git a/work/wiki/apple-browsers/chat-path-onboarding-tracker-blocking.md b/work/wiki/apple-browsers/chat-path-onboarding-tracker-blocking.md index 782dc148..7b41c28a 100644 --- a/work/wiki/apple-browsers/chat-path-onboarding-tracker-blocking.md +++ b/work/wiki/apple-browsers/chat-path-onboarding-tracker-blocking.md @@ -46,7 +46,7 @@ **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. +**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. @@ -56,7 +56,7 @@ **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. +**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. @@ -64,9 +64,9 @@ ### 🟠 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." +**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. +**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. @@ -74,9 +74,9 @@ --- -### 🔴 SR — GJ6b/ "You’ve got this" EOJ fails after tapping "Got it" on tracker blocking step +### 🔴 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." +**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. @@ -88,7 +88,7 @@ This is the same bug as **PR #4544 — Bugbot r3234645543** below. Fix is in `di **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. +**State:** 🔴 NOT implemented. Nice-to-have — defer to follow-up if time doesn't allow. --- @@ -104,7 +104,7 @@ This is the same bug as **PR #4544 — Bugbot r3234645543** below. Fix is in `di **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). +**State:** 🔴 In progress. PR #4668 has open comment threads (see aataraxiaa + Bugbot items below). --- @@ -141,6 +141,335 @@ This is the same bug as **PR #4544 — Bugbot r3234645543** below. Fix is in `di --- +## 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](https://github.com/duckduckgo/apple-browsers/pull/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](https://github.com/duckduckgo/apple-browsers/pull/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](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: 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](https://github.com/duckduckgo/apple-browsers/pull/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](https://github.com/duckduckgo/apple-browsers/pull/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](https://github.com/duckduckgo/apple-browsers/pull/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. + +--- + +### 🔴 PR #4591 — Bugbot (RebrandedNewTabDaxDialogFactory.swift): `onPresented` passes `false` but sheet was shown + +**Source:** Bugbot on PR #4591, `RebrandedNewTabDaxDialogFactory.swift` + +**Issue:** `onPresented` closure is called with `false` despite the subscription sheet having been shown, leading to incorrect state tracking. + +**State:** 🔴 [OPEN] on GitHub. Needs fix. + +--- + +### ✅ [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](https://github.com/duckduckgo/apple-browsers/pull/4664), `RebrandedNewTabDaxDialogFactory.swift` + +**Fix:** `isChatPath` check added — title now conditional on `chatPathPhase == .visitSite`. + +**State:** ✅ [Resolved] on GitHub. Fixed in PR #4664. + +--- + +### 🔴 PR #4664 — Bugbot (RebrandedContextualOnboardingDialogs+SubscriptionPromo.swift:101): Dismiss button appears on standard path + +**Source:** Bugbot on PR #4664, `RebrandedContextualOnboardingDialogs+SubscriptionPromo.swift:101` + +**Issue:** Dismiss button is rendered unconditionally, but should only appear on the chat path. + +**State:** 🔴 [OPEN] on GitHub. Needs a chat-path guard on the dismiss button. + +--- + +## 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](https://github.com/duckduckgo/apple-browsers/pull/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. + +**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:** Replaced the standalone guard with `delegate?.newTabPageControllerCurrentTabIsLoading(self) != true` as a third condition in the outer `if` block. The dialog still presents; only the bar-hiding is skipped when the tab is loading. + +**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 #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) @@ -163,59 +492,19 @@ This is the same bug as **PR #4544 — Bugbot r3234645543** below. Fix is in `di --- -### 🟠 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 +### 🟠 [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. - ---- - -### 🟠 [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. +**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](https://github.com/duckduckgo/apple-browsers/pull/4855), `OnboardingIntroViewModel.swift:450`. Also referenced in [Alessandro’s May 15 Asana comment](https://app.asana.com/1/137249556945/task/1214348462471176/comment/1214794564106665). +**Source:** [Alessandro May 15 review on PR #4855](https://github.com/duckduckgo/apple-browsers/pull/4855), `OnboardingIntroViewModel.swift:450`. Also referenced in [Alessandro's May 15 Asana comment](https://app.asana.com/1/137249556945/task/1214348462471176/comment/1214794564106665). **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. @@ -225,224 +514,6 @@ This is the same bug as **PR #4544 — Bugbot r3234645543** below. Fix is in `di --- -### 🟠 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**. - ---- - -### ✅ [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 (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 #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: 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 #4544 — Alessandro (DaxDialogs.swift:125): `DaxDialogProvider` refactoring suggestion - -**Source:** [Alessandro May 7 review on PR #4544](https://github.com/duckduckgo/apple-browsers/pull/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](https://github.com/duckduckgo/apple-browsers/pull/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. - ---- - -### ✅ [Resolved] PR #4544 — Bugbots (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` | -| `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 | -| Alessandro: tests replicating real `DaxDialogsSettings` logic | Simplified tests | - ---- - -## Open Items — PR #4591 - -> PR #4591 (`demo-tracker-blocking-onboarding-ui-polish`) is **Approved**. Open threads still need resolving before merge. - -### 🔴 PR #4591 — Alessandro (RebrandedNewTabDaxDialogFactory.swift): Closure sent via notification - -**Source:** [Alessandro review on PR #4591](https://github.com/duckduckgo/apple-browsers/pull/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](https://github.com/duckduckgo/apple-browsers/pull/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. This was 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](https://github.com/duckduckgo/apple-browsers/pull/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. - ---- - -### 🔴 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 ([comment](https://github.com/duckduckgo/apple-browsers/pull/4591)): pixel originates in upstream PR #4575, not this PR. - -**State:** 🔴 [OPEN] on GitHub. Acknowledged as upstream issue. - ---- - -### 🔴 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 (RebrandedNewTabDaxDialogFactory.swift): `onPresented` passes `false` but sheet was shown - -**Source:** Bugbot on PR #4591, `RebrandedNewTabDaxDialogFactory.swift` - -**Issue:** `onPresented` closure is called with `false` despite the subscription sheet having been shown, leading to incorrect state tracking. - -**State:** 🔴 [OPEN] on GitHub. Needs fix. - ---- - -### ✅ [Resolved] PR #4591 — Bugbots (resolved threads) - -| Thread | State | -|---|---| -| Missing experiment success pixel when `shouldShowSyncEnabled` is true (`SyncSettingsViewController.swift:567`) | [Resolved] | -| Duplicated editing-state embedding constraint logic (`NewTabPageViewController.swift`) | [Resolved] — `embedDialogInEditingState` removed in `sr-feedback` | -| Verbose multi-line comments explain standard patterns | [Resolved] | -| Missing `?? parent` fallback when finding editing controller | [Resolved] — editing state approach removed | -| Private method `embedDialogInEditingState` is never called | [Resolved] — method removed in `sr-feedback` | - ---- - -### ✅ [Resolved] PR #4664 — Bugbot: NTP dialog title changed for all paths, not just chat-path - -**Source:** Bugbot + [Alessandro review on PR #4664](https://github.com/duckduckgo/apple-browsers/pull/4664), `RebrandedNewTabDaxDialogFactory.swift` - -**Fix:** `isChatPath` check added — title now conditional on `chatPathPhase == .visitSite`. - -**State:** ✅ [Resolved] on GitHub. Fixed in PR #4664. - ---- - -### 🔴 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. @@ -512,7 +583,7 @@ All resolved on GitHub. Fixed in later commits on the `#4544` branch. **O-N Live Onboarding Experiment Details ([task 1214601039604921](https://app.asana.com/1/137249556945/task/1214601039604921))** 🔴 Name now known (`onboardingDuckAIQueryTrackersDemoExperiment`). Fill into [O-N Live Onboarding Experiment Details](https://app.asana.com/1/137249556945/task/1214601039604921) and [O-J <> O-N Coordination](https://app.asana.com/1/137249556945/project/1214157224317277/task/1214288645859692) once PRE-SHIP-2 is implemented. -**Alessandro’s May 15 QA** — Tested PR #4855 against test cases in [task 1214683268207880](https://app.asana.com/1/137249556945/task/1214683268207880). Test run: [task 1214794564106677](https://app.asana.com/1/137249556945/project/1206329551987282/task/1214794564106677). Left notes in test run and new comments in PR (see PR #4855 — Alessandro line 450 item above). +**Alessandro's May 15 QA** — Tested PR #4855 against test cases in [task 1214683268207880](https://app.asana.com/1/137249556945/task/1214683268207880). Test run: [task 1214794564106677](https://app.asana.com/1/137249556945/project/1206329551987282/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.