fix: keep the dedup window for its full minute; add the idempotency_key track property - #89
Conversation
…urrences with idempotency_key Ports getformo/sdk#375 and getformo/sdk#389 to the React Native SDK. Duplicate suppression used to be tied to delivery: an event's hash was dropped when its batch was sent. The first event of an app session is sent the moment it arrives, so its hash left with it and an identical track() right behind it (the double-fire this exists to catch) was accepted. The hash also folded in the minute-truncated timestamp, so a double-fire straddling a minute boundary got two different hashes. - Accepted fingerprints now live in an insertion-ordered map and expire 60 seconds after acceptance, independent of flushing. Expired entries are pruned from the front. The clock never steps backwards (max of a monotonic source and a forward-only wall-clock sum). - The fingerprint excludes the timestamp. A permanently rejected batch releases its own entries (by acceptance token) so the app can retry; a transiently failed batch keeps them while it waits. - Custom track() events are fingerprinted on what the caller passed (name, properties, caller context, address, user id), before SDK context such as screen and app state is added. Other event types keep the enriched fingerprint. - Wire identity is split from the fingerprint. Unkeyed custom events get a random UUID. Automatic events keep the deterministic content-and-minute id, so ingestion counts do not shift on upgrade. The reserved idempotency_key property hashes event type + name + key into a stable id; the key is lifted out of the properties and never sent. Strings and finite numbers are accepted; anything else drops the call with a warning and never throws into the host. - hash() is synchronous so the enqueue path gains no extra yield. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Afmyxxipx1ATADBWi2bFGZ
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
1 issue found across 11 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/lib/event/EventQueue.ts">
<violation number="1" location="src/lib/event/EventQueue.ts:659">
P2: When the device clock jumps forward during the dedup window, this expires fingerprints early and allows an identical event within the intended 60-second window to be sent. Base expiry on a monotonic elapsed-time source, or otherwise bound wall-clock adjustments so forward steps cannot shorten the duplicate-suppression window.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| const wallNow = Date.now(); | ||
| const delta = wallNow - this.lastWall; | ||
| this.lastWall = wallNow; | ||
| if (delta > 0) this.wallElapsed += delta; |
There was a problem hiding this comment.
P2: When the device clock jumps forward during the dedup window, this expires fingerprints early and allows an identical event within the intended 60-second window to be sent. Base expiry on a monotonic elapsed-time source, or otherwise bound wall-clock adjustments so forward steps cannot shorten the duplicate-suppression window.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/lib/event/EventQueue.ts, line 659:
<comment>When the device clock jumps forward during the dedup window, this expires fingerprints early and allows an identical event within the intended 60-second window to be sent. Base expiry on a monotonic elapsed-time source, or otherwise bound wall-clock adjustments so forward steps cannot shorten the duplicate-suppression window.</comment>
<file context>
@@ -539,6 +600,85 @@ export class EventQueue implements IEventQueue {
+ const wallNow = Date.now();
+ const delta = wallNow - this.lastWall;
+ this.lastWall = wallNow;
+ if (delta > 0) this.wallElapsed += delta;
+
+ const mono = monotonicNow();
</file context>
There was a problem hiding this comment.
Considered and kept as designed; this mirrors the web SDK's accepted #375 implementation.
A forward wall-clock step expiring entries early is the deliberate trade-off, and the comment on elapsedNow() states it. The monotonic clock cannot be the only source: on some platforms performance.now() stops while the device sleeps, so a phone that sleeps mid-window would wake still inside the window, potentially long after the 60 seconds are really over. The wall clock is what counts suspension. Reading it as a sum of forward deltas means a backward step cannot pin the window, and a forward step can only shorten it. For a duplicate guard, shortening is the safe direction: the failure mode is one extra event after a clock correction, not a real event silently dropped. The two sources are combined with max(), so the monotonic clock still governs whenever it is running.
There was a problem hiding this comment.
Same finding as the earlier thread on this file, kept as designed: the monotonic clock can stop while the device sleeps, so the wall clock must count, and it is read as a forward-only sum so a step can only shorten the window. For a duplicate guard, shortening is the safe direction; the sources are combined with max() so the monotonic clock governs whenever it runs. Mirrors the accepted web SDK implementation (getformo/sdk#375).
An object whose prototype carries idempotency_key is not a keyed call. Mirrors the web SDK fix. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Afmyxxipx1ATADBWi2bFGZ
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Afmyxxipx1ATADBWi2bFGZ
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Afmyxxipx1ATADBWi2bFGZ
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Afmyxxipx1ATADBWi2bFGZ
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Afmyxxipx1ATADBWi2bFGZ
Screen size and network state can change between two calls that are the same call (a rotation, a wifi-to-cellular handover), and they made such a double-fire look like two events to the 60-second guard. They are left out of the fallback fingerprint only; the wire id is unchanged, so what ingestion collapses does not move. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Afmyxxipx1ATADBWi2bFGZ
…ide the error boundary Mirrors the web SDK fixes from its review: - The track fingerprint is taken before the enrichment await, so a properties object the app mutates while enrichment is pending cannot record the event under values it did not carry. - Numeric keys must be safe integers: above 2^53 distinct ids compare equal as numbers and would share one message id. - A throwing getter or proxy on the property bag is logged, not rejected into the host. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Afmyxxipx1ATADBWi2bFGZ
There was a problem hiding this comment.
All reported issues were addressed across 4 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
|
@codex review |
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Afmyxxipx1ATADBWi2bFGZ
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: de8bfd1f43
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } else if (typeof rawKey === "number" && Number.isSafeInteger(rawKey)) { | ||
| // Above 2^53 distinct ids compare equal as numbers; pass those as | ||
| // strings. | ||
| idempotencyKey = String(rawKey); |
There was a problem hiding this comment.
Honor the documented finite-number idempotency keys
When a caller follows the track() documentation and supplies a finite non-integer key such as idempotency_key: 1.5, Number.isSafeInteger rejects it and returns before trackEvent, silently losing the event. Either accept all finite numbers as the documented cross-SDK contract states or narrow the public contract so callers are not told this input is supported.
Useful? React with 👍 / 👎.
| // Check if an identical event was accepted within the dedup window | ||
| if (this.isDuplicate(dedupKey)) { |
There was a problem hiding this comment.
Preserve repeated lifecycle transitions within the window
When an app completes two genuine background/foreground cycles within 60 seconds and the first cycle's batch has already flushed, the second Application Backgrounded, Application Opened, and optional Application Foregrounded calls have identical pre-enrichment fingerprints and are dropped here. AppLifecycleManager explicitly emits these events for every qualifying AppState transition, so retaining their fingerprints after delivery undercounts rapid but valid lifecycle activity; these SDK-generated occurrence events need a per-transition identity or an exemption from this deduplication.
Useful? React with 👍 / 👎.
…ase for the window clock Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Afmyxxipx1ATADBWi2bFGZ
…t-native 1.3.0 Every web example moves to ^1.41.0 and with-react-native to ^1.3.0, lockfiles refreshed lockfile-only. The examples that enforce a seven-day minimumReleaseAge already exclude the first-party SDK, so frozen installs accept the new release. 1.41.0 / 1.3.0 add the reserved idempotency_key track property and the rolling 60-second duplicate window (getformo/sdk#389, #375; getformo/sdk-react-native#89). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Afmyxxipx1ATADBWi2bFGZ
…t-native 1.3.0 (#340) * chore: bump SDKs to @formo/analytics 1.41.0 and @formo/analytics-react-native 1.3.0 Every web example moves to ^1.41.0 and with-react-native to ^1.3.0, lockfiles refreshed lockfile-only. The examples that enforce a seven-day minimumReleaseAge already exclude the first-party SDK, so frozen installs accept the new release. 1.41.0 / 1.3.0 add the reserved idempotency_key track property and the rolling 60-second duplicate window (getformo/sdk#389, #375; getformo/sdk-react-native#89). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Afmyxxipx1ATADBWi2bFGZ * chore(e2e): give each harness batch distinct call values The SDK's queue drops an event whose payload is identical to one accepted within the last minute. getformo/sdk#372 makes that window independent of flush timing, and transaction:started has no batch id yet, so three identical batches in one second lost their later started rows to the guard rather than to anything under test. Distinct values per batch keep every row meaningful on the published 1.38.0 and on the fix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YEjK9qkwf3u1CpGkb5NvYG --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Ports getformo/sdk#375 (merged) and getformo/sdk#389 (open) to the React Native SDK.
Problem
Duplicate suppression was tied to delivery: an event's hash was dropped when its batch was sent. The first event of an app session is sent the moment it arrives, so its hash left with it and an identical
track()right behind it, the double-fire this exists to catch, was accepted. The hash also folded in the minute-truncated timestamp, so a double-fire straddling a minute boundary got two different hashes. And the hash covered the enriched event, so SDK-generated context that changes between two identical calls (screen, app state) made the second call look new.Changes
track()calls are judged on name, properties, caller context, address, and user id, before SDK context is added. Other event types keep the enriched fingerprint minus the volatile generated fields (screen size, network state), so a rotation or a wifi-to-cellular handover between two identical calls does not split them. The wire id is unchanged.message_id. Automatic events (screen, connect, chain, signature, transaction, disconnect, identify, detect) keep the deterministic content-and-minute id, so ingestion counts do not shift on upgrade.idempotency_keyproperty. Same API as the web SDK: a reservedtrack()property, following the convention forvolume,revenue,currency, andpoints. It hashes event type + name + key into a stable id and is lifted out of the properties before the event is built, so the raw key is never sent. Strings and finite numbers are accepted; any other value drops the call with a warning and never throws into the host. The caller's properties object is not mutated. Thetrack()signature is unchanged.hash()is now synchronous so the enqueue path gains no extra microtask yield.Verification
jest: 440 passing (21 new: rolling window, minute boundary, expiry, permanent vs transient failure, wire identity per event type, keyed identity across instances and event names, caller fingerprint, key lifting and validation).tsc --noEmit: clean.eslint: clean.bob build: green.Do not merge before getformo/sdk#389 lands, so both SDKs ship the same API.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Afmyxxipx1ATADBWi2bFGZ
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.