Skip to content

Commit cd0bb33

Browse files
authored
feat(experiments/mcp): Event search for shared metrics (#67283)
1 parent fe36c7f commit cd0bb33

13 files changed

Lines changed: 197 additions & 40 deletions

File tree

ee/clickhouse/views/experiment_saved_metrics.py

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
1+
from django.db.models import QuerySet
12
from django.db.models.functions import Lower
23

3-
from drf_spectacular.utils import extend_schema
4+
from drf_spectacular.utils import OpenApiParameter, extend_schema, extend_schema_view
45
from rest_framework import filters, serializers, viewsets
56

67
from posthog.api.routing import TeamAndOrgViewSetMixin
@@ -9,7 +10,7 @@
910
from posthog.rbac.user_access_control import UserAccessControlSerializerMixin
1011

1112
from products.experiments.backend.experiment_saved_metric_service import ExperimentSavedMetricService
12-
from products.experiments.backend.metric_utils import refresh_action_names_in_metric
13+
from products.experiments.backend.metric_utils import filter_metric_group_ids_by_event, refresh_action_names_in_metric
1314
from products.experiments.backend.models.experiment import ExperimentSavedMetric, ExperimentToSavedMetric
1415

1516
from ee.api.rbac.access_control import AccessControlViewSetMixin
@@ -137,14 +138,50 @@ def _build_service(self) -> ExperimentSavedMetricService:
137138
return ExperimentSavedMetricService(team=self.context["get_team"](), user=request.user)
138139

139140

141+
@extend_schema_view(
142+
list=extend_schema(
143+
parameters=[
144+
OpenApiParameter(
145+
name="event",
146+
type=str,
147+
location=OpenApiParameter.QUERY,
148+
required=False,
149+
description="Filter to shared metrics whose query references this event name. Matches events "
150+
"used directly in metric queries as well as events behind any actions those metrics reference. "
151+
"Use this for reuse discovery (find a metric by what it measures); distinct from 'search', "
152+
"which matches the metric's own name/description/tags.",
153+
),
154+
],
155+
),
156+
)
140157
@extend_schema(extensions={"x-swagger-tag": "experiment_saved_metrics", "x-product": "experiments"})
141158
class ExperimentSavedMetricViewSet(TeamAndOrgViewSetMixin, AccessControlViewSetMixin, viewsets.ModelViewSet):
142159
scope_object = "experiment_saved_metric"
143160
queryset = ExperimentSavedMetric.objects.prefetch_related("created_by").order_by(Lower("name")).distinct()
144161
serializer_class = ExperimentSavedMetricSerializer
145162
filter_backends = [filters.SearchFilter]
163+
# `search` matches the metric's own name/description/tags, while `event` looks in metrics
146164
search_fields = ["name", "description", "tagged_items__tag__name"]
147165

166+
def safely_get_queryset(self, queryset: QuerySet) -> QuerySet:
167+
# `?event=` matches metrics whose query references the event directly (EventsNode) or via an action's
168+
# step events (ActionsNode resolved live) — mirrors the experiments-list filter. Scoped to `list` so it
169+
# never narrows a detail/update/destroy lookup. Rows are read team-scoped (safely_get_queryset runs
170+
# before the mixin's team filter), then matched in Python since event references live deep in the JSON.
171+
if self.action != "list":
172+
return queryset
173+
event = self.request.query_params.get("event")
174+
if event:
175+
# One group per saved metric: (pk, [its query]). `.order_by()` drops the class-level
176+
# ordering for this internal fetch — the result is matched in Python, so ordering is
177+
# irrelevant, and it avoids a needless sort (and the DISTINCT+ORDER BY column injection).
178+
groups = [
179+
(pk, [query] if query else [])
180+
for pk, query in queryset.filter(team_id=self.team.pk).order_by().values_list("pk", "query")
181+
]
182+
queryset = queryset.filter(pk__in=filter_metric_group_ids_by_event(groups, event, self.team))
183+
return queryset
184+
148185
def perform_destroy(self, instance: ExperimentSavedMetric) -> None:
149186
service = ExperimentSavedMetricService(team=self.team, user=self.request.user)
150187
service.delete_saved_metric(instance)

ee/clickhouse/views/test/test_experiment_saved_metrics.py

Lines changed: 72 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,9 @@
33
from parameterized import parameterized
44
from rest_framework import status
55

6+
from products.actions.backend.models.action import Action
67
from products.experiments.backend.experiment_saved_metric_service import ExperimentSavedMetricService
7-
from products.experiments.backend.models.experiment import Experiment, ExperimentToSavedMetric
8+
from products.experiments.backend.models.experiment import Experiment, ExperimentSavedMetric, ExperimentToSavedMetric
89

910
from ee.api.test.base import APILicensedTest
1011

@@ -981,3 +982,73 @@ def test_list_paginates_with_limit_and_offset(self) -> None:
981982
ids_page1 = {row["id"] for row in page1["results"]}
982983
ids_page2 = {row["id"] for row in page2["results"]}
983984
self.assertEqual(ids_page1 & ids_page2, set())
985+
986+
def _seed_metrics_for_event_filter(self) -> None:
987+
# Created via the ORM so the stored query JSON is exactly what the filter inspects. The action fires
988+
# on the "signup" event, so its metric is discoverable by that event — not by the action's label.
989+
signup_action = Action.objects.create(team=self.team, name="Completed signup", steps_json=[{"event": "signup"}])
990+
for name, source in [
991+
("Pageview mean", {"kind": "EventsNode", "event": "$pageview"}),
992+
("Purchase mean", {"kind": "EventsNode", "event": "purchase"}),
993+
("Signup via action", {"kind": "ActionsNode", "id": signup_action.id, "name": "Completed signup"}),
994+
]:
995+
ExperimentSavedMetric.objects.create(
996+
team=self.team,
997+
created_by=self.user,
998+
name=name,
999+
query={"kind": "ExperimentMetric", "metric_type": "mean", "source": source},
1000+
)
1001+
1002+
@parameterized.expand(
1003+
[
1004+
# `event` matches the events a metric references — directly, or via an action's step events.
1005+
("direct_event", "$pageview", {"Pageview mean"}),
1006+
("another_direct_event", "purchase", {"Purchase mean"}),
1007+
("event_behind_an_action", "signup", {"Signup via action"}),
1008+
# It matches events, not the action's label, nor query structure/type tokens.
1009+
("action_label_not_matched", "Completed signup", set()),
1010+
("metric_type_token_not_matched", "mean", set()),
1011+
("node_kind_token_not_matched", "EventsNode", set()),
1012+
("no_match", "not_an_event", set()),
1013+
]
1014+
)
1015+
def test_event_filter_matches_referenced_events_only(
1016+
self, _name: str, event: str, expected_names: set[str]
1017+
) -> None:
1018+
self._seed_metrics_for_event_filter()
1019+
1020+
response = self.client.get(f"/api/projects/{self.team.id}/experiment_saved_metrics/", data={"event": event})
1021+
1022+
self.assertEqual(response.status_code, status.HTTP_200_OK)
1023+
returned = {row["name"] for row in response.json()["results"]}
1024+
self.assertEqual(returned, expected_names)
1025+
1026+
def test_event_filter_composes_with_search(self) -> None:
1027+
# `event` (references) and `search` (name/description/tags) apply through different mechanisms;
1028+
# both must narrow the result set together (AND), not clobber each other.
1029+
purchase_query = {
1030+
"kind": "ExperimentMetric",
1031+
"metric_type": "mean",
1032+
"source": {"kind": "EventsNode", "event": "purchase"},
1033+
}
1034+
for name in ["Alpha purchase", "Beta purchase"]:
1035+
ExperimentSavedMetric.objects.create(team=self.team, created_by=self.user, name=name, query=purchase_query)
1036+
1037+
response = self.client.get(
1038+
f"/api/projects/{self.team.id}/experiment_saved_metrics/?event=purchase&search=Alpha"
1039+
)
1040+
1041+
self.assertEqual(response.status_code, status.HTTP_200_OK)
1042+
returned = {row["name"] for row in response.json()["results"]}
1043+
self.assertEqual(returned, {"Alpha purchase"})
1044+
1045+
def test_event_param_does_not_filter_detail_retrieve(self) -> None:
1046+
# `event` is a list-only filter; it must never narrow a detail lookup into a 404.
1047+
metric_id = self._create_saved_metric("Pageview mean")
1048+
1049+
response = self.client.get(
1050+
f"/api/projects/{self.team.id}/experiment_saved_metrics/{metric_id}?event=not_an_event"
1051+
)
1052+
1053+
self.assertEqual(response.status_code, status.HTTP_200_OK)
1054+
self.assertEqual(response.json()["id"], metric_id)

products/experiments/backend/experiment_service.py

Lines changed: 9 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@
4343
from products.experiments.backend.hogql_queries.base_query_utils import is_threshold_supported_math
4444
from products.experiments.backend.hogql_queries.experiment_metric_fingerprint import compute_metric_fingerprint
4545
from products.experiments.backend.hogql_queries.funnel_validation import FunnelDWValidator
46-
from products.experiments.backend.metric_utils import collect_metric_events_and_action_ids, resolve_action_events
46+
from products.experiments.backend.metric_utils import filter_metric_group_ids_by_event
4747
from products.experiments.backend.models.experiment import (
4848
LEGACY_METRIC_KINDS,
4949
Experiment,
@@ -2658,28 +2658,14 @@ def _experiments_matching_event(self, queryset: QuerySet[Experiment], event: str
26582658
if query:
26592659
saved_queries_by_experiment[experiment_id].append(query)
26602660

2661-
per_experiment: list[tuple[int, set[str], set[int]]] = []
2662-
all_action_ids: set[int] = set()
2663-
for pk, metrics, metrics_secondary in inline_metrics:
2664-
combined: list[dict[str, Any]] = [
2665-
*(metrics or []),
2666-
*(metrics_secondary or []),
2667-
*saved_queries_by_experiment.get(pk, []),
2668-
]
2669-
events, action_ids = collect_metric_events_and_action_ids(combined)
2670-
per_experiment.append((pk, events, action_ids))
2671-
all_action_ids |= action_ids
2672-
2673-
action_events = resolve_action_events(all_action_ids, self.team)
2674-
2675-
matching_ids: list[int] = []
2676-
for pk, events, action_ids in per_experiment:
2677-
resolved = set(events)
2678-
for action_id in action_ids:
2679-
resolved |= action_events.get(action_id, set())
2680-
if event in resolved:
2681-
matching_ids.append(pk)
2682-
return matching_ids
2661+
metric_groups: list[tuple[int, list[dict[str, Any]]]] = [
2662+
(
2663+
pk,
2664+
[*(metrics or []), *(metrics_secondary or []), *saved_queries_by_experiment.get(pk, [])],
2665+
)
2666+
for pk, metrics, metrics_secondary in inline_metrics
2667+
]
2668+
return filter_metric_group_ids_by_event(metric_groups, event, self.team)
26832669

26842670
def filter_experiments_queryset(
26852671
self,

products/experiments/backend/metric_utils.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,3 +202,32 @@ def resolve_action_events(action_ids: set[int], team: Team) -> dict[int, set[str
202202
action.id: {event for event in action.get_step_events() if event}
203203
for action in Action.objects.filter(id__in=action_ids, team=team, deleted=False)
204204
}
205+
206+
207+
def filter_metric_group_ids_by_event(
208+
metric_groups: list[tuple[int, list[dict[str, Any]]]], event: str, team: Team
209+
) -> list[int]:
210+
"""Return the ids of metric groups whose queries reference ``event``, directly or via an action.
211+
212+
Each group is ``(id, [metric_query, ...])`` — a single query for one saved metric, or an experiment's
213+
inline + secondary + attached saved-metric queries. Every referenced action is resolved to its step
214+
events in one batched query across all groups, so an action-based metric is matched by the events its
215+
action fires on (not by the action's stored — possibly stale — name).
216+
"""
217+
per_group: list[tuple[int, set[str], set[int]]] = []
218+
all_action_ids: set[int] = set()
219+
for group_id, metrics in metric_groups:
220+
events, action_ids = collect_metric_events_and_action_ids(metrics)
221+
per_group.append((group_id, events, action_ids))
222+
all_action_ids |= action_ids
223+
224+
action_events = resolve_action_events(all_action_ids, team)
225+
226+
matching_ids: list[int] = []
227+
for group_id, events, action_ids in per_group:
228+
resolved = set(events)
229+
for action_id in action_ids:
230+
resolved |= action_events.get(action_id, set())
231+
if event in resolved:
232+
matching_ids.append(group_id)
233+
return matching_ids

products/experiments/frontend/generated/api.schemas.ts

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

products/experiments/mcp/tools.yaml

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -712,13 +712,23 @@ tools:
712712
title: List shared experiment metrics
713713
description: >
714714
List shared metrics in the current project. Returns paginated results with id, name, description, query,
715-
created_at, updated_at, tags. Use experiment-saved-metrics-retrieve for a single metric by ID, or this
716-
listing to resolve by name.
715+
created_at, updated_at, tags.
716+
717+
718+
To REUSE a metric by what it measures (rather than by its name): pass ?event=<the event you're measuring> to
719+
find metrics that reference it — directly or via an action — then compare each candidate's 'query'
720+
(metric_type + math) against the metric you'd otherwise build. Use 'search' (name/description/tags) only
721+
when the user named a specific metric to attach.
717722
param_overrides:
718723
limit:
719724
cast: string-int
720725
offset:
721726
cast: string-int
727+
event:
728+
description: >
729+
Filter to shared metrics whose query references this event name — matched directly (an EventsNode)
730+
or via the step events of any action the metric references. For finding a reusable metric by what it
731+
measures; then confirm the match against each row's 'query'.
722732
response:
723733
include:
724734
- id

products/experiments/skills/configuring-experiment-analytics/SKILL.md

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -81,16 +81,18 @@ compare on what each metric measures (its `query`), never on its title.
8181
(mean / funnel / ratio / retention) before searching — see Step 2 to confirm the event exists via
8282
`read-data-schema`. You can only recognize a duplicate once you know the concrete event/action,
8383
so this check runs _after_ you've pinned down the event, not before.
84-
2. **List the library and compare each candidate's `query`.** Call `experiment-saved-metrics-list`
85-
and inspect every result's **`query`** field (not just `name`/`description`). A saved metric is a
86-
reuse match when its `query` measures the **same event or action with the same `metric_type`**
87-
(and compatible `math`) as the metric you'd otherwise build — even if its name is
88-
different.
89-
- **Match locally, not via `search`.** `search` matches only `name` / `description` / tags —
90-
never the underlying event or action — so it cannot find a definition match, and an empty
91-
result means nothing here. Page through the full library with `limit`/`offset` and compare each
92-
row's `query` yourself. (Use `search` only when the user names a specific saved metric to
93-
attach — that's name resolution, not a definition match.)
84+
2. **Search by the event, then compare each candidate's `query`.** Call `experiment-saved-metrics-list`
85+
with `?event=<the event you're measuring>` to find metrics that reference it — matched directly (an
86+
`EventsNode`) **or** via the step events of any action a metric references, so action-based metrics are
87+
found by the event their action fires on. Then for each returned row, inspect its **`query`** (not the
88+
`name`/`description`): a saved metric is a reuse match when its `query` measures the **same event or
89+
action with the same `metric_type`** (and compatible `math`) as the metric you'd otherwise build, even
90+
if its name is different.
91+
- **Match on the event, not the action's name.** An action-based metric is discoverable by the event
92+
the action fires on — pass that event, not the action's label.
93+
- **Do not use `search` for this.** `search` matches only the metric's own `name` / `description` / tags —
94+
never the underlying event or action — so it cannot find a definition match. Use `search` only when the
95+
user names a specific saved metric to attach (name resolution, not a definition match).
9496
3. **If a saved metric matches the definition** — confirm the match with the user by name/description,
9597
then attach it instead of building a new one:
9698
- Call `experiment-get` to read the experiment's current `saved_metrics`.

services/mcp/schema/generated-tool-definitions.json

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

services/mcp/schema/tool-definitions-all.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3297,7 +3297,7 @@
32973297
}
32983298
},
32993299
"experiment-saved-metrics-list": {
3300-
"description": "List shared metrics in the current project. Returns paginated results with id, name, description, query, created_at, updated_at, tags. Use experiment-saved-metrics-retrieve for a single metric by ID, or this listing to resolve by name.",
3300+
"description": "List shared metrics in the current project. Returns paginated results with id, name, description, query, created_at, updated_at, tags.\n\nTo REUSE a metric by what it measures (rather than by its name): pass ?event=<the event you're measuring> to find metrics that reference it — directly or via an action — then compare each candidate's 'query' (metric_type + math) against the metric you'd otherwise build. Use 'search' (name/description/tags) only when the user named a specific metric to attach.",
33013301
"category": "Experiments",
33023302
"feature": "experiments",
33033303
"summary": "List shared experiment metrics",

services/mcp/src/api/generated.ts

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)