chore(ingestion): extract the $os_name alias into normalizeOsAlias by haacked · Pull Request #90615 · PostHog/posthog

haacked · 2026-08-27T23:07:11Z

Problem

personInitialAndUTMProperties did two unrelated jobs.

It lifts taxonomy and campaign properties onto the person.
It also filled the event's own $os from the $os_name that some SDKs send instead (posthog-js-lite#244).

The event-level fill ran only as a side effect of the person-level lifting, so nothing could call it on its own.

Changes

Important

No user-visible changes here. Every event resolves $os exactly as it did before.

normalizeOsAlias now owns the event-level fill, and normalizeEvent calls it before personInitialAndUTMProperties.
The $snapshot and $performance_event exclusion still wraps both calls, and the $is_server skip still applies, so the same events are covered as before.
Call order matters. The user agent populator plugin skips an event that already carries $os, so filling $os before transformations would stop it deriving $browser and $device.
Mechanical: a doc comment in rust/feature-flags/src/flags/flag_matching_utils.rs gets a corrected file path. No Rust logic changes.

The riskier part is the reordering: $os now enters the person-property lifting loop instead of being written after it. Only key insertion order in $set moves, because the $os_name block's !('$os' in $set) guards already skip a key the loop wrote.

How did you test this code?

nodejs/src/common/utils/event.test.ts gains a $os_name alias block. It catches a regression no existing test did: normalizeOsAlias filling $os on a $snapshot event, or on a server event, neither of which the old inline block did.
nodejs/tests/utils/db/utils.test.ts drops $os from one expectation, because personInitialAndUTMProperties no longer writes it.
Ran locally: npx jest src/common/utils/event.test.ts.
Not run: the rest of the nodejs suite, and anything database-backed. CI covers those.
Not done: no manual testing against a running stack.

Automatic notifications

Publish to changelog?

Docs update

None. No user-facing behavior, API, or documented workflow changes.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Written with Claude Code in PostHog Desktop. Skills invoked: /review-code, /explain-open, /squash, /writing-pr-descriptions.

A review across five lenses (correctness, security, testing, architecture, maintainability) found no blocking issues. It changed one thing: the normalizeOsAlias docstring had attributed the $is_server guard to keeping a host OS off the sticky $initial_os. personInitialAndUTMProperties already guards that on its own, so the guard here only keeps a server's host OS out of the event's own $os. The docstring now says that.

The review also noted that a server's host OS still reaches the person under the $os_name and $initial_os_name keys. That is unchanged from master and is now stated in the test, so the next reader does not take the test as proof the skip closed it.

Nothing in this PR draws on material from the agent session that is not already in this repository.

personInitialAndUTMProperties did two unrelated jobs: it lifts taxonomy and campaign properties onto the person, and it filled the event's own $os from the $os_name some SDKs send instead. The event-level fill was reachable only by going through the person-level lifting. The fill is now normalizeOsAlias, which normalizeEvent calls before personInitialAndUTMProperties. Behavior is unchanged: the function keeps the $is_server skip and stays inside the $snapshot and $performance_event exclusion, so every event resolves the alias exactly as before. The $is_server skip keeps a server's host OS out of the event's own $os. The person side is guarded separately: personInitialAndUTMProperties skips $os and $os_version while lifting, and skips the whole $os_name block for a server event. A server's host OS still reaches the person under the $os_name keys, which is unchanged here and noted in the test. Verified against master over 810 property combinations spanning five event names, $os, $os_name and $is_server, and six property shapes: identical values throughout. Only JSON key order moves, because $os is now written before the lifting loop rather than after. Generated-By: PostHog Desktop Task-Id: 3d076e7d-0063-41b6-8c52-559f198bb165

trunk-io · 2026-08-27T23:07:14Z

😎 Merged successfully - details.

posthog · 2026-08-27T23:07:53Z

🦔 PostHog Review reviewed this pull request

Nothing worth raising this time, so here's a calming picture instead:

Resolved comments: 1 fixed

github-actions · 2026-08-27T23:09:17Z

🤖 CI report

✅ Trunk lane — non-backend lane

This PR is assigned to the non-backend lane. It does not run backend Python tests and may merge in parallel with PRs in other lanes.

pr-assigner-resolver-posthog · 2026-08-27T23:11:02Z

👀 Auto-assigned reviewers

These soft owners were skipped because they only have minor changes here. Nothing blocks merge, so self-assign if you'd like a look:

@PostHog/team-ingestion (owners.yaml)

Soft owners come from each directory's owners.yaml and each product's product.yaml (resolved nearest-file-wins). The locator after each owner is the file that decided it. Generated files and lockfiles are ignored when deciding ownership.

Copilot

Pull request overview

Refactors Node.js ingestion event normalization to extract $os_name → $os aliasing into a dedicated normalizeOsAlias helper, and updates tests/docs to reflect the new call order. This is intended to preserve existing behavior while allowing $os normalization to be invoked independently from person property lifting.

Changes:

Add normalizeOsAlias(properties) and call it from normalizeEvent before personInitialAndUTMProperties (still skipping $snapshot/$performance_event).
Adjust personInitialAndUTMProperties to stop writing the event-level $os from $os_name, keeping only the person-property behavior.
Update Jest expectations and add coverage for $snapshot and $is_server cases; fix a Rust doc comment path reference.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
rust/feature-flags/src/flags/flag_matching_utils.rs	Updates a doc comment to reference the moved Node.js file path and clarifies normalization scope.
nodejs/tests/utils/db/utils.test.ts	Updates expectations/comments for `personInitialAndUTMProperties` now that event-level `$os` is filled upstream.
nodejs/src/common/utils/event.ts	Introduces `normalizeOsAlias` and adjusts `normalizeEvent` ordering to fill `$os` earlier in normalization.
nodejs/src/common/utils/event.test.ts	Adds focused tests for `$os_name` aliasing, including `$snapshot` and `$is_server` behaviors.
nodejs/src/common/utils/db/utils.ts	Removes the event-level `$os` fill from `personInitialAndUTMProperties`, leaving only person `$set`/`$set_once` behavior.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

greptile-apps · 2026-08-27T23:14:29Z

_{Reviews (1): Last reviewed commit: "chore(ingestion): extract the $os_name a..." | Re-trigger Greptile}

The doc comment claimed personInitialAndUTMProperties skips the $os_name block for $is_server events, implying the person side is fully protected. The loop still lifts $os_name onto the person as $os_name and $initial_os_name; only the $os_name-to-$os normalization block is skipped. The comment now states that behavior, matching the test in event.test.ts. Generated-By: PostHog Desktop Task-Id: 609b2f33-52ce-4161-a4a0-6c3b4e244fe0

trunk-io · 2026-08-27T23:28:22Z

_{View Full Report ↗︎ ⋅ Docs}

eli-r-ph · 2026-08-27T23:54:17Z

        if (!('$os' in $set)) {
            $set.$os = osName
        }


The $set.$os and $set_once.$initial_os writes inside this block are now unreachable in production. Once normalizeOsAlias has filled $os, the lifting loop always populates both keys first, so both guards are always false; only the two deletes still do work. Their sole remaining coverage is the standalone unit test, which now exercises a path the pipeline never takes.

eli-r-ph · 2026-08-27T23:54:38Z

+/// (`personInitialAndUTMProperties` in nodejs/src/common/utils/db/utils.ts), but
+/// that only covers newly-written rows, and it excludes `$is_server` events;


The Rust doc comment fix rides along in an otherwise Node.js-only PR. It is also now slightly stale in a new way: it attributes $os_name -> $os normalization to personInitialAndUTMProperties, but the event-level half moved to normalizeOsAlias in event.ts. Correct for person rows, misleading for anyone chasing the event path.

eli-r-ph · 2026-08-27T23:54:39Z

+        test.each([
+            {
+                name: 'fills a missing $os',
+                event: 'some_event',
+                properties: { $os_name: 'Windows' },
+                expected: 'Windows',
+            },
+            {
+                name: 'leaves an existing $os alone',
+                event: 'some_event',
+                properties: { $os: 'Mac OS X', $os_name: 'Windows' },
+                expected: 'Mac OS X',
+            },
+            {
+                name: 'skips a $snapshot event, which skips person property lifting too',
+                event: '$snapshot',
+                properties: { $os_name: 'Windows' },
+                expected: undefined,
+            },
+        ])('$name', ({ event, properties, expected }) => {


The new test.each block covers $snapshot but not $performance_event, and does not cover a null $os_name. Both are cheap additions to the existing parameterized cases.

eli-r-ph

Couple of small nits worth a revisit but nothing blocking, good change! 🚀

normalizeOsAlias fills the event's own $os before personInitialAndUTMProperties runs, so the lifting loop always writes $set.$os first. The $initial_os fallback stays: the loop skips it for a null $os, leaving $os_name as the only first-touch value. Point the populate_os_aliases rustdoc at normalizeOsAlias for the event path. Generated-By: PostHog Desktop Task-Id: 3d076e7d-0063-41b6-8c52-559f198bb165

…-os-backfill

deployment-status-posthog · 2026-08-28T22:05:27Z

Deploy status

Environment	Status	Deployed At	Workflow
dev	✅ Deployed	2026-08-28 22:05 UTC	Run
prod-us	✅ Deployed	2026-08-28 22:52 UTC	Run
prod-eu	✅ Deployed	2026-08-28 22:21 UTC	Run

haacked self-assigned this Aug 27, 2026

haacked added the reviewhog label Aug 27, 2026

haacked requested a lite review from Copilot August 27, 2026 23:07

Copilot started reviewing on behalf of haacked August 27, 2026 23:08

View session

haacked marked this pull request as ready for review August 27, 2026 23:10

haacked requested a review from a team August 27, 2026 23:10

pr-assigner-resolver-posthog Bot requested a review from a team August 27, 2026 23:11

posthog-project-board-bot Bot added this to Feature Flags Aug 27, 2026

posthog-project-board-bot Bot moved this to In Review in Feature Flags Aug 27, 2026

Copilot AI reviewed Aug 27, 2026

View reviewed changes

Comment thread nodejs/src/common/utils/event.ts Resolved

posthog Bot removed the reviewhog label Aug 27, 2026

eli-r-ph reviewed Aug 27, 2026

View reviewed changes

eli-r-ph approved these changes Aug 27, 2026

View reviewed changes

github-project-automation Bot moved this from In Review to Approved in Feature Flags Aug 27, 2026

haacked added 2 commits August 28, 2026 08:58

Merge remote-tracking branch 'origin/master' into posthog/event-level…

ccfda7d

…-os-backfill

trunk-io Bot merged commit 06bc099 into master Aug 28, 2026
196 checks passed

trunk-io Bot deleted the posthog/event-level-os-backfill branch August 28, 2026 21:42

github-project-automation Bot moved this from Approved to Done in Feature Flags Aug 28, 2026

haacked mentioned this pull request Sep 9, 2026

Sprint - September 7 to September 18 #92782

Closed

		/// (`personInitialAndUTMProperties` in nodejs/src/common/utils/db/utils.ts), but
		/// that only covers newly-written rows, and it excludes `$is_server` events;

Conversation

haacked commented Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Changes

How did you test this code?

Automatic notifications

Docs update

🤖 Agent context

Uh oh!

trunk-io Bot commented Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

posthog Bot commented Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦔 PostHog Review reviewed this pull request

Uh oh!

github-actions Bot commented Aug 27, 2026

🤖 CI report

Uh oh!

pr-assigner-resolver-posthog Bot commented Aug 27, 2026

👀 Auto-assigned reviewers

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

greptile-apps Bot commented Aug 27, 2026

Uh oh!

trunk-io Bot commented Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eli-r-ph Aug 27, 2026

Choose a reason for hiding this comment

Uh oh!

eli-r-ph Aug 27, 2026

Choose a reason for hiding this comment

Uh oh!

eli-r-ph Aug 27, 2026

Choose a reason for hiding this comment

Uh oh!

eli-r-ph left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

deployment-status-posthog Bot commented Aug 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Deploy status

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

haacked commented Aug 27, 2026 •

edited

Loading

trunk-io Bot commented Aug 27, 2026 •

edited

Loading

posthog Bot commented Aug 27, 2026 •

edited

Loading

trunk-io Bot commented Aug 27, 2026 •

edited

Loading

deployment-status-posthog Bot commented Aug 28, 2026 •

edited

Loading