Repository navigation
fix: skip the person-properties hash for maybe-personless updates and fill $lib as a pair - #1050
Merged
turnipdabeets merged 3 commits intoNov 10, 2026
Conversation
… fill $lib as a pair The hash guard read only the process_person_profile option, but a registered or legacy $process_person_profile false is hoisted into options later, at send. Such a $set recorded a hash it couldn't apply, so the same update sent after unregistering was dropped as a duplicate. Resolve the setting the way it's sent (non-null option, then the legacy property) and record a hash only when it's missing or a Boolean true. Fill the SDK's $lib and $lib_version as a pair after the caller's properties, per sdk-specs #146: another library's $lib never gets this SDK's version, and a $lib_version without a $lib is replaced. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
PostHog Review · Standard · reviewed 3dffbef: 1 Should fix.
See this review and its findings in PostHog Review. Comment @posthog review --deep for a Deep review. A Deep review takes longer and looks deeper than a Standard review.
… update An update whose setting isn't a clear Boolean, such as the string "true", skipped the hash but left the previous one in place. PostHog may still apply it, so a later update back to the previous properties was dropped as a duplicate. Clear the stored hash instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…-lib-pair # Conflicts: # MIGRATION.md
Contributor
posthog-ios Compliance ReportDate: 2026-11-09T22:30:54.496476+00:00 ✅ All Tests Passed!234/234 tests passed Capture_V1 Tests✅ 106/106 tests passed View Details
Capture_Ai_V1 Tests✅ 111/111 tests passed View Details
Feature_Flags Tests✅ 17/17 tests passed View Details
|
This was referenced Nov 10, 2026
dustinbyrne
approved these changes
Nov 10, 2026
turnipdabeets
merged commit Nov 10, 2026
0a5e765
into
chore/v4-remove-deprecated
42 of 46 checks passed
This was referenced Nov 10, 2026
Merged
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.
💡 Motivation and Context
Addresses dustinbyrne's two review comments on #873.
Person-properties hash (comment). The dedup guard in
queueEventread onlyoptions["process_person_profile"]. A registered or legacy$process_person_profile: falseis still a property at that point and becomes an option only incaptureV1JSON. So this sequence recorded a hash for a$setthe server can't apply, then dropped the next, valid update as a duplicate:maindoesn't have this bug, because it overwrote$process_person_profilewith the SDK's own state after the caller merge. The guard now reads the setting the way it's sent (a non-null option, then the legacy property). It records a hash only when the setting is missing or a Booleantrue, so string"false"and any value the server might read as false skip the hash too.queueEventruns afterbeforeSend, so a legacy false set by the hook is covered as well.$lib/$lib_version(comment). This follows "Events name their own library" in sdk-specs #146. The SDK fills its own pair last, after the caller's properties and beforebeforeSend:$libis missing or empty, the SDK sets both, replacing a lone$lib_version.$libis the SDK's own name and$lib_versionis missing, the SDK sets the version.$libnever gets this SDK's version.Before this change the two keys were filled independently. Wrappers are unaffected because they set
postHogSdkName, which counts as "own name".The 4.0 changeset and
MIGRATION.md(Property precedence) describe the new$libbehavior.💚 How did you test it?
PostHogCaptureV1Test:$setdoesn't suppress a later identical one: registered false, string"false"option, legacy false under a null optiontrueoption beats a legacy false, so the later update is still deduplicated$libpair cases, including the three chore: screenshot masking #146 scenarios$libcases and all 3 personless cases).make test filter=PostHogCaptureV1Testpasses. In fullmake test, everything passes exceptPostHogStorageTest's App Group cases. Those fail identically on the base branch on my machine, because of a permission error in~/Library/Group Containers.make buildSdkSpmandmake buildSdkSpmNoTraitspass.make buildSdkcouldn't run locally (xcprettyisn't installed). SwiftLint is clean on the changed lines. No public API change.📝 Checklist
If releasing new changes
pnpm changesetto generate a changeset file (updated the existing 4.0 changeset)🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Claude Code (Opus 5.5) checked both review comments against
mainand the PR head, read the #146 requirement and capture'scoerce_bool, and implemented the fixes. The hash check is conservative on purpose: it doesn't copy the server's bool parsing, and it records a hash only for a missing setting or a Booleantrue. The only cost when a value is ambiguous is one repeated$set.🤖 Generated with Claude Code