diff --git a/src/sentry/ai_monitoring/utils.py b/src/sentry/ai_monitoring/utils.py index 272b9b8ded14..db207035ee35 100644 --- a/src/sentry/ai_monitoring/utils.py +++ b/src/sentry/ai_monitoring/utils.py @@ -84,6 +84,48 @@ def clamp_conversation_id_for_storage(conversation_id: str) -> str: return conversation_id[:CONVERSATION_ID_TRUNCATE_TO] + "..." +# Earliest title source wins (closest to the first user message); project_id breaks ties. +TITLE_ORDER_BY = (F("title_source_timestamp").asc(nulls_last=True), "project_id") + + +def fetch_conversation_titles( + conversation_project_pairs: Collection[tuple[str, int]], +) -> dict[str, str]: + """One title per conversation_id among the given (conversation_id, project_id) pairs. + + Only requested pairs are considered (ids are unique per project). Among those, + earliest ``title_source_timestamp`` wins; ``project_id`` breaks ties. + """ + if not conversation_project_pairs: + return {} + + requested_pairs = set(conversation_project_pairs) + conversation_id_by_hash = { + conversation_id_hash(conversation_id): conversation_id + for conversation_id, _ in requested_pairs + } + + rows = ( + AIConversationMetadata.objects.filter( + project_id__in={project_id for _, project_id in requested_pairs}, + conversation_id_hash__in=conversation_id_by_hash, + title__isnull=False, + ) + .exclude(title="") + .order_by(*TITLE_ORDER_BY) + .values_list("conversation_id_hash", "project_id", "title") + ) + + titles: dict[str, str] = {} + for row_hash, project_id, title in rows: + if title is None: + continue + conversation_id = conversation_id_by_hash[row_hash] + if (conversation_id, project_id) in requested_pairs: + titles.setdefault(conversation_id, title) + return titles + + def fetch_conversation_title( conversation_id: str, project_ids: Collection[int], @@ -105,45 +147,11 @@ def fetch_conversation_title( title__isnull=False, ) .exclude(title="") - .order_by(F("title_source_timestamp").asc(nulls_last=True), "project_id") + .order_by(*TITLE_ORDER_BY) .first() ) -def fetch_conversation_titles( - conversation_project_pairs: Collection[tuple[str, int]], -) -> dict[tuple[str, int], str]: - """Look up stored titles for the given (conversation_id, project_id) pairs. - - A conversation id is only unique within a project, so callers must key on the - pair. Pairs without a titled row are simply absent from the result. - """ - if not conversation_project_pairs: - return {} - - requested_pairs = set(conversation_project_pairs) - conversation_id_by_hash = { - conversation_id_hash(conversation_id): conversation_id - for conversation_id, _ in requested_pairs - } - - rows = AIConversationMetadata.objects.filter( - project_id__in={project_id for _, project_id in requested_pairs}, - conversation_id_hash__in=conversation_id_by_hash, - title__isnull=False, - ).values_list("conversation_id_hash", "project_id", "title") - - # The filter matches the cross product of the hashes and the projects, so drop - # any (conversation, project) combination the caller did not ask about. - titles: dict[tuple[str, int], str] = {} - for row_hash, project_id, title in rows: - pair = (conversation_id_by_hash[row_hash], project_id) - if title and pair in requested_pairs: - titles[pair] = title - - return titles - - def _extract_first_user_message(messages: Any) -> str | None: if isinstance(messages, str) and messages == FILTERED: return None diff --git a/src/sentry/api/endpoints/organization_ai_conversations.py b/src/sentry/api/endpoints/organization_ai_conversations.py index b716ff7b41e4..5ade3bd71f82 100644 --- a/src/sentry/api/endpoints/organization_ai_conversations.py +++ b/src/sentry/api/endpoints/organization_ai_conversations.py @@ -1,7 +1,7 @@ import logging import re from collections import defaultdict -from collections.abc import Mapping, Sequence +from collections.abc import Mapping from datetime import datetime from typing import Any, TypedDict @@ -80,17 +80,6 @@ def _compute_timestamp_ms(finish_ts: float) -> int: return int(finish_ts * 1000) if finish_ts else 0 -def _first_title( - titles: Mapping[tuple[str, int], str], conv_id: str, project_ids: Sequence[int] -) -> str | None: - """Lowest project id with a stored title wins, so results are stable across requests.""" - for project_id in project_ids: - title = titles.get((conv_id, project_id)) - if title is not None: - return title - return None - - def _extract_first_user_message(messages: Any) -> str | None: """Extract first user message, handling both old (content) and new (parts) formats.""" if isinstance(messages, str) and messages == FILTERED: @@ -592,17 +581,23 @@ def _apply_titles( conversations_map: dict[str, dict[str, Any]], project_ids_by_conversation: Mapping[str, set[int]], ) -> None: - """Attach stored conversation titles, leaving `title` as None when we have none.""" - sorted_project_ids = { - conv_id: sorted(project_ids_by_conversation.get(conv_id, ())) + """Set each conversation's `title` from storage when present. + + On lookup failure, log and leave titles unset so the list response still succeeds. + """ + pairs = [ + (conv_id, project_id) for conv_id in conversations_map - } - titles = fetch_conversation_titles( - [ - (conv_id, project_id) - for conv_id, project_ids in sorted_project_ids.items() - for project_id in project_ids - ] - ) - for conv_id, conversation in conversations_map.items(): - conversation["title"] = _first_title(titles, conv_id, sorted_project_ids[conv_id]) + for project_id in project_ids_by_conversation.get(conv_id, ()) + ] + try: + titles = fetch_conversation_titles(pairs) + except Exception: + logger.exception( + "Failed to resolve titles for AI conversations", + extra={"project_ids": sorted({project_id for _, project_id in pairs})}, + ) + return + + for conv_id, title in titles.items(): + conversations_map[conv_id]["title"] = title diff --git a/tests/sentry/ai_monitoring/test_utils.py b/tests/sentry/ai_monitoring/test_utils.py index 0d75ecff4b39..92b2ba3acda3 100644 --- a/tests/sentry/ai_monitoring/test_utils.py +++ b/tests/sentry/ai_monitoring/test_utils.py @@ -17,7 +17,7 @@ def test_returns_title_for_requested_pair(self) -> None: titles = fetch_conversation_titles([("conv-1", self.project.id)]) - assert titles == {("conv-1", self.project.id): "Reset my password"} + assert titles == {"conv-1": "Reset my password"} def test_skips_untitled_rows(self) -> None: self.create_ai_conversation_metadata( @@ -63,32 +63,99 @@ def test_does_not_return_pairs_that_were_not_requested(self) -> None: ) assert titles == { - ("conv-1", self.project.id): "Owned by project one", - ("conv-2", other_project.id): "Second conversation", + "conv-1": "Owned by project one", + "conv-2": "Second conversation", } - def test_returns_both_projects_when_both_requested(self) -> None: + def test_earliest_source_timestamp_wins(self) -> None: other_project = self.create_project(organization=self.organization) + self.create_ai_conversation_metadata( + project=self.project, + conversation_id="conv-1", + title="Later half of the conversation", + title_source_timestamp=datetime(2024, 5, 1, 12, 0, tzinfo=UTC), + ) + self.create_ai_conversation_metadata( + project=other_project, + conversation_id="conv-1", + title="Start of the conversation", + title_source_timestamp=datetime(2024, 5, 1, 11, 0, tzinfo=UTC), + ) + + titles = fetch_conversation_titles( + [("conv-1", self.project.id), ("conv-1", other_project.id)] + ) + + assert titles == {"conv-1": "Start of the conversation"} + def test_null_source_timestamp_loses(self) -> None: + other_project = self.create_project(organization=self.organization) self.create_ai_conversation_metadata( project=self.project, conversation_id="conv-1", - title="Owned by project one", + title="Unknown when this started", + title_source_timestamp=None, ) self.create_ai_conversation_metadata( project=other_project, conversation_id="conv-1", - title="Owned by project two", + title="Start of the conversation", + title_source_timestamp=datetime(2024, 5, 1, 11, 0, tzinfo=UTC), ) titles = fetch_conversation_titles( [("conv-1", self.project.id), ("conv-1", other_project.id)] ) - assert titles == { - ("conv-1", self.project.id): "Owned by project one", - ("conv-1", other_project.id): "Owned by project two", - } + assert titles == {"conv-1": "Start of the conversation"} + + def test_ties_break_on_project_id(self) -> None: + source_timestamp = datetime(2024, 5, 1, tzinfo=UTC) + lower_project, higher_project = sorted( + ( + self.create_project(organization=self.organization), + self.create_project(organization=self.organization), + ), + key=lambda project: project.id, + ) + + self.create_ai_conversation_metadata( + project=lower_project, + conversation_id="conv-1", + title="Lower project id", + title_source_timestamp=source_timestamp, + ) + self.create_ai_conversation_metadata( + project=higher_project, + conversation_id="conv-1", + title="Higher project id", + title_source_timestamp=source_timestamp, + ) + + titles = fetch_conversation_titles( + [("conv-1", lower_project.id), ("conv-1", higher_project.id)] + ) + + assert titles == {"conv-1": "Lower project id"} + + def test_unrequested_earlier_title_does_not_win(self) -> None: + other_project = self.create_project(organization=self.organization) + self.create_ai_conversation_metadata( + project=self.project, + conversation_id="conv-1", + title="Requested project title", + title_source_timestamp=datetime(2024, 5, 1, 12, 0, tzinfo=UTC), + ) + self.create_ai_conversation_metadata( + project=other_project, + conversation_id="conv-1", + title="Earlier but unrequested", + title_source_timestamp=datetime(2024, 5, 1, 11, 0, tzinfo=UTC), + ) + + titles = fetch_conversation_titles([("conv-1", self.project.id)]) + + assert titles == {"conv-1": "Requested project title"} class FetchConversationTitleTest(TestCase): diff --git a/tests/sentry/api/endpoints/test_organization_ai_conversations.py b/tests/sentry/api/endpoints/test_organization_ai_conversations.py index 2362e91af3cc..4f03bd6abe30 100644 --- a/tests/sentry/api/endpoints/test_organization_ai_conversations.py +++ b/tests/sentry/api/endpoints/test_organization_ai_conversations.py @@ -1,6 +1,7 @@ import json # noqa: S003 from datetime import timedelta from typing import Any +from unittest.mock import MagicMock, patch from uuid import uuid4 from django.urls import reverse @@ -1527,7 +1528,7 @@ def test_title_not_taken_from_unrelated_project(self) -> None: assert response.data[0]["title"] is None def test_title_for_conversation_spanning_projects(self) -> None: - """When a conversation spans projects, the lowest project id with a title wins.""" + """Equal source timestamps: lowest project id breaks the tie.""" now = before_now(days=25).replace(microsecond=0) conversation_id = uuid4().hex lower_project = self.create_project(organization=self.organization) @@ -1565,6 +1566,45 @@ def test_title_for_conversation_spanning_projects(self) -> None: assert len(response.data) == 1 assert response.data[0]["title"] == "Lower project id title" + def test_title_earliest_source_timestamp_wins_across_projects(self) -> None: + """Across projects, earliest title_source_timestamp wins (not lowest project id).""" + now = before_now(days=25).replace(microsecond=0) + conversation_id = uuid4().hex + lower_project = self.create_project(organization=self.organization) + higher_project = self.create_project(organization=self.organization) + assert lower_project.id < higher_project.id + + self._store_conversation_span( + conversation_id, now - timedelta(seconds=2), project=lower_project + ) + self._store_conversation_span( + conversation_id, now - timedelta(seconds=1), project=higher_project + ) + + self.create_ai_conversation_metadata( + project=lower_project, + conversation_id=conversation_id, + title="Later titled project", + title_source_timestamp=now - timedelta(seconds=1), + ) + self.create_ai_conversation_metadata( + project=higher_project, + conversation_id=conversation_id, + title="Earlier titled project", + title_source_timestamp=now - timedelta(seconds=2), + ) + + query = { + "project": [lower_project.id, higher_project.id], + "start": (now - timedelta(hours=1)).isoformat(), + "end": (now + timedelta(hours=1)).isoformat(), + } + + response = self.do_request(query) + assert response.status_code == 200, response.data + assert len(response.data) == 1 + assert response.data[0]["title"] == "Earlier titled project" + def test_title_found_when_only_higher_project_id_has_one(self) -> None: """Every project the conversation spans is searched, not just the lowest.""" now = before_now(days=25).replace(microsecond=0) @@ -1597,3 +1637,27 @@ def test_title_found_when_only_higher_project_id_has_one(self) -> None: assert response.status_code == 200, response.data assert len(response.data) == 1 assert response.data[0]["title"] == "Higher project id title" + + @patch( + "sentry.api.endpoints.organization_ai_conversations.fetch_conversation_titles", + side_effect=Exception("metadata unavailable"), + ) + def test_title_lookup_failure_does_not_break_list( + self, mock_fetch_conversation_titles: MagicMock + ) -> None: + now = before_now(days=25).replace(microsecond=0) + conversation_id = uuid4().hex + + self._store_conversation_span(conversation_id, now) + + query = { + "project": [self.project.id], + "start": (now - timedelta(hours=1)).isoformat(), + "end": (now + timedelta(hours=1)).isoformat(), + } + + response = self.do_request(query) + assert response.status_code == 200, response.data + assert len(response.data) == 1 + assert response.data[0]["title"] is None + assert mock_fetch_conversation_titles.call_count == 1