Skip to content
Merged
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
9 changes: 9 additions & 0 deletions src/sentry/investigations/templates/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
from .registry import get_investigation_template
from .types import InvestigationTemplateSpec, TemplateBlockSpec, TemplateParameterSpec

__all__ = (
"InvestigationTemplateSpec",
"TemplateBlockSpec",
"TemplateParameterSpec",
"get_investigation_template",
)
66 changes: 66 additions & 0 deletions src/sentry/investigations/templates/breached_metric.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
from sentry.investigations.models import InvestigationBlockKind, InvestigationSourceType
from sentry.investigations.templates.types import InvestigationTemplateSpec, TemplateBlockSpec

BREACHED_METRIC_TEMPLATE = InvestigationTemplateSpec(
key="breached_metric",
version=1,
source_type=InvestigationSourceType.BREACHED_METRIC,
parameters=(),
blocks=(
TemplateBlockSpec(
key="metric_chart",
kind=InvestigationBlockKind.QUERY,
title="Breached metric",
generation_prompt=(
"Query the exact supplied monitor definition over the supplied analysis window. "
"Make the breach immediately visible in a time-series chart spanning the equal "
"pre-breach baseline and open-period portions. Plot the observed metric and the "
"supplied threshold or comparison as separate series so the crossing is clear. "
"Use the monitor time window for the chart interval when supported."
),
config={"autoRun": True, "preferChart": True},
display={"version": 1, "type": "table", "defaultView": "chart"},
),
TemplateBlockSpec(
key="overview",
kind=InvestigationBlockKind.TEXT,
title="Overview",
generation_prompt=(
"Give the reader a useful overview of this breached metric using the supplied "
"monitor, open-period, project, threshold, direction, and analysis-window facts. "
"Accurately describe whether this is an upward or downward breach. Do not claim a "
"cause before examining the telemetry. Keep the overview to two short paragraphs."
),
config={"autoRun": True},
display={"type": "markdown"},
),
TemplateBlockSpec(
key="synthesis",
kind=InvestigationBlockKind.TEXT,
title="What explains the change",
generation_prompt=(
"Explain what the breached-metric result above and contributor result below show "
"together. Focus on evidence, distinguish correlation from causation, and state "
"uncertainty when the telemetry does not establish a convincing explanation. Keep "
"the answer to two or three short paragraphs unless a tiny table is essential."
),
config={"autoRun": True},
display={"type": "markdown"},
dependencies=("metric_chart", "contributors"),
),
TemplateBlockSpec(
key="contributors",
kind=InvestigationBlockKind.QUERY,
title="Likely contributors",
generation_prompt=(
"Compare telemetry during the supplied open-period window with its equal baseline. "
"Use as many supported telemetry calls and local transformations as useful. Let "
"the evidence determine whether issue groups, tags, or other metadata best explain "
"the change, then chart the strongest available evidence. If no convincing "
"contributor exists, show the most useful evidence and say so in the result."
),
config={"autoRun": True, "preferChart": True},
display={"version": 1, "type": "table", "defaultView": "chart"},
),
),
)
12 changes: 12 additions & 0 deletions src/sentry/investigations/templates/registry.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
from types import MappingProxyType

from sentry.investigations.templates.breached_metric import BREACHED_METRIC_TEMPLATE
from sentry.investigations.templates.types import InvestigationTemplateSpec

_TEMPLATES = MappingProxyType(
{(BREACHED_METRIC_TEMPLATE.key, BREACHED_METRIC_TEMPLATE.version): (BREACHED_METRIC_TEMPLATE)}
)


def get_investigation_template(key: str, version: int) -> InvestigationTemplateSpec | None:
return _TEMPLATES.get((key, version))
38 changes: 38 additions & 0 deletions src/sentry/investigations/templates/types.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
from __future__ import annotations

from dataclasses import dataclass, field
from typing import Any


@dataclass(frozen=True)
class TemplateParameterSpec:
key: str
label: str
type: str
description: str = ""
required: bool = False
default_value: Any = None
constraints: dict[str, Any] = field(default_factory=dict)


