Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .sampo/changesets/starts-with-ends-with-operators.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
pypi/posthog: minor
---

Support the `starts_with`, `not_starts_with`, `ends_with`, and `not_ends_with` property filter operators in feature flag local evaluation. Matching is case-insensitive and mirrors `icontains`, so flags using these operators no longer fall back to remote evaluation.
23 changes: 22 additions & 1 deletion posthog/feature_flags.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,16 @@ class ConditionMatch(Enum):

# All operators supported by match_property, grouped by category.
EQUALITY_OPERATORS = ("exact", "is_not", "is_set", "is_not_set")
STRING_OPERATORS = ("icontains", "not_icontains", "regex", "not_regex")
STRING_OPERATORS = (
"icontains",
"not_icontains",
"regex",
"not_regex",
"starts_with",
"not_starts_with",
"ends_with",
"not_ends_with",
)
Comment on lines +61 to +70

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New public API surface (STRING_OPERATORS value, str_istartswith/str_iendswith) not reflected in the generated public API snapshot

must_fix best_practice

Why we think it's a valid issue
  • Checked: the snapshot enforcement chain and whether the PR includes a regenerated snapshot β€” read references/public_api_snapshot.txt, posthog/utils.py (for __all__), .github/workflows/ci.yml, Makefile, AGENTS.md, and counted the PR's changed files.
  • Found: references/public_api_snapshot.txt:667 literally holds the stale value STRING_OPERATORS = ('icontains', 'not_icontains', 'regex', 'not_regex'); posthog/utils.py has no __all__, so every non-underscore name is tracked, and sibling helpers str_icontains/str_iequals already appear at snapshot lines 1165-1166 β€” meaning new public str_istartswith/str_iendswith must be added too. The check is a required CI job: .github/workflows/ci.yml:65 (public-api) runs make public_api_check (line 78), backed by .github/scripts/check_public_api.py and documented in AGENTS.md:47-51.
  • Found: the PR's 5 changed files are fully accounted for by the changeset .md, feature_flags.py, utils.py, and the two test files β€” so references/public_api_snapshot.txt was not regenerated, matching the reviewer's confirmation that it is stale.
  • Impact: the public-api CI job fails deterministically (snapshot vs. actual surface mismatch on the changed STRING_OPERATORS value and the two new functions), blocking merge until make public_api_snapshot is run and committed. Concrete trigger + concrete consequence, directly caused by this PR's public-API changes β€” a real, actionable blocker, not speculative. (Caveat: this checkout is at main, not the PR head fd4fcab, so I validated the mechanism and corroborating metadata rather than re-running the check on the PR diff.)
Issue description

This chunk changes the public value of posthog.feature_flags.STRING_OPERATORS and adds two new public functions, posthog.utils.str_istartswith and posthog.utils.str_iendswith (posthog/utils.py:498-537). The repo enforces its public API surface via a generated snapshot at references/public_api_snapshot.txt, checked by .github/scripts/check_public_api.py and run as a dedicated required CI job (public-api / make public_api_check, wired in .github/workflows/ci.yml). AGENTS.md explicitly documents that contributors must run make public_api_snapshot && make public_api_check whenever the public API surface changes. I ran python .github/scripts/check_public_api.py against this PR's actual head commit (fd4fcab) and confirmed the snapshot is stale: it still shows STRING_OPERATORS = ('icontains', 'not_icontains', 'regex', 'not_regex') (missing the four new operators) and is missing entries for str_iendswith/str_istartswith entirely. This is a verified, concrete CI failure, not a hypothetical one, and it directly reflects the exact public API surface added in this chunk.

Suggested fix

Run make public_api_snapshot (then make public_api_check to confirm) and commit the regenerated references/public_api_snapshot.txt alongside this change, so the public API contract file stays in sync with the new starts_with/ends_with operators and the two new str_istartswith/str_iendswith functions.

Prompt to fix with AI (copy-paste)
## Context
@posthog/feature_flags.py#L61-70

<issue_description>
This chunk changes the public value of `posthog.feature_flags.STRING_OPERATORS` and adds two new public functions, `posthog.utils.str_istartswith` and `posthog.utils.str_iendswith` (posthog/utils.py:498-537). The repo enforces its public API surface via a generated snapshot at `references/public_api_snapshot.txt`, checked by `.github/scripts/check_public_api.py` and run as a dedicated required CI job (`public-api` / `make public_api_check`, wired in `.github/workflows/ci.yml`). AGENTS.md explicitly documents that contributors must run `make public_api_snapshot && make public_api_check` whenever the public API surface changes. I ran `python .github/scripts/check_public_api.py` against this PR's actual head commit (fd4fcab) and confirmed the snapshot is stale: it still shows `STRING_OPERATORS = ('icontains', 'not_icontains', 'regex', 'not_regex')` (missing the four new operators) and is missing entries for `str_iendswith`/`str_istartswith` entirely. This is a verified, concrete CI failure, not a hypothetical one, and it directly reflects the exact public API surface added in this chunk.
</issue_description>

