Skip to content

fix(flags): align local string matching with the flags service - #236

Open
marandaneto wants to merge 5 commits into
mainfrom
fix/issue-78019-case-folding
Open

fix(flags): align local string matching with the flags service#236
marandaneto wants to merge 5 commits into
mainfrom
fix/issue-78019-case-folding

Conversation

@marandaneto

@marandaneto marandaneto commented Aug 27, 2026

Copy link
Copy Markdown
Member

💡 Motivation and Context

Local feature flag evaluation did not match the /flags service for several case-insensitive string comparisons. Exact matching used ASCII-only lowercasing, integral floats such as 323.0 lost their decimal suffix, and not_icontains treated a match at offset zero as no match.

This fixes PostHog/posthog#78019 by:

  • using simple Unicode lowercase conversion for exact and is_not, including array membership
  • keeping icontains, prefix, and suffix operators on the existing ASCII-only lowercase path
  • preserving backend-compatible finite float formatting, including integral and scientific values
  • preserving the previous behavior for non-finite PHP floats instead of throwing
  • using strict strpos comparison for not_icontains
  • adding symfony/polyfill-mbstring and symfony/polyfill-iconv so Unicode lowercase conversion works without either native extension, including context-independent sigma and U+0130 expansion semantics

A patch changeset is included for posthog-php.

💚 How did you test it?

  • vendor/bin/phpunit --no-coverage test/FeatureFlagLocalEvaluationTest.php - 120 tests and 2,578 assertions passed
  • vendor/bin/phpunit --no-coverage - 479 tests and 3,904 assertions passed
  • vendor/bin/phpcs --warning-severity=0 - no errors
  • composer validate --no-check-publish - valid, with the existing package version warning
  • composer api:check - public API snapshot is up to date
  • Composer resolution and Unicode matching were validated with native ext-mbstring and ext-iconv disabled

PHPUnit reported existing warnings and deprecations, but no test failures.

📝 Checklist

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • I updated the docs if needed.
  • No breaking change or entry added to the changelog.

If releasing new changes

  • Ran pnpm change to generate a change intent file

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Implemented with Pi using the PR and autoreview skills. The reviews identified scientific float formatting, contextual Greek sigma behavior, and the missing MB_CASE_LOWER_SIMPLE constant without native mbstring. These were fixed with regression coverage. Exact matching uses native simple lowercase when available and the polyfill’s context-independent lowercase mapping otherwise, with explicit U+0130 expansion to match the Rust backend.

@marandaneto marandaneto self-assigned this Aug 27, 2026
@github-actions

github-actions Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

posthog-php Compliance Report

Date: 2026-08-27 06:46:44 UTC
Duration: 95318ms

✅ All Tests Passed!

46/46 tests passed


Capture Tests

29/29 tests passed

View Details
Test Status Duration
Format Validation.Event Has Required Fields 13ms
Format Validation.Event Has Uuid 6ms
Format Validation.Event Has Lib Properties 5ms
Format Validation.Distinct Id Is String 6ms
Format Validation.Token Is Present 6ms
Format Validation.Custom Properties Preserved 6ms
Format Validation.Event Has Timestamp 5ms
Retry Behavior.Retries On 503 5316ms
Retry Behavior.Does Not Retry On 400 2009ms
Retry Behavior.Does Not Retry On 401 2010ms
Retry Behavior.Respects Retry After Header 8014ms
Retry Behavior.Implements Backoff 15730ms
Retry Behavior.Retries On 500 5111ms
Retry Behavior.Retries On 502 5115ms
Retry Behavior.Retries On 504 5114ms
Retry Behavior.Max Retries Respected 16529ms
Deduplication.Generates Unique Uuids 11ms
Deduplication.Preserves Uuid On Retry 5113ms
Deduplication.Preserves Uuid And Timestamp On Retry 10315ms
Deduplication.Preserves Uuid And Timestamp On Batch Retry 5117ms
Deduplication.No Duplicate Events In Batch 12ms
Deduplication.Different Events Have Different Uuids 7ms
Compression.Sends Gzip When Enabled 6ms
Batch Format.Uses Proper Batch Structure 6ms
Batch Format.Flush With No Events Sends Nothing 3ms
Batch Format.Multiple Events Batched Together 10ms
Error Handling.Does Not Retry On 403 2008ms
Error Handling.Does Not Retry On 413 2010ms
Error Handling.Retries On 408 5115ms

Feature_Flags Tests

17/17 tests passed

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

@marandaneto
marandaneto requested a review from a team August 27, 2026 05:47
@posthog-project-board-bot posthog-project-board-bot Bot moved this to In Progress in Feature Flags Aug 27, 2026
@marandaneto
marandaneto marked this pull request as ready for review August 27, 2026 05:51
@marandaneto
marandaneto requested a review from a team as a code owner August 27, 2026 05:51
@posthog-project-board-bot posthog-project-board-bot Bot moved this from In Progress to In Review in Feature Flags Aug 27, 2026
@greptile-apps

greptile-apps Bot commented Aug 27, 2026

Copy link
Copy Markdown
Prompt To Fix All With AI
### Issue 1
composer.json:20
**Iconv becomes mandatory**

If a PHP 8.2+ installation lacks `ext-iconv`, requiring `symfony/polyfill-mbstring` makes Composer reject the installation even when native `ext-mbstring` is available, so environments that previously satisfied the SDK's declared requirements can no longer install or upgrade it.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "fix(flags): use simple Unicode lowercase..." | Re-trigger Greptile

Comment thread composer.json Outdated
@github-project-automation github-project-automation Bot moved this from In Review to Approved in Feature Flags Aug 27, 2026
@arnohillen

Copy link
Copy Markdown

valueToString at lib/FeatureFlag.php:593 formats finite floats with json_encode, whose scientific-notation cutovers differ from serde_json's (what the service compares via to_string_representation): floats in [1e16, 1e17) stringify as plain decimals here but exponentially there (1.0e16 -> 10000000000000000.0 vs 1e+16) and floats in [1e-5, 1e-4) stringify exponentially here but as plain decimals there (0.00001 -> 1e-5 vs 0.00001, 9.9e-5 -> 9.9e-5 vs 0.000099), so exact with filter "1e+16" vs property 1.0e16 returns false (service true) and filter "0.00001" vs 0.00001 returns false (service true).

@arnohillen

Copy link
Copy Markdown

valueToString at lib/FeatureFlag.php:596 falls through to strval for array property values, so property [1, 2] stringifies to "Array" (plus an Array to string conversion warning) instead of the service's "[1,2]", making exact with filter "[1,2]" return false where the service returns true, and filter "Array" return true where the service returns false.

@arnohillen

Copy link
Copy Markdown

unicodeLowercase at lib/FeatureFlag.php:580 uses MB_CASE_LOWER_SIMPLE, which skips the Final_Sigma mapping that Rust str::to_lowercase applies in the flags service (rust/feature-flags/src/properties/property_matching.rs:230), so an exact filter "οδος" against property "ΟΔΟΣ" evaluates false locally (reproduced on php:8.2-cli and php:8.4-cli at be566e7, including through matchFeatureFlagProperties) while the service returns true, and is_not inverts the same way; note native MB_CASE_LOWER does apply Final_Sigma on PHP 8.3/8.4 and only lacks it on 8.2, and the new test pins both directions of the gap via the ["ΟΣ", "ος"] assertFalse and the trailing assertTrue for "ΟΣ" vs "οσ" in test/FeatureFlagLocalEvaluationTest.php.

@arnohillen

Copy link
Copy Markdown

computeExactMatch at lib/FeatureFlag.php:558 has no boolean-coercion step, so exact diverges from the flags service whenever the filter value is boolean-ish: filter false or ["false"] against property "banana", 0 or "" returns false here but true in the service (rust/feature-flags/src/properties/property_matching.rs L212-220), and filter ["true","false"] against property "true" returns true here but false there — pre-existing, but this PR is the exact-match alignment change.

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

Labels

None yet

Projects

Status: Approved

Development

Successfully merging this pull request may close these issues.

Case folding and numeric stringification are inconsistent between SDK local evaluation and the flags service

3 participants