Conversation
| "owners": ["dus7", "hassaanelgarem"], | ||
| "triggers": ["other"], | ||
| "suffixes": ["platform", "form_factor"], | ||
| "parameters": ["appVersion", "burnSource", "tabType"] |
There was a problem hiding this comment.
Boolean-typed params defined as string with enum
Low Severity
The m_automatic_data_clearing_options_updated definition (modified in this PR) has three inline parameters (tabs, data, chats) defined as "type": "string" with "enum": ["true", "false"]. Per review rules, parameters whose enum contains only "true" and/or "false" need to be "type": "boolean" with no enum.
Additional Locations (2)
Triggered by project rule: PR Review Guidelines for Cursor Bugbot
Reviewed by Cursor Bugbot for commit 45b5379. Configure here.
|
Privacy Review task: https://app.asana.com/0/69071770703008/1214043759314110 |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit b4f9326. Configure here.
| case false: | ||
| return "normal" | ||
| } | ||
| } |
There was a problem hiding this comment.
Redundant Tab.pixelParamValue duplicates BrowsingMode.pixelParamValue
Low Severity
The new Tab.pixelParamValue computed property re-implements the same fireTab→"fire"/"normal" mapping that BrowsingMode.pixelParamValue already provides. Since Tab already has a mode property (of type BrowsingMode), the extension body could simply delegate to mode.pixelParamValue instead of duplicating the switch logic. This avoids a maintenance risk if the set of modes ever changes.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit b4f9326. Configure here.


Task/Issue URL: https://app.asana.com/1/137249556945/project/72649045549333/task/1213299214394241
Description
browsing_modeparameter to existing data clearing pixels (forgetAllPressedBrowsing,forgetAllPressedTabSwitching,forgetAllExecuted,singleTabBurnExecuted,singleTabDataClearedand their daily variants) to distinguish fire vs normal mode usagebrowsing_modeparameter to core tab switcher pixels (new tab, switch tabs, close tab, long press, opened daily) and address bar click pixelsFireModeSwitchSourceenum to track how users enter Fire Mode (tab selection, long press tabs icon, NTP/menu promotion, long press link, browsing menu)PixelParameters.browsingModeconstant,BrowsingMode.pixelParamValuehelper, anddurationshared param in pixel definitionsTesting Steps
m_tab-switcher_mode-toggledfires withsource=tap. Swipe between modes — verify same pixel fires withsource=swipem_browsing-mode_switchedfires withsource=tab_selectionm_browsing-mode_switchedfires withsource=long_press_tabs_iconm_link-long-press_fire-tabfiresm_fire-mode_last-tab-closed_burnfiresm_fire-mode_empty-state_new-tabfiresbrowsing_modeparam appears onmf_bp,mf, and scoped burn pixelsImpact and Risks
Low: Minor visual changes, small bug fixes, improvement to existing features
What could go wrong?
The PR only adds or modifies pixels. The only risk is incorrect mode sent with pixels.
Quality Considerations
N/A
Notes to Reviewer
N/A
Internal references:
Definition of Done | Engineering Expectations | Tech Design Template
Note
Medium Risk
Primarily telemetry changes, but it touches core browsing-mode switching APIs (
TabManager.setBrowsingMode) and multiple user interaction paths, so incorrect parameterization or missed call sites could skew analytics or affect mode switching behavior.Overview
Adds a new Fire Mode pixel set covering NTP/menu promotions, mode switching (with source attribution), tab switcher mode toggles (tap vs swipe), scoped burns/data-clearing, last-fire-tab auto-burn, Fire Mode empty-state actions, and link long-press actions.
Introduces a shared
browsing_modeparameter (pluspixelParamValuehelpers) and threads it through existing pixels (Fire button flows, tab switcher interactions, address bar click, etc.), while updating timed data-clearing pixels to use the shareddurationSeconds/durparameter definition.Updates browsing-mode switching to require a
FireModeSwitchSource, firingm_browsing-mode_switchedwhenever the mode actually changes, and adjusts call sites/tests/mocks accordingly.Reviewed by Cursor Bugbot for commit b4f9326. Bugbot is set up for automated code reviews on this repo. Configure here.