<issue_validation>
- **Checked:** the snapshot enforcement chain and whether the PR includes a regenerated snapshot β€” read `references/public_api_snapshot.txt`, `posthog/utils.py` (for `__all__`), `.github/workflows/ci.yml`, `Makefile`, `AGENTS.md`, and counted the PR's changed files.
- **Found:** `references/public_api_snapshot.txt:667` literally holds the stale value `STRING_OPERATORS = ('icontains', 'not_icontains', 'regex', 'not_regex')`; `posthog/utils.py` has no `__all__`, so every non-underscore name is tracked, and sibling helpers `str_icontains`/`str_iequals` already appear at snapshot lines 1165-1166 β€” meaning new public `str_istartswith`/`str_iendswith` must be added too. The check is a required CI job: `.github/workflows/ci.yml:65` (`public-api`) runs `make public_api_check` (line 78), backed by `.github/scripts/check_public_api.py` and documented in `AGENTS.md:47-51`.
- **Found:** the PR's 5 changed files are fully accounted for by the changeset `.md`, `feature_flags.py`, `utils.py`, and the two test files β€” so `references/public_api_snapshot.txt` was not regenerated, matching the reviewer's confirmation that it is stale.
- **Impact:** the `public-api` CI job fails deterministically (snapshot vs. actual surface mismatch on the changed `STRING_OPERATORS` value and the two new functions), blocking merge until `make public_api_snapshot` is run and committed. Concrete trigger + concrete consequence, directly caused by this PR's public-API changes β€” a real, actionable blocker, not speculative. (Caveat: this checkout is at `main`, not the PR head `fd4fcab`, so I validated the mechanism and corroborating metadata rather than re-running the check on the PR diff.)
</issue_validation>

## Task
Investigate the issue and solve it

<potential_solution>
Run `make public_api_snapshot` (then `make public_api_check` to confirm) and commit the regenerated `references/public_api_snapshot.txt` alongside this change, so the public API contract file stays in sync with the new `starts_with`/`ends_with` operators and the two new `str_istartswith`/`str_iendswith` functions.
</potential_solution>