@dataclass(frozen=True)
class TemplateBlockSpec:
key: str
kind: str
title: str
content: str = ""
generation_prompt: str = ""
generated_content: str = ""
config: dict[str, Any] = field(default_factory=dict)
display: dict[str, Any] = field(default_factory=dict)
dependencies: tuple[str, ...] = ()
parameters: tuple[str, ...] = ()

Comment on lines +20 to +30

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.

Bug: The frozen=True dataclasses TemplateBlockSpec and TemplateParameterSpec are unhashable because they contain dict fields, which will cause a TypeError if they are ever used in sets or as dict keys.
Severity: LOW

Suggested Fix

To make the dataclasses hashable as intended by frozen=True, convert the dict fields to an immutable type. For example, change dict[str, Any] to MappingProxyType[str, Any] and ensure the dictionary is wrapped in MappingProxyType during initialization. This preserves the immutable intent while making the objects correctly hashable.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: src/sentry/investigations/templates/types.py#L18-L30

Potential issue: The dataclasses `TemplateBlockSpec` and `TemplateParameterSpec` are
defined with `frozen=True`, which implicitly generates a `__hash__` method. However,
these classes contain fields of type `dict` (`config`, `display`, `constraints`), which
are unhashable. Any attempt to add instances of these classes to a set or use them as
dictionary keys will result in a `TypeError: unhashable type: 'dict'`. While the current
pull request does not perform any hashing operations on these objects, this design
creates a latent bug that will likely cause runtime crashes in future code that handles
instantiation, caching, or deduplication of these spec objects.

Also affects:

  • src/sentry/investigations/templates/types.py:7~16


@dataclass(frozen=True)
class InvestigationTemplateSpec:
key: str
version: int
source_type: str
parameters: tuple[TemplateParameterSpec, ...]
blocks: tuple[TemplateBlockSpec, ...]
2 changes: 2 additions & 0 deletions tests/sentry/investigations/test_models.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from __future__ import annotations

import pytest
from django.db import IntegrityError, router, transaction

Expand Down
155 changes: 155 additions & 0 deletions tests/sentry/investigations/test_templates.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
from __future__ import annotations

import pytest

from sentry.investigations.models import InvestigationBlockKind, InvestigationSourceType
from sentry.investigations.templates import (
InvestigationTemplateSpec,
get_investigation_template,
)
from sentry.investigations.templates.breached_metric import BREACHED_METRIC_TEMPLATE
from sentry.investigations.templates.registry import _TEMPLATES

ALL_TEMPLATES = tuple(_TEMPLATES.values())


def test_registry_resolves_a_registered_template() -> None:
template = get_investigation_template("breached_metric", 1)

assert template is BREACHED_METRIC_TEMPLATE


def test_registry_returns_none_for_an_unknown_key() -> None:
assert get_investigation_template("does_not_exist", 1) is None


def test_registry_returns_none_for_an_unknown_version() -> None:
assert get_investigation_template("breached_metric", 2) is None


def test_registry_is_keyed_by_key_and_version() -> None:
for (key, version), template in _TEMPLATES.items():
assert template.key == key
assert template.version == version


def test_registry_is_immutable() -> None:
with pytest.raises(TypeError):
_TEMPLATES["breached_metric", 1] = BREACHED_METRIC_TEMPLATE # type: ignore[index]


@pytest.mark.parametrize("template", ALL_TEMPLATES, ids=lambda t: f"{t.key}-v{t.version}")
def test_block_keys_are_unique(template: InvestigationTemplateSpec) -> None:
keys = [block.key for block in template.blocks]

assert len(keys) == len(set(keys))


@pytest.mark.parametrize("template", ALL_TEMPLATES, ids=lambda t: f"{t.key}-v{t.version}")
def test_dependencies_name_blocks_in_the_same_template(
template: InvestigationTemplateSpec,
) -> None:
"""A typo'd dependency key would otherwise only surface at instantiation."""
block_keys = {block.key for block in template.blocks}

for block in template.blocks:
unknown = set(block.dependencies) - block_keys
assert not unknown, f"{block.key} depends on unknown block(s): {sorted(unknown)}"


