From 0794a5f5706fcbccc67678ec061502dcdcd12cd7 Mon Sep 17 00:00:00 2001 From: Richard Roggenkemper Date: Wed, 24 Jun 2026 13:48:15 -0400 Subject: [PATCH] feat(issue-search): per-request weight override for recommended_v2 sort Compare alternate recommended_v2 weights against the live default in a single request via ?recommendedWeights={...}, without touching options or adding a second sort. The weights override the registered options for that request only and rerank just the requester's own feed: base factors flow into the Snuba aggregation through the existing aggregate_kwargs channel, boost weights rebuild the score_fn, and any unspecified weight falls back to options. Gated to the recommended_v2 sort since aggregate_kwargs is shared with trends. Payload is sanitized before it reaches the ClickHouse scoring SQL: weights and boost values must be finite floats, and group-type-boost keys are coerced to ints. Co-Authored-By: Claude --- src/sentry/api/helpers/group_index/index.py | 62 +++++++++++++++- src/sentry/search/snuba/executors.py | 74 ++++++++++++++----- .../search/test_postgres_sort_framework.py | 50 +++++++++++++ 3 files changed, 167 insertions(+), 19 deletions(-) diff --git a/src/sentry/api/helpers/group_index/index.py b/src/sentry/api/helpers/group_index/index.py index a518c7d71e02f0..efb59b569205cd 100644 --- a/src/sentry/api/helpers/group_index/index.py +++ b/src/sentry/api/helpers/group_index/index.py @@ -1,5 +1,6 @@ from __future__ import annotations +import math from collections.abc import Callable, Collection, Sequence from datetime import datetime from typing import Any @@ -27,7 +28,7 @@ from sentry.signals import advanced_search_feature_gated from sentry.snuba.referrer import Referrer from sentry.users.models.user import User -from sentry.utils import metrics +from sentry.utils import json, metrics from sentry.utils.cursors import Cursor, CursorResult from . import SEARCH_MAX_HITS @@ -144,9 +145,68 @@ def build_query_params_from_request( query, organization, projects, environments, request.user ) + # Per-request weight override for the recommended_v2 sort (?recommendedWeights={...}). + # Gated to that sort: aggregate_kwargs is shared with trends, which needs its own keys. + raw_weights = request.GET.get("recommendedWeights") + if raw_weights and query_kwargs["sort_by"] == "recommended_v2": + query_kwargs["aggregate_kwargs"] = {"recommended": _parse_recommended_weights(raw_weights)} + return query_kwargs +_RECOMMENDED_WEIGHT_KEYS = frozenset( + { + "recency", + "spike", + "severity", + "event_volume", + "user_impact", + "message_penalty", + "group_type_boost", + "assignment", + "fixability", + "agent", + "regressed", + "newness", + "newness_halflife_hours", + } +) + + +def _finite_float(value: Any) -> float: + # These reach the ClickHouse scoring SQL, so reject anything but a finite number. + parsed = float(value) + if not math.isfinite(parsed): + raise ValueError("weight must be finite") + return parsed + + +def _parse_recommended_weights(raw: str) -> dict[str, Any]: + try: + parsed = json.loads(raw) + except (ValueError, TypeError): + raise ValidationError("recommendedWeights must be valid JSON") + if not isinstance(parsed, dict): + raise ValidationError("recommendedWeights must be a JSON object") + clean: dict[str, Any] = {} + for key, value in parsed.items(): + if key not in _RECOMMENDED_WEIGHT_KEYS: + continue + try: + if key == "group_type_boost": + if not isinstance(value, dict): + raise ValueError("group_type_boost must be an object") + # Keys are issue type ids interpolated into SQL -- int() so only numeric ids pass. + clean[key] = { + str(int(type_id)): _finite_float(boost) for type_id, boost in value.items() + } + else: + clean[key] = _finite_float(value) + except (ValueError, TypeError): + raise ValidationError(f"invalid recommendedWeights value for '{key}'") + return clean + + def validate_search_filter_permissions( organization: Organization, search_filters: Sequence[AggregateFilter | SearchFilter], diff --git a/src/sentry/search/snuba/executors.py b/src/sentry/search/snuba/executors.py index b16c05136e702a..f5b435be00d2c4 100644 --- a/src/sentry/search/snuba/executors.py +++ b/src/sentry/search/snuba/executors.py @@ -799,23 +799,29 @@ def trends_aggregation_impl( def _recommended_aggregation( - timestamp_column: str, type_column: str | None = None + timestamp_column: str, type_column: str | None = None, overrides: dict[str, Any] | None = None ) -> Sequence[str]: hour = 3600 + # ?recommendedWeights values (if any) win over the registered option, per weight. + overrides = overrides or {} # Recency: exponential decay based on time since last event (24hr halflife) - recency_weight = options.get("snuba.search.recommended.recency-weight") + recency_weight = overrides.get( + "recency", options.get("snuba.search.recommended.recency-weight") + ) age_hours = f"divide(minus(now(), max({timestamp_column})), {hour})" recency = f"divide(1, pow(2, divide({age_hours}, 24)))" # Spike: ratio of recent 6hr events to total 3d events - spike_weight = options.get("snuba.search.recommended.spike-weight") + spike_weight = overrides.get("spike", options.get("snuba.search.recommended.spike-weight")) recent_6h = f"countIf(lessOrEquals(minus(now(), {timestamp_column}), {6 * hour}))" total_3d = f"countIf(lessOrEquals(minus(now(), {timestamp_column}), {3 * 24 * hour}))" spike = f"least(1.0, divide({recent_6h}, plus({total_3d}, 1)))" # Severity: max log level - maps fatal=1.0, error=0.75, warning=0.5, info=0.25, debug=0.0 - severity_weight = options.get("snuba.search.recommended.severity-weight") + severity_weight = overrides.get( + "severity", options.get("snuba.search.recommended.severity-weight") + ) severity = ( "max(multiIf(" "equals(level, 'fatal'), 1.0, " @@ -826,20 +832,28 @@ def _recommended_aggregation( ) # User impact: ln(uniq(tags[sentry:user]) + 1)/ln(1001) - maps 1→~0, 10→0.33, 100→0.67, 1000→1.0 - user_impact_weight = options.get("snuba.search.recommended.user-impact-weight") + user_impact_weight = overrides.get( + "user_impact", options.get("snuba.search.recommended.user-impact-weight") + ) user_impact = "least(1.0, divide(log(plus(uniq(tags[sentry:user]), 1)), log(1001)))" # Event volume: ln(count() + 1)/ln(10001) - maps 1→~0, 10→0.25, 100→0.50, 1000→0.75, 10000+→1.0 - event_volume_weight = options.get("snuba.search.recommended.event-volume-weight") + event_volume_weight = overrides.get( + "event_volume", options.get("snuba.search.recommended.event-volume-weight") + ) event_volume = "least(1.0, divide(log(plus(count(), 1)), log(10001)))" # Group type boost: additive signal per issue type - group_type_boosts = options.get("snuba.search.recommended.group-type-boost") + group_type_boosts = overrides.get( + "group_type_boost", options.get("snuba.search.recommended.group-type-boost") + ) # Message penalty: downranks capture_message issues (no exception/stacktrace). # Subtracted from the score below, and only on the events dataset -- issue-platform # occurrences don't have exception_stacks. - message_penalty_weight = options.get("snuba.search.recommended.message-penalty-weight") + message_penalty_weight = overrides.get( + "message_penalty", options.get("snuba.search.recommended.message-penalty-weight") + ) # Skip zero-weighted factors: their term is always 0, so computing them in # ClickHouse is wasted work -- especially expensive aggregates like user @@ -887,7 +901,10 @@ def recommended_aggregation( end: datetime, aggregate_kwargs: Any = None, ) -> Sequence[str]: - return _recommended_aggregation(timestamp_column="timestamp") + # aggregate_kwargs is already scoped to this aggregation -- the weight-override dict, if any. + return _recommended_aggregation( + timestamp_column="timestamp", overrides=aggregate_kwargs or None + ) def recommended_issue_platform_aggregation( @@ -896,7 +913,9 @@ def recommended_issue_platform_aggregation( aggregate_kwargs: Any = None, ) -> Sequence[str]: return _recommended_aggregation( - timestamp_column="client_timestamp", type_column="occurrence_type_id" + timestamp_column="client_timestamp", + type_column="occurrence_type_id", + overrides=aggregate_kwargs or None, ) @@ -979,16 +998,30 @@ def resolve_issue_agent_signal( return signal -def recommended_v2_strategy() -> PostgresSortStrategy: +def recommended_v2_strategy(overrides: dict[str, Any] | None = None) -> PostgresSortStrategy: """Recommended sort v2: the Snuba recommended score (recency/spike/severity/user impact/event volume) plus additive boosts for viewer relevance (assignment or suspect - commit), Seer fixability, Seer agent progress, regressed issues, and newly-seen issues.""" - assignment_weight = options.get("snuba.search.recommended.assignment-weight") - fixability_weight = options.get("snuba.search.recommended.fixability-weight") - agent_weight = options.get("snuba.search.recommended.agent-weight") - regressed_weight = options.get("snuba.search.recommended.regressed-weight") - newness_weight = options.get("snuba.search.recommended.newness-weight") - newness_halflife_hours = options.get("snuba.search.recommended.newness-halflife-hours") + commit), Seer fixability, Seer agent progress, regressed issues, and newly-seen issues. + + overrides: ?recommendedWeights values that win over the option, per weight (boosts only; + base-factor overrides are applied in _recommended_aggregation).""" + overrides = overrides or {} + assignment_weight = overrides.get( + "assignment", options.get("snuba.search.recommended.assignment-weight") + ) + fixability_weight = overrides.get( + "fixability", options.get("snuba.search.recommended.fixability-weight") + ) + agent_weight = overrides.get("agent", options.get("snuba.search.recommended.agent-weight")) + regressed_weight = overrides.get( + "regressed", options.get("snuba.search.recommended.regressed-weight") + ) + newness_weight = overrides.get( + "newness", options.get("snuba.search.recommended.newness-weight") + ) + newness_halflife_hours = overrides.get( + "newness_halflife_hours", options.get("snuba.search.recommended.newness-halflife-hours") + ) # Captured once per query so every group decays against the same clock. now = timezone.now() @@ -1378,6 +1411,11 @@ def query( pg_overflow_fallback = False pg_strategy = self.postgres_sort_strategies.get(sort_by) + # Base-factor overrides reach the aggregation via aggregate_kwargs; the score_fn binds + # its weights at construction, so rebuild the strategy here with the same overrides. + rec_overrides = cast("dict[str, Any]", aggregate_kwargs or {}).get("recommended") + if sort_by == "recommended_v2" and rec_overrides: + pg_strategy = recommended_v2_strategy(overrides=rec_overrides) if pg_strategy is not None: pg_result = self._execute_postgres_sort( strategy=pg_strategy, diff --git a/tests/snuba/search/test_postgres_sort_framework.py b/tests/snuba/search/test_postgres_sort_framework.py index 14a7670493a9bb..10d31199026c5c 100644 --- a/tests/snuba/search/test_postgres_sort_framework.py +++ b/tests/snuba/search/test_postgres_sort_framework.py @@ -18,6 +18,8 @@ InvalidQueryForExecutor, PostgresSnubaQueryExecutor, PostgresSortStrategy, + _recommended_aggregation, + recommended_v2_strategy, ) from sentry.snuba.referrer import Referrer from sentry.testutils.cases import SnubaTestCase, TestCase @@ -588,3 +590,51 @@ def test_progress_registered(self): # progress maps to last_seen in sort_strategies so the chunked Snuba path has a # real aggregation to fall back to on candidate overflow. assert PostgresSnubaQueryExecutor.sort_strategies["progress"] == "last_seen" + + +class TestRecommendedWeightOverrides(TestCase): + """Staff weight overrides (?recommendedWeights=...) flow into both the base-factor + Snuba aggregation and the recommended_v2 boost score_fn, without touching options.""" + + def test_base_factor_override_reaches_aggregation_sql(self): + # The override weight is interpolated into the generated ClickHouse expression. + with override_options({"snuba.search.recommended.severity-weight": 0.20}): + default_expr = _recommended_aggregation(timestamp_column="timestamp")[0] + override_expr = _recommended_aggregation( + timestamp_column="timestamp", overrides={"severity": 0.99} + )[0] + assert "0.99" in override_expr + assert "0.99" not in default_expr + + def test_boost_override_reaches_score_fn(self): + # With assignment relevance present, raising assignment-weight via override must + # raise the score by the corresponding amount vs the registered option default. + data = {"recommended": 0.0, "assignment": 1.0} + with override_options({"snuba.search.recommended.assignment-weight": 0.20}): + default_score = recommended_v2_strategy().score_fn(data) + override_score = recommended_v2_strategy(overrides={"assignment": 0.90}).score_fn(data) + assert override_score == pytest.approx(default_score + 0.70, abs=1e-6) + + def test_parse_recommended_weights_validates_and_coerces(self): + from sentry.api.helpers.group_index.index import _parse_recommended_weights + from sentry.api.helpers.group_index.validators import ValidationError + + # numeric coercion + unknown keys dropped + out = _parse_recommended_weights('{"spike": 0, "severity": 0.3, "bogus": 9}') + assert out == {"spike": 0.0, "severity": 0.3} + # group_type_boost dict is coerced per-type + assert _parse_recommended_weights('{"group_type_boost": {"8001": 0.1}}') == { + "group_type_boost": {"8001": 0.1} + } + # non-numeric value is rejected (guards SQL interpolation) + with pytest.raises(ValidationError): + _parse_recommended_weights('{"severity": "DROP TABLE"}') + # non-object payload is rejected + with pytest.raises(ValidationError): + _parse_recommended_weights("[1, 2, 3]") + # injection via a non-numeric group_type_boost KEY is rejected (the key reaches SQL) + with pytest.raises(ValidationError): + _parse_recommended_weights('{"group_type_boost": {"1) OR 1=1 --": 0.1}}') + # non-finite values are rejected (inf/nan are not numeric SQL literals) + with pytest.raises(ValidationError): + _parse_recommended_weights('{"severity": "inf"}')