feat(klaviyo): resolve unique_id from a mapping rule - #709
Open
chrizzellu wants to merge 2 commits into
Open
Conversation
A mapping rule's `settings.value` was assigned to `properties.value`, where Klaviyo treats it as a segmentable custom property and ignores it for revenue reporting, while `valueCurrency` was emitted on the event attributes with no sibling `value` to denominate. `klaviyo-api`'s ObjectSerializer builds the request body from EventCreateQueryV2ResourceObjectAttributes.attributeTypeMap, which maps `value` -> `value` and `valueCurrency` -> `value_currency` at the attributes level; `properties` is typed `object` and passes through untouched. So the currency was reaching Klaviyo and the amount never was. Both now sit together on the attributes. Properties are unaffected -- map a value through the rule's `data` mapping to also keep it as a custom property. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Mapping rules accept `settings.uniqueId`, resolved onto the event's `uniqueId` attribute (`unique_id` on the wire). Klaviyo keeps only the first event with a given value for one profile and metric, so the same order can reach it from several producers -- browser tracking, a CSV or backfill import, a retried delivery -- without being counted twice. Absent the field Klaviyo dedups on the event time truncated to the second, which admits one event per profile per metric per second. Numeric ids are coerced to strings: order ids are commonly numeric, and dropping one would silently fall back to that window and re-admit the duplicates the key exists to prevent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
Included review availability: Your plan includes up to 2 reviews per rolling hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe Klaviyo destination now emits revenue as top-level ChangesKlaviyo event mapping
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The change adds rule-level resolution of Klaviyo's unique event identifier, including numeric values, while invalid values remain unset. No actionable merge-blocking risk remains after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant Mapping
participant Push as push.ts
participant ResolveId as resolveId
participant Klaviyo
Mapping->>Push: Provide value and uniqueId mappings
Push->>ResolveId: Resolve mapped uniqueId
ResolveId-->>Push: Return string identifier
Push->>Klaviyo: Emit value, valueCurrency, and uniqueId
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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
1 participant
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.
What
Mapping rules accept
settings.uniqueId, resolved onto the event'suniqueIdattribute (unique_idon the wire).Why
Klaviyo deduplicates on the tuple
(profile, metric, unique_id). Without the field, the API defaults it to "the time to the second", so two events for one profile and metric landing in the same second collide and one is dropped — data loss, not just a missed dedup.With it, the same event can reach Klaviyo from several producers — browser tracking running alongside this destination, a CSV or backfill import, a retried delivery — without being counted twice. There is currently no way to express that key through this destination.
Verified against the live API
I ran a dedup probe against a real Klaviyo account (four groups, two sends each, controls included):
unique_id×2 (control)unique_id×2 (control)unique_id→ serverunique_id$event_idproperty → serverunique_idBoth controls dedup, so the method holds.
unique_iddeduplicates across the browser (/client/events/) and server (/api/events/) boundary; a$event_idevent property does not participate at all. That is what makes this the right field to expose.Details
Rule-level only, mirroring the existing
settings.value— a dedup key is inherently per-event, so a destination-level default did not seem worth it (happy to add if you disagree).Numeric ids are coerced to strings: order ids are commonly numeric, and silently dropping one would fall back to the one-per-second window and re-admit exactly the duplicates the key exists to prevent.
NaN,Infinity, empty string, booleans and objects all resolve to no key rather than a garbage one.Verification
npm run verify:touched -- server-destination-klaviyo— greennpm run verify:affected— 77/77 greenpublic: falseregression fixture for the numeric case), each watched fail before implementing.Changeset included (minor).
Summary by CodeRabbit