fix: skip profile reset when setProfile called with same identifiers#543
Open
jason-myers-klaviyo wants to merge 6 commits intomasterfrom
Open
fix: skip profile reset when setProfile called with same identifiers#543jason-myers-klaviyo wants to merge 6 commits intomasterfrom
jason-myers-klaviyo wants to merge 6 commits intomasterfrom
Conversation
When setProfile() was called with a profile whose identifiers matched current state, it unconditionally called reset(), regenerating the anonymous ID. This caused unnecessary anonymous ID churn, triggering spurious push-token API requests — the same bug as the Android SDK (klaviyo/klaviyo-android-sdk#435). Now enqueueProfile only resets when a provided (non-nil) identifier actually differs from current state. Anonymous ID is the lowest-order identifier and doesn't need regeneration when higher-order identifiers are unchanged. resetProfile() remains available for explicitly clobbering all state. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
evan-masseau
reviewed
Apr 8, 2026
njparadis
reviewed
Apr 8, 2026
njparadis
left a comment
There was a problem hiding this comment.
reviews for the review god, comments for the comment throne
…rm fix) Cover the new identifiers-changed guard in enqueueProfile: - Same identifiers → no reset, anonymousId preserved, pushTokenData stays - Different identifiers → reset fires, pushTokenData cleared - Same identifiers + different attributes → no reset, attributes still sent - Partial identifier match → reset fires - Nil identifiers (not specified) → skips comparison, no reset - resetProfile → still clobbers all state unconditionally Part of MAGE-473 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Profile values are trimmed (via trimWhiteSpaceOrReturnNilIfEmpty) before being stored in state, but the identifiers-changed comparison was using raw values. Whitespace-padded inputs like " user@email.com " would never match the trimmed state value, defeating the optimization and reproducing the original reset storm. Apply the same normalization in the comparison. nil still means "not specified" (skip comparison), but empty strings still trigger reset (they represent explicit identifier clearing). Part of MAGE-473 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The nil-skip approach (nil = "not specified", skip comparison) silently changed setProfile from a clobbering to a merging operation for unspecified fields. This is a bigger behavioral departure than intended for this hotfix. Align with Android: compare all three identifiers including nil, with an isIdentified guard so fresh state (no prior identifiers) skips the comparison. The only new behavior vs the old code is: exact match on all identifiers → skip reset. Part of MAGE-473 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 3 total unresolved issues (including 2 from previous reviews).
❌ 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 ac611dc. Configure here.
With the isIdentified guard, setProfile on fresh state (no prior identifiers) no longer triggers reset(). Two existing tests expected pushTokenData to be cleared by reset — update them to reflect that pushTokenData is now preserved when going from anonymous to identified. Part of MAGE-473 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Summary
Fixes excessive
POST /client/push-tokensrequests caused bysetProfile()being called repeatedly with the same identifiers.Root cause:
enqueueProfileunconditionally calledstate.reset(preserveTokenData: false)regardless of whether the incoming identifiers matched current state. Each reset regenerated the anonymous ID (inreset()line 223), which triggered spurious push-token API requests. Under rate limiting, this compounded into a request storm.Same bug as the Android SDK: klaviyo/klaviyo-android-sdk#435
Fix: Compare incoming (non-nil) identifiers against current state before calling
reset(). Anonymous ID is the lowest-order identifier — there's no reason to regenerate it when higher-order identifiers (email, phone, externalId) haven't changed.resetProfile()remains available for explicitly clobbering all state.Only non-nil identifiers are compared, since nil means "not specified" per
updateStateWithProfilesemantics.Slack thread: https://klaviyo.slack.com/archives/C09DKBLQ3PX/p1775509483748149
Test plan
setProfile(same identifiers)repeatedly — confirm no spurious push-token requestssetProfile(different identifiers)— confirm reset still fires, anonymous ID regeneratedsetProfile(same identifiers, different attributes)— confirm attributes still updateresetProfile()— confirm still clobbers all state as before🤖 Generated with Claude Code