fix: preserve null array positions in event serialization - #770
Draft
marandaneto wants to merge 3 commits into
Draft
fix: preserve null array positions in event serialization#770marandaneto wants to merge 3 commits into
marandaneto wants to merge 3 commits into
Conversation
Contributor
posthog-android Compliance ReportDate: 2026-09-09 10:03:46 UTC ✅ All Tests Passed!46/46 tests passed Capture Tests✅ 29/29 tests passed View Details
Feature_Flags Tests✅ 17/17 tests passed View Details
|
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
Event serialization currently removes null array entries, shifting the remaining values. Preserve those positions while continuing to omit null-valued object members recursively, as required by PostHog/sdk-specs#60.
Only
PostHogEventencoding opts into this behavior in the shared core Gson serializer. Event disk persistence and batch delivery use the same fix, including restored events and properties added by before-send hooks. Java and Kotlin public inputs, caller-owned data, empty objects, other scalar values, and existing envelope metadata rules stay unchanged. General maps, log disk records, and server feature-flag caches retain their previous behavior. No wrapper serializer or dependency update is added. The server wire snapshot now expects the preserved null positions in its ordered list and array. Unsupported NaN values remain omitted.💚 How did you test it?
javap -publicoutput for all 191 core and 27 server classes listed in the committed API surfaces. No signature changes.git diff --checkpassed.47bdf22084f2c5e0ef17858beaef1f2a4357d683, based on maind4a3a0387ab232414de887152aa27e4f5cb13b54. No actionable findings.Local validation limits: These are Kotlin CLI 2.4.10 checks with language/API compatibility 2.0 and JVM target 1.8 on Corretto 21, not the pinned Kotlin 2.1.21 Gradle release toolchain. Full Makefile/Gradle suites and Kotlin metadata/API-validator checks were not run locally. Android device/instrumentation, replay runtime, real process restart/crash, old QueueFile migration, and exhaustive custom-converter/cycle cases remain unvalidated. A fresh queue object is not a process restart. Dedicated AI/generated flag-event wire cases were not run. This draft is for human review, not merge or release clearance.
The previous Build & Test run failed because the server wire snapshot still expected null array slots to be removed. Reproduced that exact failure locally, updated only the two expected null slots, and reran all 18 focused tests successfully. Hosted CI for
47bdf22084f2c5e0ef17858beaef1f2a4357d683passed all 16 checks, including Build & Test, SDK Compliance Tests, CodeQL, Semgrep, Wiz, wrapper validation, changeset hygiene, and PR title validation. Build & Test completedmake compileand the release-task/dependency-lock check successfully. The existing compliance harness does not establish this null-property contract.📝 Checklist
If releasing new changes
pnpm changesetto generate a changeset fileThe previously reviewed patch changeset for core, Android, and server is retained. No package-manager setup or release command was run for this PR.
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Implemented and validated with Pi coding agents, Git, Kotlin/JUnit, ktlint, and an isolated Pi autoreview helper. The human-directed scope keeps the fix in shared event serialization and preserves unrelated log/cache behavior. The private agent session is not publicly shareable. Human review is required before merge.