@pytest.mark.parametrize("template", ALL_TEMPLATES, ids=lambda t: f"{t.key}-v{t.version}")
def test_no_block_depends_on_itself(template: InvestigationTemplateSpec) -> None:
for block in template.blocks:
assert block.key not in block.dependencies


@pytest.mark.parametrize("template", ALL_TEMPLATES, ids=lambda t: f"{t.key}-v{t.version}")
def test_block_parameters_are_declared_by_the_template(
template: InvestigationTemplateSpec,
) -> None:
parameter_keys = {parameter.key for parameter in template.parameters}

for block in template.blocks:
unknown = set(block.parameters) - parameter_keys
assert not unknown, f"{block.key} uses undeclared parameter(s): {sorted(unknown)}"


@pytest.mark.parametrize("template", ALL_TEMPLATES, ids=lambda t: f"{t.key}-v{t.version}")
def test_parameter_keys_are_unique(template: InvestigationTemplateSpec) -> None:
keys = [parameter.key for parameter in template.parameters]

assert len(keys) == len(set(keys))


@pytest.mark.parametrize("template", ALL_TEMPLATES, ids=lambda t: f"{t.key}-v{t.version}")
def test_block_kinds_are_valid(template: InvestigationTemplateSpec) -> None:
valid = set(InvestigationBlockKind.values)

assert all(block.kind in valid for block in template.blocks)


@pytest.mark.parametrize("template", ALL_TEMPLATES, ids=lambda t: f"{t.key}-v{t.version}")
def test_source_type_is_valid_and_not_manual(template: InvestigationTemplateSpec) -> None:
assert template.source_type in set(InvestigationSourceType.values)
# A template-backed investigation records source lineage, which the model's
# check constraint forbids for the manual source type.
assert template.source_type != InvestigationSourceType.MANUAL


@pytest.mark.parametrize("template", ALL_TEMPLATES, ids=lambda t: f"{t.key}-v{t.version}")
def test_specs_are_frozen(template: InvestigationTemplateSpec) -> None:
with pytest.raises(AttributeError):
template.key = "mutated" # type: ignore[misc]

for block in template.blocks:
with pytest.raises(AttributeError):
block.key = "mutated" # type: ignore[misc]


class TestBreachedMetricTemplate:
def test_declares_the_expected_blocks(self) -> None:
assert [block.key for block in BREACHED_METRIC_TEMPLATE.blocks] == [
"metric_chart",
"overview",
"synthesis",
"contributors",
]

def test_is_a_breached_metric_source(self) -> None:
assert BREACHED_METRIC_TEMPLATE.source_type == InvestigationSourceType.BREACHED_METRIC

def test_synthesis_depends_on_both_query_blocks(self) -> None:
synthesis = next(
block for block in BREACHED_METRIC_TEMPLATE.blocks if block.key == "synthesis"
)

assert set(synthesis.dependencies) == {"metric_chart", "contributors"}

def test_every_block_auto_runs(self) -> None:
assert all(block.config.get("autoRun") is True for block in BREACHED_METRIC_TEMPLATE.blocks)

def test_query_blocks_default_to_the_chart_view(self) -> None:
query_blocks = [
block
for block in BREACHED_METRIC_TEMPLATE.blocks
if block.kind == InvestigationBlockKind.QUERY
]

assert query_blocks
assert all(block.display["defaultView"] == "chart" for block in query_blocks)

def test_text_blocks_use_the_markdown_display(self) -> None:
text_blocks = [
block
for block in BREACHED_METRIC_TEMPLATE.blocks
if block.kind == InvestigationBlockKind.TEXT
]

assert text_blocks
assert all(block.display == {"type": "markdown"} for block in text_blocks)

def test_every_block_carries_a_generation_prompt(self) -> None:
assert all(block.generation_prompt.strip() for block in BREACHED_METRIC_TEMPLATE.blocks)

def test_takes_no_parameters(self) -> None:
assert BREACHED_METRIC_TEMPLATE.parameters == ()
Loading