From 482646657fc33eb754b86ded0ce52f9c4317b413 Mon Sep 17 00:00:00 2001 From: Kyle Consalus Date: Mon, 6 Jul 2026 12:54:38 -0700 Subject: [PATCH 1/2] fix(action-log): Serialization improvements --- src/sentry/issues/derived/features.py | 6 +- src/sentry/issues/derived/framework.py | 21 +++++- src/sentry/issues/derived/store.py | 7 +- .../sentry/issues/derived/test_processing.py | 66 +++++++++++++++++++ 4 files changed, 92 insertions(+), 8 deletions(-) diff --git a/src/sentry/issues/derived/features.py b/src/sentry/issues/derived/features.py index 2371eebd3781..00fa9fb7ed81 100644 --- a/src/sentry/issues/derived/features.py +++ b/src/sentry/issues/derived/features.py @@ -1,7 +1,7 @@ from datetime import datetime from enum import StrEnum -from sentry.issues.derived.framework import EnumCodec, Feature, OptionalCodec +from sentry.issues.derived.framework import DateTimeCodec, EnumCodec, Feature, OptionalCodec from sentry.issues.progress_state import IssueProgressState @@ -24,4 +24,6 @@ class IssueStatus(StrEnum): ) # The last time the progress was advanced. -LAST_PROGRESSED_AT = Feature[datetime | None]("last_progressed_at", default=None) +LAST_PROGRESSED_AT = Feature[datetime | None]( + "last_progressed_at", default=None, codec=OptionalCodec(DateTimeCodec()) +) diff --git a/src/sentry/issues/derived/framework.py b/src/sentry/issues/derived/framework.py index b499730e7a87..c657b35d886a 100644 --- a/src/sentry/issues/derived/framework.py +++ b/src/sentry/issues/derived/framework.py @@ -6,6 +6,7 @@ import copy from collections.abc import Callable, Iterable, Iterator from dataclasses import dataclass +from datetime import datetime from enum import IntEnum, StrEnum from typing import Any, Protocol, runtime_checkable @@ -18,7 +19,12 @@ class Codec[T]: - """Handles serialization of a Feature value to/from JSON-compatible form.""" + """Converts a Feature's Python value to/from a JSON-compatible representation. + + Used only for JSON serialization (e.g. the ``data`` JSONField blob). + Column-backed features bypass the codec entirely — Django model fields + handle native Python values directly. + """ def dump(self, value: T) -> Any: return value @@ -41,6 +47,14 @@ def dump(self, value: E) -> str: return value.value +class DateTimeCodec(Codec[datetime]): + def dump(self, value: datetime) -> str: + return value.isoformat() + + def load(self, raw: Any) -> datetime: + return datetime.fromisoformat(raw) + + class OptionalCodec[T](Codec[T | None]): def __init__(self, inner: Codec[T]) -> None: self._inner = inner @@ -62,8 +76,9 @@ def dump(self, value: T | None) -> Any: class Feature[T]: """A named, typed slot in derived state with a default value. - The `codec` handles conversion to/from JSON-compatible representations. - Defaults to identity (pass-through) for JSON-native types. + The ``codec`` handles conversion to/from JSON-compatible representations + and is used only for features stored in the JSON blob. Column-backed + features are stored as raw Python values; no codec is applied. """ def __init__( diff --git a/src/sentry/issues/derived/store.py b/src/sentry/issues/derived/store.py index e639d7c45203..f3769e6bbeeb 100644 --- a/src/sentry/issues/derived/store.py +++ b/src/sentry/issues/derived/store.py @@ -27,7 +27,8 @@ class GroupDerivedDataStore: """Translates between Pipeline State and GroupDerivedData storage. Features listed in COLUMN_MAP are read from / written to dedicated - model columns. All other features use the `data` JSON blob. + model columns as raw Python values (the column type must be compatible). + All other features use the ``data`` JSON blob via the Feature's codec. """ @staticmethod @@ -37,7 +38,7 @@ def load(pipeline: Pipeline[Any], derived: GroupDerivedData) -> State: for f in pipeline.features: column = COLUMN_MAP.get(f) if column: - result[f] = f.load(getattr(derived, column)) + result[f] = getattr(derived, column) elif f.name in data: result[f] = f.load(data[f.name]) else: @@ -58,7 +59,7 @@ def build_update(pipeline: Pipeline[Any], state: State) -> dict[str, Any]: for f in pipeline.features: column = COLUMN_MAP.get(f) if column and f in updated: - update[column] = f.dump(state[f]) + update[column] = state[f] # If any JSON feature was updated, include all of them (the blob is replaced wholesale) if updated.intersection(json_features): update["data"] = {f.name: f.dump(state[f]) for f in json_features} diff --git a/tests/sentry/issues/derived/test_processing.py b/tests/sentry/issues/derived/test_processing.py index 0f165e2c48ee..01e724dbbb30 100644 --- a/tests/sentry/issues/derived/test_processing.py +++ b/tests/sentry/issues/derived/test_processing.py @@ -1,3 +1,5 @@ +from datetime import datetime, timezone + import pytest from django.db import router, transaction @@ -26,7 +28,10 @@ ) from sentry.issues.derived.framework import ( AggregatorResult, + DateTimeCodec, + EnumCodec, Feature, + OptionalCodec, Pipeline, State, StateUpdate, @@ -46,6 +51,7 @@ from sentry.testutils.cases import TestCase from sentry.testutils.helpers.features import with_feature from sentry.testutils.outbox import outbox_runner +from sentry.utils import json SOURCE = ActionSource.API @@ -333,6 +339,66 @@ def test_store_apply_to_instance() -> None: assert derived.view_count == 5 +def test_all_feature_defaults_round_trip_through_json() -> None: + state = PIPELINE.initial_state() + blob = {f.name: f.dump(state[f]) for f in PIPELINE.features} + serialized = json.loads(json.dumps(blob)) + for f in PIPELINE.features: + assert f.load(serialized[f.name]) == state[f], f"round-trip failed for {f.name}" + + +# --- Codec tests --- + + +class TestDateTimeCodec: + def test_round_trip(self) -> None: + codec = DateTimeCodec() + dt = datetime(2025, 3, 15, 12, 30, 45, tzinfo=timezone.utc) + assert codec.load(codec.dump(dt)) == dt + + def test_dump_produces_iso_string(self) -> None: + codec = DateTimeCodec() + dt = datetime(2025, 3, 15, 12, 30, 45, tzinfo=timezone.utc) + dumped = codec.dump(dt) + assert isinstance(dumped, str) + assert dumped == dt.isoformat() + + def test_optional_none(self) -> None: + codec = OptionalCodec(DateTimeCodec()) + assert codec.dump(None) is None + assert codec.load(None) is None + + def test_optional_round_trip(self) -> None: + codec = OptionalCodec(DateTimeCodec()) + dt = datetime(2025, 3, 15, 12, 30, 45, tzinfo=timezone.utc) + assert codec.load(codec.dump(dt)) == dt + + +class TestEnumCodecCoverage: + @pytest.mark.parametrize("raw", ["open", "closed"]) + def test_issue_status_loads_known_values(self, raw: str) -> None: + codec = EnumCodec(IssueStatus) + loaded = codec.load(raw) + assert codec.dump(loaded) == raw + + @pytest.mark.parametrize( + "raw", ["identified", "assigned", "diagnosed", "fix_proposed", "fix_applied"] + ) + def test_issue_progress_state_loads_known_values(self, raw: str) -> None: + codec = EnumCodec(IssueProgressState) + loaded = codec.load(raw) + assert codec.dump(loaded) == raw + + @pytest.mark.parametrize( + "raw", + [None, "identified", "assigned", "diagnosed", "fix_proposed", "fix_applied"], + ) + def test_optional_progress_loads_known_values(self, raw: str | None) -> None: + codec = OptionalCodec(EnumCodec(IssueProgressState)) + loaded = codec.load(raw) + assert codec.dump(loaded) == raw + + # --- Store tests (need DB) --- From c59a5f6a6fa3817cacbc1ba05ec11f4e191b4236 Mon Sep 17 00:00:00 2001 From: Kyle Consalus Date: Tue, 7 Jul 2026 10:32:33 -0700 Subject: [PATCH 2/2] add column/json-specific methods --- src/sentry/issues/derived/framework.py | 71 +++++++++++++------ src/sentry/issues/derived/store.py | 13 ++-- .../sentry/issues/derived/test_processing.py | 71 ++++++++++++++----- 3 files changed, 109 insertions(+), 46 deletions(-) diff --git a/src/sentry/issues/derived/framework.py b/src/sentry/issues/derived/framework.py index c657b35d886a..d98773e4c64f 100644 --- a/src/sentry/issues/derived/framework.py +++ b/src/sentry/issues/derived/framework.py @@ -19,17 +19,28 @@ class Codec[T]: - """Converts a Feature's Python value to/from a JSON-compatible representation. + """Converts a Feature's Python value to/from storage representations. - Used only for JSON serialization (e.g. the ``data`` JSONField blob). - Column-backed features bypass the codec entirely — Django model fields - handle native Python values directly. + Two pairs of methods handle different storage backends: + + * ``to_json`` / ``from_json`` — for the ``data`` JSONField blob. + * ``to_column`` / ``from_column`` — for dedicated Django model columns. + + The default implementation is identity for both pairs. Override as + needed (e.g. ``EnumCodec`` wraps raw strings back into enum members + on ``from_column`` so that column-loaded values are real enum instances). """ - def dump(self, value: T) -> Any: + def to_json(self, value: T) -> Any: return value - def load(self, raw: Any) -> T: + def from_json(self, raw: Any) -> T: + return raw + + def to_column(self, value: T) -> Any: + return value + + def from_column(self, raw: Any) -> T: return raw @@ -40,18 +51,24 @@ class EnumCodec[E: StrEnum](Codec[E]): def __init__(self, enum_cls: type[E]) -> None: self._enum_cls = enum_cls - def load(self, raw: Any) -> E: + def to_json(self, value: E) -> str: + return value.value + + def from_json(self, raw: Any) -> E: return self._enum_cls(raw) - def dump(self, value: E) -> str: + def to_column(self, value: E) -> str: return value.value + def from_column(self, raw: Any) -> E: + return self._enum_cls(raw) + class DateTimeCodec(Codec[datetime]): - def dump(self, value: datetime) -> str: + def to_json(self, value: datetime) -> str: return value.isoformat() - def load(self, raw: Any) -> datetime: + def from_json(self, raw: Any) -> datetime: return datetime.fromisoformat(raw) @@ -59,11 +76,17 @@ class OptionalCodec[T](Codec[T | None]): def __init__(self, inner: Codec[T]) -> None: self._inner = inner - def load(self, raw: Any) -> T | None: - return self._inner.load(raw) if raw is not None else None + def to_json(self, value: T | None) -> Any: + return self._inner.to_json(value) if value is not None else None + + def from_json(self, raw: Any) -> T | None: + return self._inner.from_json(raw) if raw is not None else None - def dump(self, value: T | None) -> Any: - return self._inner.dump(value) if value is not None else None + def to_column(self, value: T | None) -> Any: + return self._inner.to_column(value) if value is not None else None + + def from_column(self, raw: Any) -> T | None: + return self._inner.from_column(raw) if raw is not None else None # --------------------------------------------------------------------------- @@ -76,9 +99,9 @@ def dump(self, value: T | None) -> Any: class Feature[T]: """A named, typed slot in derived state with a default value. - The ``codec`` handles conversion to/from JSON-compatible representations - and is used only for features stored in the JSON blob. Column-backed - features are stored as raw Python values; no codec is applied. + The ``codec`` handles conversion to/from storage representations. + JSON-blob features use ``to_json`` / ``from_json``; column-backed + features use ``to_column`` / ``from_column``. """ def __init__( @@ -102,11 +125,17 @@ def initial_value(self) -> T: return self._default_factory() return self._default - def dump(self, value: T) -> Any: - return self._codec.dump(value) + def to_json(self, value: T) -> Any: + return self._codec.to_json(value) + + def from_json(self, raw: Any) -> T: + return self._codec.from_json(raw) + + def to_column(self, value: T) -> Any: + return self._codec.to_column(value) - def load(self, raw: Any) -> T: - return self._codec.load(raw) + def from_column(self, raw: Any) -> T: + return self._codec.from_column(raw) def value(self, val: T) -> FeatureEntry: return (self, val) diff --git a/src/sentry/issues/derived/store.py b/src/sentry/issues/derived/store.py index f3769e6bbeeb..1e585a43533b 100644 --- a/src/sentry/issues/derived/store.py +++ b/src/sentry/issues/derived/store.py @@ -27,8 +27,9 @@ class GroupDerivedDataStore: """Translates between Pipeline State and GroupDerivedData storage. Features listed in COLUMN_MAP are read from / written to dedicated - model columns as raw Python values (the column type must be compatible). - All other features use the ``data`` JSON blob via the Feature's codec. + model columns via the Feature's ``from_column`` / ``to_column`` codec + methods. All other features use the ``data`` JSON blob via + ``from_json`` / ``to_json``. """ @staticmethod @@ -38,9 +39,9 @@ def load(pipeline: Pipeline[Any], derived: GroupDerivedData) -> State: for f in pipeline.features: column = COLUMN_MAP.get(f) if column: - result[f] = getattr(derived, column) + result[f] = f.from_column(getattr(derived, column)) elif f.name in data: - result[f] = f.load(data[f.name]) + result[f] = f.from_json(data[f.name]) else: result[f] = f.initial_value() return State(result) @@ -59,10 +60,10 @@ def build_update(pipeline: Pipeline[Any], state: State) -> dict[str, Any]: for f in pipeline.features: column = COLUMN_MAP.get(f) if column and f in updated: - update[column] = state[f] + update[column] = f.to_column(state[f]) # If any JSON feature was updated, include all of them (the blob is replaced wholesale) if updated.intersection(json_features): - update["data"] = {f.name: f.dump(state[f]) for f in json_features} + update["data"] = {f.name: f.to_json(state[f]) for f in json_features} return update @staticmethod diff --git a/tests/sentry/issues/derived/test_processing.py b/tests/sentry/issues/derived/test_processing.py index 01e724dbbb30..968fdf16f265 100644 --- a/tests/sentry/issues/derived/test_processing.py +++ b/tests/sentry/issues/derived/test_processing.py @@ -341,62 +341,94 @@ def test_store_apply_to_instance() -> None: def test_all_feature_defaults_round_trip_through_json() -> None: state = PIPELINE.initial_state() - blob = {f.name: f.dump(state[f]) for f in PIPELINE.features} + blob = {f.name: f.to_json(state[f]) for f in PIPELINE.features} serialized = json.loads(json.dumps(blob)) for f in PIPELINE.features: - assert f.load(serialized[f.name]) == state[f], f"round-trip failed for {f.name}" + assert f.from_json(serialized[f.name]) == state[f], f"round-trip failed for {f.name}" # --- Codec tests --- class TestDateTimeCodec: - def test_round_trip(self) -> None: + def test_json_round_trip(self) -> None: codec = DateTimeCodec() dt = datetime(2025, 3, 15, 12, 30, 45, tzinfo=timezone.utc) - assert codec.load(codec.dump(dt)) == dt + assert codec.from_json(codec.to_json(dt)) == dt - def test_dump_produces_iso_string(self) -> None: + def test_to_json_produces_iso_string(self) -> None: codec = DateTimeCodec() dt = datetime(2025, 3, 15, 12, 30, 45, tzinfo=timezone.utc) - dumped = codec.dump(dt) + dumped = codec.to_json(dt) assert isinstance(dumped, str) assert dumped == dt.isoformat() + def test_column_round_trip_is_identity(self) -> None: + codec = DateTimeCodec() + dt = datetime(2025, 3, 15, 12, 30, 45, tzinfo=timezone.utc) + assert codec.from_column(codec.to_column(dt)) == dt + assert codec.to_column(dt) is dt + def test_optional_none(self) -> None: codec = OptionalCodec(DateTimeCodec()) - assert codec.dump(None) is None - assert codec.load(None) is None + assert codec.to_json(None) is None + assert codec.from_json(None) is None - def test_optional_round_trip(self) -> None: + def test_optional_json_round_trip(self) -> None: codec = OptionalCodec(DateTimeCodec()) dt = datetime(2025, 3, 15, 12, 30, 45, tzinfo=timezone.utc) - assert codec.load(codec.dump(dt)) == dt + assert codec.from_json(codec.to_json(dt)) == dt class TestEnumCodecCoverage: @pytest.mark.parametrize("raw", ["open", "closed"]) - def test_issue_status_loads_known_values(self, raw: str) -> None: + def test_issue_status_json_round_trip(self, raw: str) -> None: codec = EnumCodec(IssueStatus) - loaded = codec.load(raw) - assert codec.dump(loaded) == raw + loaded = codec.from_json(raw) + assert codec.to_json(loaded) == raw + + @pytest.mark.parametrize("raw", ["open", "closed"]) + def test_issue_status_column_round_trip(self, raw: str) -> None: + codec = EnumCodec(IssueStatus) + loaded = codec.from_column(raw) + assert isinstance(loaded, IssueStatus) + assert codec.to_column(loaded) == raw @pytest.mark.parametrize( "raw", ["identified", "assigned", "diagnosed", "fix_proposed", "fix_applied"] ) - def test_issue_progress_state_loads_known_values(self, raw: str) -> None: + def test_issue_progress_state_json_round_trip(self, raw: str) -> None: codec = EnumCodec(IssueProgressState) - loaded = codec.load(raw) - assert codec.dump(loaded) == raw + loaded = codec.from_json(raw) + assert codec.to_json(loaded) == raw + + @pytest.mark.parametrize( + "raw", ["identified", "assigned", "diagnosed", "fix_proposed", "fix_applied"] + ) + def test_issue_progress_state_column_produces_enum(self, raw: str) -> None: + codec = EnumCodec(IssueProgressState) + loaded = codec.from_column(raw) + assert isinstance(loaded, IssueProgressState) + + @pytest.mark.parametrize( + "raw", + [None, "identified", "assigned", "diagnosed", "fix_proposed", "fix_applied"], + ) + def test_optional_progress_json_round_trip(self, raw: str | None) -> None: + codec = OptionalCodec(EnumCodec(IssueProgressState)) + loaded = codec.from_json(raw) + assert codec.to_json(loaded) == raw @pytest.mark.parametrize( "raw", [None, "identified", "assigned", "diagnosed", "fix_proposed", "fix_applied"], ) - def test_optional_progress_loads_known_values(self, raw: str | None) -> None: + def test_optional_progress_column_round_trip(self, raw: str | None) -> None: codec = OptionalCodec(EnumCodec(IssueProgressState)) - loaded = codec.load(raw) - assert codec.dump(loaded) == raw + loaded = codec.from_column(raw) + if raw is not None: + assert isinstance(loaded, IssueProgressState) + assert codec.to_column(loaded) == raw # --- Store tests (need DB) --- @@ -429,6 +461,7 @@ def test_load_populates_columns_and_json(self) -> None: state = GroupDerivedDataStore.load(PIPELINE, derived) assert state[VIEW_COUNT] == 3 assert state[PROGRESS] == IssueProgressState.DIAGNOSED + assert isinstance(state[PROGRESS], IssueProgressState) assert state[STATUS] == IssueStatus.CLOSED def test_load_null_progress(self) -> None: