Repository navigation
fix(flags): fall back for incompatible local regex evaluation - #1108
Merged
Merged
Conversation
Contributor
posthog-python Compliance ReportDate: 2026-10-23T10:00:01.483498+00:00 ✅ All Tests Passed!121/121 tests passed Capture_V1 Tests✅ 95/95 tests passed View Details
Capture_Ai Tests✅ 5/5 tests passed View Details
Feature_Flags Tests✅ 17/17 tests passed View Details
Feature_Flags_Local_Evaluation Tests✅ 4/4 tests passed View Details
|
Contributor
There was a problem hiding this comment.
PostHog Review · Standard · reviewed 7915272: 2 Should fix.
This is the only automatic review while this pull request is a draft. Comment @posthog review to review new changes sooner. Otherwise, a review of the new changes runs when you mark it ready for review.
marandaneto
marked this pull request as ready for review
October 23, 2026 08:42
ioannisj
approved these changes
Oct 23, 2026
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
Python's regex engine can return a different result from the flags service even when a pattern compiles. Python matches
^foo$against"foo\n", while the service does not. Python also treats^[a&&b]$differently and rejects Unicode property escapes that the service supports. Definitive local results prevent the existing remote fallback from running.This adapts the agreed approach in posthog-js#5354 and its cross-SDK discussion to Python
re. It does not depend on that PR being merged or released.regexandnot_regex. Do not invert unsupported patterns or failures into matches, including through cohort negation.Review feedback updates
Tightening
$can turn a fast successful native search into expensive rejection. The reviewed^(a+)+$case with 27acharacters and a final newline took about 2.86 seconds before these updates. Quantified groups containing another quantifier now defer before native matching.A related
^(a|aa)+$case exceeded an owned four-second subprocess limit with 50acharacters and a final newline. Before native search, patterns using an unescaped outside-class$plus quantification or alternation now defer when the input ends in LF. Compilation still happens first, preserving invalid-pattern false/false. Ordinary bounded and quantified alternation controls remain local, as does simple^foo$rejection on a final newline.ASCII inputs now use built-in checks instead of Python-level surrogate and word-class loops. Built-in string membership still catches Python-only ASCII whitespace controls. Unicode safety scans and the empty-pattern early return remain intact.
Pattern classification, anchor translation and compilation now use a private 256-entry standard-library LRU cache. Entries contain immutable pattern metadata or the known-invalid marker, never property values or match results. Unsupported or transient compilation exceptions are not retained. Every per-value LF, Unicode, surrogate and whitespace check still runs outside the cache.
This classifier is deliberately conservative. Some patterns and final-newline inputs now require server evaluation even when a particular local search would be safe. This is not a complete ReDoS detector or a guarantee of equivalence across all Unicode versions, compiler limits or resource budgets. No new engine, timeout architecture, dependency or public option is added.
💚 How did you test it?
Regression tests preceded production edits. The original port had 104 failures before its fix. Review-feedback tests produced 26 missing-deferral/fallback failures before the nested-quantifier change, then 20 before the approved input-sensitive anchor policy. The same commands pass afterward. No wall-clock assertions or regex-engine mocks were added.
On the final signed code, CPython 3.13.13:
uv run --no-sync pytest posthog/test/test_feature_flags_regex.py --timeout=30 -q: 333 passed.unlistedtook a median 2.18ms per call. The same bounded workload on final committed code took about 0.20µs per call (five samples of 100 calls), with the same false result. This is a measured observation, not a timing assertion.^z$benchmark improved from a five-sample median of about 2.42ms per call to about 1.10µs, with the same false result. These are local observations, not throughput guarantees or unit-test thresholds.The required isolated review on the final signed HEAD against
origin/mainreported no actionable findings. An earlier class-union claim was disproved using actual service results:[a||b]matches a pipe character in the service as well as the committed SDK. The final review was required after integrating new main signing documentation. Earlier review feedback and backtracking safeguards are preserved.The full repository suite, other Python runtimes locally, OpenFeature provider, application integration, mirror/release builds and live-service tests were not run. GitHub CI is not claimed green by these local checks.
📝 Checklist
All commits are signed (see Commit signing).
I reviewed the submitted code.
I added tests to verify the changes.
I updated the docs if needed. No new public configuration requires documentation.
No breaking change or entry added to the changelog.
If releasing new changes
sampo addto generate a changeset file.The existing
.sampo/changesets/local-regex-compatibility.mdpatch metadata is retained. The generator was not rerun and no duplicate changeset was added.🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Pi assisted with implementation, test auditing and publication using Git, GitHub CLI, pytest, Ruff, mypy and isolated autoreview. Bounded subprocess probes and actual service results were used to verify performance concerns and review claims. The existing inconclusive/fallback path was retained instead of adding an engine or public API. Human review is required before merge.