Skip to content

fix: omit null object properties from serialized events - #319

Draft
marandaneto wants to merge 1 commit into
mainfrom
fix/null-property-serialization-sqq3ojkm
Draft

fix: omit null object properties from serialized events#319
marandaneto wants to merge 1 commit into
mainfrom
fix/null-property-serialization-sqq3ojkm

Conversation

@marandaneto

@marandaneto marandaneto commented Sep 9, 2026

Copy link
Copy Markdown
Member

💡 Motivation and Context

Event properties currently retain null-valued object members on the wire. This implements the recursive omission rule in sdk-specs PR #60 without narrowing supported public inputs.

The core event serializer and immediate capture boundary now omit those members after enrichment and hooks. Arrays keep their null positions. Empty objects, other scalar values, ordered duplicate and case-distinct property names, and caller-owned data are preserved. Generic JSON serialization for flags and caches is unchanged. Only the root $exception_list on $exception keeps its existing typed metadata semantics.

AI events use the same core serializer, with no AI-specific cleanup. A patch changeset selects only PostHog, where the production change lives. No versions or dependencies are changed.

💚 How did you test it?

  • 35 selected core tests passed, including actual client capture, exception capture, before-send drops/enrichment, intercepted plain/gzip batch bytes and immediate Identify HTTP contents.
  • Eight boundary cases passed against the netstandard2.1 library through the legacy test target. Coverage includes custom converters, POCO/DOM inputs, null array positions, duplicate keys, numeric token precision, generic serialization and caller nonmutation.
  • One actual nonstreaming OpenAI handler/context test passed through the core HTTP-content boundary. Both provider responses and SDK delivery use in-process fake transports. No live ingestion or provider requests were made.
  • Core netstandard2.0, netstandard2.1 and net8.0 builds passed with public API analyzers enabled and zero warnings/errors. Changed-file whitespace/style checks passed, with two formatter workspace-loading warnings.
  • Isolated committed-branch autoreview passed at 1886b0f5d701982624d5a2fd57111318a2d7ada8 against eadb15eaa78ee86469f9841eb23b8b14770b9e96, with no findings.
  • All 19 registered CI checks passed on that HEAD, including build/package, Test, Format, SDK compliance, CodeQL, Semgrep and Wiz checks. The existing SDK compliance harness is not evidence of the new null-property contract.

Validation limits: Tests ran on .NET 9.0.19 with major-version roll-forward, not actual .NET 3.1/8 runtimes. netstandard2.0 has build-only evidence. The preexisting ReadOnlyDictionary converter stack overflow remains deferred and is not newly declared unsupported. The full suite, minimum-runtime matrix, AI streaming/error paths, retries, every immediate API and performance were not validated locally. This SDK has no disk-backed event queue. These gaps remain open for human review and are not cleared merge or release gates.

📝 Checklist

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • I updated the docs if needed. The shared specification is linked above.
  • No breaking change or entry added to the changelog.

If releasing new changes

  • Ran pnpm changeset to generate a changeset file

A core-only patch changeset was written directly. The package-manager command was not run.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Pi agents assisted with implementation, focused validation and review using Git, the .NET CLI, the isolated autoreview helper and GitHub CLI. @marandaneto directed the work and is the DRI. Session details remain private.

The change stays in the shared core serializer rather than duplicating policy in AI. Ordered JsonElement traversal replaced an earlier mutable-object approach that rejected supported duplicate/case-distinct keys. This is a draft for human review, not release approval.

@marandaneto marandaneto self-assigned this Sep 9, 2026
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

posthog-dotnet Compliance Report

Date: 2026-09-09 08:56:51 UTC
Duration: 4486ms

✅ All Tests Passed!

17/17 tests passed


Feature_Flags Tests

17/17 tests passed

View Details
Test Status Duration
Request Payload.Request With Person Properties Device Id 294ms
Request Payload.Flags Request Uses V2 Query Param 129ms
Request Payload.Flags Request Hits Flags Path Not Decide 110ms
Request Payload.Flags Request Omits Authorization Header 108ms
Request Payload.Token In Flags Body Matches Init 109ms
Request Payload.Groups Round Trip 113ms
Request Payload.Groups Default To Empty Object 109ms
Request Payload.Disable Geoip False Propagates As Geoip Disable False 108ms
Request Payload.Disable Geoip Omitted Defaults To False 109ms
Request Payload.Flag Keys To Evaluate Contains Only Requested Key 111ms
Request Lifecycle.No Flags Request On Init Alone 4ms
Request Lifecycle.No Flags Request On Normal Capture 115ms
Request Lifecycle.Two Flag Calls Produce Two Remote Requests 213ms
Request Lifecycle.Mock Response Value Is Returned To Caller 111ms
Retry Behavior.Retries Flags On 502 1115ms
Retry Behavior.Retries Flags On 504 1113ms
Side Effect Events.Get Feature Flag Captures Feature Flag Called Event 212ms

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant