Skip to content

fix(flags): match null properties for string operators like the flags service - #5307

Merged
marandaneto merged 7 commits into
PostHog:mainfrom
quotentiroler:fix/null-string-operators
Oct 21, 2026
Merged

marandaneto merged 7 commits into
PostHog:mainfrom
quotentiroler:fix/null-string-operators

Conversation

@quotentiroler

@quotentiroler quotentiroler commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Problem

Local flag evaluation in @posthog/core returned false for every operator except is_not and is_set when the property value was null. The flags service matches null as the string "null" for string operators (rust/feature-flags/src/properties/property_matching.rs, to_string_representation), and the sdk-specs local-feature-flag-evaluator spec requires the same canonical stringification. So local and remote evaluation disagreed for an explicit null property:

Operator Value Flags service Before
not_icontains x true false
not_starts_with x true false
not_ends_with x true false
not_regex x true false
icontains nul true false
regex ^nu true false

Changes

icontains, not_icontains, starts_with, not_starts_with, ends_with, not_ends_with, regex and not_regex now pass the null guard. The existing string helpers already turn null into "null", so no other logic changed. Numeric, date and semver operators still return false for null, which matches the service.

@posthog/convex evaluates through the same matchFeatureFlagProperty, so it gets the fix too.

Release info Sub-libraries affected

Libraries affected

  • All of them
  • posthog-js (web)
  • posthog-js-lite (web lite)
  • posthog-node
  • posthog-react-native
  • @posthog/react-native-plugin
  • @posthog/react
  • @posthog/ai
  • @posthog/convex
  • @posthog/next
  • @posthog/nextjs-config
  • @posthog/nuxt
  • @posthog/openfeature-node-provider
  • @posthog/openfeature-web-provider
  • @posthog/rollup-plugin
  • @posthog/webpack-plugin
  • @posthog/types
  • @posthog/browser-common

Checklist

  • Tests for new code
  • Accounted for the impact of any changes across different platforms
  • Accounted for backwards compatibility of any changes (no breaking changes!)
  • Took care not to unnecessarily increase the bundle size

If releasing new changes

  • Ran pnpm changeset to generate a changeset file

🤖 Agent context

Autonomy: Human-driven (agent-assisted). DRI: @quotentiroler (an outside contributor can't be set as assignee).

  • Found and fixed with Claude Code; I reviewed and tested the change.
  • The divergence came out of running one set of property-matching cases through every server SDK and the flags service's own matcher, compiled from rust/feature-flags. Every expected value in the new test was checked against that matcher.
  • Verified in a Linux node:22 container on a fresh clone: featureFlagLocalEvaluation.spec.ts passes (116 tests), tsc --noEmit and oxlint are clean, and oxfmt 0.65 reports no changes. With the fix reverted, the 6 divergent cases fail. The full core suite's only failures are src/traces/index.spec.ts Retry-After timing tests, which also fail on unpatched main in the same environment.
  • The changeset was written by hand in the pnpm changeset format.

Maintainer follow-up

The PR commits have been signed without changing their contents or original authorship. Latest main was merged without rewriting history. Separate test-only commits update the three Node assertions and two Convex assertions that still expected the old null behavior and remove their stale null-guard comments. The null guard now consults one list containing exact, is_not and the eight string/regex operators. The redundant is_set entry and separate equality exceptions have been removed without changing behavior.

Verified on Node 24.18.1 after building the workspace dependencies:

  • Reproduced all three Node failures and both Convex failures before updating the expectations.
  • pnpm --filter posthog-node test:unit --coverage.enabled=false: 1,166 tests passed.
  • pnpm --filter posthog-node test:edge-compat: 1,166 tests passed.
  • pnpm --filter @posthog/convex test:unit: 198 tests passed.
  • Core local-evaluator and versioned matching suites: 200 tests passed.
  • Null-guard cleanup: 3,360 before/after comparisons produced identical values, errors and warnings.
  • Scoped Oxlint, Oxfmt and git diff --check passed.
  • Branch autoreview against origin/main found no actionable issues at fc21b21dedfa21cd3e8d394e5ab54de17d94e895.

This is API-compatible, but explicit-null local flag decisions can change. Missing properties and undefined retain their existing behavior.

Related ports of the same fix:

@quotentiroler
quotentiroler requested a review from a team as a code owner October 15, 2026 19:55
@marandaneto
marandaneto requested a review from a team October 21, 2026 09:12
@marandaneto

Copy link
Copy Markdown
Member

thanks for the pr @quotentiroler

@marandaneto

Copy link
Copy Markdown
Member

i think we need this fix in python, go and ruby, i will follow up

quotentiroler and others added 2 commits October 21, 2026 11:39
… service

matchFeatureFlagProperty returned false for every operator outside
is_not/is_set when the property was null. The flags service matches null
as the string "null" for the string operators
(rust/feature-flags property_matching.rs, to_string_representation), as
the sdk-specs local-feature-flag-evaluator requires, so negated operators
disagreed: not_icontains, not_starts_with, not_ends_with and not_regex on
null returned false where the service returns true, and icontains "nul"
and regex "^nu" missed a match.

icontains, not_icontains, starts_with, not_starts_with, ends_with,
not_ends_with, regex and not_regex now pass the null guard; the existing
string helpers already stringify null as "null". Numeric, date and semver
operators keep returning false for null, which the service also does.
@marandaneto
marandaneto force-pushed the fix/null-string-operators branch from be3e315 to c8a38e5 Compare October 21, 2026 09:39
@marandaneto

Copy link
Copy Markdown
Member

Neither was a dedicated null-string-matching PR. The null behavior follows from those broader requirements. sdk-specs PostHog/sdk-specs#49 was specifically about presence checks (is_set / is_not_set), which is a separate issue.

Copy link
Copy Markdown
Contributor

I reviewed the logic. It agrees with property_matching.rs in the flags service. I have some small notes. They do not stop the merge.

  1. undefined with not_* operators. Locally, an undefined value gives false for not_icontains, not_starts_with, not_ends_with and not_regex. When the SDK sends the same properties to the server, JSON removes undefined. Then the key is missing, and the service returns true for these operators. This difference was there before this PR, so it can be a separate follow-up.
  2. is_set in NULL_VALUES_ALLOWED_OPERATORS has no effect. matchFeatureFlagProperty returns for is_set before the null guard. Also, the guard checks exact and is_not separately from the list. One list with all of the allowed operators would be clearer. This is only cosmetic.
  3. JS and Rust regex engines. For some patterns (for example lookarounds, flags and Unicode classes), JS RegExp and the Rust regex engine give different results. This was there before this PR and is not related to it.

@marandaneto

Copy link
Copy Markdown
Member

Addressed point 2 in fc21b21. The null guard now uses one list containing exact, is_not, and the eight string/regex operators. Removed the redundant is_set entry and separate equality exceptions. Verified identical values, errors, and warnings across 3,360 before/after comparisons. Core, Node, edge-runtime, and Convex tests pass locally. Points 1 and 3 are unchanged by this cleanup.

@marandaneto

marandaneto commented Oct 21, 2026 •

Copy link
Copy Markdown
Member

I reviewed the logic. It agrees with property_matching.rs in the flags service. I have some small notes. They do not stop the merge.

  1. undefined with not_* operators. Locally, an undefined value gives false for not_icontains, not_starts_with, not_ends_with and not_regex. When the SDK sends the same properties to the server, JSON removes undefined. Then the key is missing, and the service returns true for these operators. This difference was there before this PR, so it can be a separate follow-up.
  2. is_set in NULL_VALUES_ALLOWED_OPERATORS has no effect. matchFeatureFlagProperty returns for is_set before the null guard. Also, the guard checks exact and is_not separately from the list. One list with all of the allowed operators would be clearer. This is only cosmetic.
  3. JS and Rust regex engines. For some patterns (for example lookarounds, flags and Unicode classes), JS RegExp and the Rust regex engine give different results. This was there before this PR and is not related to it.
  1. will open follow up pr
  2. fixed
  3. is a known issue, i think @haacked created an issue for that stating the difference between regexes, but i dont find it right now - edit Case folding and numeric stringification are inconsistent between SDK local evaluation and the flags service posthog#78019 i think

@marandaneto
marandaneto added this pull request to the merge queue Oct 21, 2026
Merged via the queue into PostHog:main with commit 189da75 Oct 21, 2026
71 of 73 checks passed
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.

3 participants