NUMERIC_OPERATORS = ("gt", "gte", "lt", "lte")
DATE_OPERATORS = ("is_date_before", "is_date_after")
SEMVER_COMPARISON_OPERATORS = (
Expand Down Expand Up @@ -538,6 +547,18 @@ def compute_exact_match(value, override_value):
if operator == "not_icontains":
return not utils.str_icontains(override_value, value)

if operator == "starts_with":
return utils.str_istartswith(override_value, value)

if operator == "not_starts_with":
return not utils.str_istartswith(override_value, value)

if operator == "ends_with":
return utils.str_iendswith(override_value, value)

if operator == "not_ends_with":
return not utils.str_iendswith(override_value, value)

if operator == "regex":
return (
is_valid_regex(str(value))
Expand Down
59 changes: 59 additions & 0 deletions posthog/test/test_feature_flags.py
Original file line number Diff line number Diff line change
Expand Up @@ -4741,6 +4741,65 @@ def test_match_properties_icontains(self):

self.assertFalse(match_property(property_b, {"key": "three"}))

def test_match_properties_starts_with(self):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Duplicated operator test matrix

The starts-with and ends-with tests manually repeat the same positive, negative, numeric-coercion, negation, and missing-key scenarios. Parameterizing this matrix would express the shared behavior once and prevent coverage for the two operator families from drifting apart.

Context Used: Do not attempt to comment on incorrect alphabetica... (source)

Prompt To Fix With AI
This is a comment left during a code review.
Path: posthog/test/test_feature_flags.py
Line: 4744

Comment:
**Duplicated operator test matrix**

The starts-with and ends-with tests manually repeat the same positive, negative, numeric-coercion, negation, and missing-key scenarios. Parameterizing this matrix would express the shared behavior once and prevent coverage for the two operator families from drifting apart.

**Context Used:** Do not attempt to comment on incorrect alphabetica... ([source](https://app.greptile.com/review/custom-context?memory=instruction-0))

---

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

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

property_a = self.property(key="key", value="Val", operator="starts_with")
self.assertTrue(match_property(property_a, {"key": "value"}))
self.assertTrue(match_property(property_a, {"key": "VALUE"}))
self.assertTrue(match_property(property_a, {"key": "vaLue4"}))

self.assertFalse(match_property(property_a, {"key": "prevalue"}))
self.assertFalse(match_property(property_a, {"key": "Alakazam"}))
self.assertFalse(match_property(property_a, {"key": 123}))

property_b = self.property(key="key", value="3", operator="starts_with")
self.assertTrue(match_property(property_b, {"key": "3"}))
self.assertTrue(match_property(property_b, {"key": 323}))

self.assertFalse(match_property(property_b, {"key": 123}))
self.assertFalse(match_property(property_b, {"key": "val3"}))

property_c = self.property(key="key", value="Val", operator="not_starts_with")
self.assertFalse(match_property(property_c, {"key": "value"}))
self.assertFalse(match_property(property_c, {"key": "VALUE"}))

self.assertTrue(match_property(property_c, {"key": "prevalue"}))
self.assertTrue(match_property(property_c, {"key": "Alakazam"}))

with self.assertRaises(InconclusiveMatchError):
match_property(property_a, {"key2": "value"})
with self.assertRaises(InconclusiveMatchError):
match_property(property_a, {})

def test_match_properties_ends_with(self):
property_a = self.property(key="key", value="lUe", operator="ends_with")
self.assertTrue(match_property(property_a, {"key": "value"}))
self.assertTrue(match_property(property_a, {"key": "VALUE"}))
self.assertTrue(match_property(property_a, {"key": "343tfvalue"}))

self.assertFalse(match_property(property_a, {"key": "value2"}))
self.assertFalse(match_property(property_a, {"key": "Alakazam"}))
self.assertFalse(match_property(property_a, {"key": 123}))

property_b = self.property(key="key", value="3", operator="ends_with")
self.assertTrue(match_property(property_b, {"key": "3"}))
self.assertTrue(match_property(property_b, {"key": 323}))
self.assertTrue(match_property(property_b, {"key": 13}))

self.assertFalse(match_property(property_b, {"key": 321}))
self.assertFalse(match_property(property_b, {"key": "3val"}))

property_c = self.property(key="key", value="lUe", operator="not_ends_with")
self.assertFalse(match_property(property_c, {"key": "value"}))
self.assertFalse(match_property(property_c, {"key": "VALUE"}))

self.assertTrue(match_property(property_c, {"key": "value2"}))
self.assertTrue(match_property(property_c, {"key": "Alakazam"}))

with self.assertRaises(InconclusiveMatchError):
match_property(property_a, {"key2": "value"})
with self.assertRaises(InconclusiveMatchError):
match_property(property_a, {})

def test_match_properties_regex(self):
property_a = self.property(key="key", value=r"\.com$", operator="regex")
self.assertTrue(match_property(property_a, {"key": "value.com"}))
Expand Down
4 changes: 4 additions & 0 deletions posthog/test/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,10 @@ def test_regex_datetime_and_case_helpers(self):
assert utils.str_icontains("Hello World", "python") is False
assert utils.str_iequals("Hello World", "hello world") is True
assert utils.str_iequals("Hello World", "hello") is False
assert utils.str_istartswith("Hello World", "HELLO") is True
assert utils.str_istartswith("Hello World", "World") is False
assert utils.str_iendswith("Hello World", "WORLD") is True
assert utils.str_iendswith("Hello World", "Hello") is False

@parameterized.expand(
[
Expand Down
40 changes: 40 additions & 0 deletions posthog/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,46 @@ def str_iequals(value, comparand):
return str(value).casefold() == str(comparand).casefold()


def str_istartswith(source, search):
"""
Check if a string starts with another string, ignoring case.

Args:
source: The string to check
search: The prefix to look for

Returns:
bool: True if source starts with search (case-insensitive), False otherwise

Examples:
>>> str_istartswith("Hello World", "HELLO")
True
>>> str_istartswith("Hello World", "World")
False
"""
return str(source).casefold().startswith(str(search).casefold())


def str_iendswith(source, search):
"""
Check if a string ends with another string, ignoring case.

Args:
source: The string to check
search: The suffix to look for

Returns:
bool: True if source ends with search (case-insensitive), False otherwise

Examples:
>>> str_iendswith("Hello World", "WORLD")
True
>>> str_iendswith("Hello World", "Hello")
False
"""
return str(source).casefold().endswith(str(search).casefold())


def _platform_release():
release = getattr(platform, "release", None)
if callable(release):
Expand Down
Loading