From 44d79826af4e77a883b8faeb43fa21e6e7626908 Mon Sep 17 00:00:00 2001 From: Brandon Shippy Date: Thu, 17 Sep 2026 10:59:53 -0700 Subject: [PATCH 1/8] campaigns resource --- python/CHANGELOG.md | 3 +- .../_internal/low_level_wrappers/campaigns.py | 257 +++++++++++++ python/lib/sift_client/_tests/conftest.py | 1 + .../_tests/resources/test_campaigns.py | 327 +++++++++++++++++ .../_tests/sift_types/test_campaign.py | 195 ++++++++++ python/lib/sift_client/client.py | 7 + python/lib/sift_client/resources/__init__.py | 4 + python/lib/sift_client/resources/campaigns.py | 340 ++++++++++++++++++ .../resources/sync_stubs/__init__.py | 3 + .../resources/sync_stubs/__init__.pyi | 224 ++++++++++++ python/lib/sift_client/sift_types/__init__.py | 10 + python/lib/sift_client/sift_types/campaign.py | 234 ++++++++++++ python/lib/sift_client/sift_types/run.py | 2 + python/lib/sift_client/util/util.py | 4 + 14 files changed, 1610 insertions(+), 1 deletion(-) create mode 100644 python/lib/sift_client/_internal/low_level_wrappers/campaigns.py create mode 100644 python/lib/sift_client/_tests/resources/test_campaigns.py create mode 100644 python/lib/sift_client/_tests/sift_types/test_campaign.py create mode 100644 python/lib/sift_client/resources/campaigns.py create mode 100644 python/lib/sift_client/sift_types/campaign.py diff --git a/python/CHANGELOG.md b/python/CHANGELOG.md index 2dc891b830..67c89dc0e4 100644 --- a/python/CHANGELOG.md +++ b/python/CHANGELOG.md @@ -6,6 +6,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] - Added `client.annotations` for data reviews, phases, and review history. +- Added `client.campaigns` for grouping report results across runs. ### What's New @@ -29,7 +30,7 @@ config.complex_types_import_mode = McapComplexTypesImportMode.STRING Variable-cardinality fields (dynamic and bounded arrays) are typed `BYTES`. As with Parquet, `complex_types_import_mode` on the config decides what each becomes: Arrow IPC bytes, a JSON string under `.json`, both (the default), or neither. Reading a file locally needs the new `mcap` extra (`pip install sift-stack-py[mcap]`), so both `detect_config` and importing without a config require it. -======= + #### Credentials from sift-cli profiles `SiftClient` now reads the same `sift.toml` profiles that `sift-cli --profile` uses. An environment that you configure once for the CLI works from Python with no arguments. diff --git a/python/lib/sift_client/_internal/low_level_wrappers/campaigns.py b/python/lib/sift_client/_internal/low_level_wrappers/campaigns.py new file mode 100644 index 0000000000..e1d20c8c4e --- /dev/null +++ b/python/lib/sift_client/_internal/low_level_wrappers/campaigns.py @@ -0,0 +1,257 @@ +from __future__ import annotations + +import logging +from typing import TYPE_CHECKING, Any, cast + +from sift.campaigns.v1.campaigns_pb2 import ( + CreateCampaignFrom, + CreateCampaignResponse, + GetCampaignReportSummariesRequest, + GetCampaignReportSummariesResponse, + GetCampaignRequest, + GetCampaignResponse, + ListCampaignsRequest, + ListCampaignsResponse, + UpdateCampaignRequest, + UpdateCampaignResponse, +) +from sift.campaigns.v1.campaigns_pb2_grpc import CampaignServiceStub +from sift.common.type.v1.resource_identifier_pb2 import ( + Ids, + NamedResources, + Names, + ResourceIdentifiers, +) +from sift.tags.v1.tags_pb2 import TagRef + +from sift_client._internal.low_level_wrappers.base import DEFAULT_PAGE_SIZE, LowLevelClientBase +from sift_client.sift_types.campaign import ( + Campaign, + CampaignCreate, + CampaignReport, + CampaignUpdate, + tag_names, +) +from sift_client.transport import WithGrpcClient + +if TYPE_CHECKING: + from sift_client.transport.grpc_transport import GrpcClient + +# Configure logging +logger = logging.getLogger(__name__) + + +class CampaignsLowLevelClient(LowLevelClientBase, WithGrpcClient): + """Low-level client for the CampaignsAPI. + + A thin wrapper around the autogenerated bindings for the CampaignService. + """ + + def __init__(self, grpc_client: GrpcClient): + """Initialize the CampaignsLowLevelClient. + + Args: + grpc_client: The gRPC client to use for making API calls. + """ + super().__init__(grpc_client) + + async def get_campaign( + self, + *, + campaign_id: str | None = None, + client_key: str | None = None, + organization_id: str | None = None, + skip_report_summaries: bool = False, + ) -> Campaign: + """Get a campaign by ID or client key. + + Args: + campaign_id: The campaign ID to get. + client_key: The client key to get, as an alternative to the ID. + organization_id: Required with `client_key` if you belong to several orgs. + skip_report_summaries: Omit the per-report counts. Much faster for large campaigns. + + Returns: + The Campaign. + + Raises: + ValueError: If neither identifier is provided. + """ + if campaign_id is None and client_key is None: + raise ValueError("Either campaign_id or client_key must be provided") + + request_kwargs: dict[str, Any] = {"skip_report_summaries": skip_report_summaries} + if campaign_id is not None: + request_kwargs["campaign_id"] = campaign_id + if client_key is not None: + request_kwargs["client_key"] = client_key + if organization_id is not None: + request_kwargs["organization_id"] = organization_id + + request = GetCampaignRequest(**request_kwargs) + response = await self._grpc_client.get_stub(CampaignServiceStub).GetCampaign(request) + return Campaign._from_proto(cast("GetCampaignResponse", response).campaign) + + async def list_campaigns( + self, + *, + page_size: int | None = DEFAULT_PAGE_SIZE, + page_token: str | None = None, + query_filter: str | None = None, + order_by: str | None = None, + include_archived: bool = False, + ) -> tuple[list[Campaign], str]: + """List one page of campaigns. + + Args: + page_size: The maximum number of campaigns to return. + page_token: A page token for pagination. + query_filter: A CEL filter string. + order_by: How to order the retrieved campaigns. + include_archived: Include archived campaigns in the results. + + Returns: + A tuple of (campaigns, next_page_token). + """ + # ListCampaigns gates archived rows on its own field, not on the CEL filter. + request_kwargs: dict[str, Any] = {"include_archived": include_archived} + if page_size is not None: + request_kwargs["page_size"] = page_size + if page_token is not None: + request_kwargs["page_token"] = page_token + if query_filter is not None: + request_kwargs["filter"] = query_filter + if order_by is not None: + request_kwargs["order_by"] = order_by + + request = ListCampaignsRequest(**request_kwargs) + response = await self._grpc_client.get_stub(CampaignServiceStub).ListCampaigns(request) + response = cast("ListCampaignsResponse", response) + + campaigns = [Campaign._from_proto(c) for c in response.campaigns] + return campaigns, response.next_page_token + + async def list_all_campaigns( + self, + *, + query_filter: str | None = None, + order_by: str | None = None, + page_size: int | None = DEFAULT_PAGE_SIZE, + max_results: int | None = None, + include_archived: bool = False, + ) -> list[Campaign]: + """List every matching campaign. + + Args: + query_filter: A CEL filter string. + order_by: How to order the retrieved campaigns. + page_size: The number of results to fetch per request. + max_results: Maximum number of results to return. + include_archived: Include archived campaigns in the results. + + Returns: + A list of all matching campaigns. + """ + return await self._handle_pagination( + self.list_campaigns, + kwargs={"query_filter": query_filter, "include_archived": include_archived}, + order_by=order_by, + max_results=max_results, + page_size=page_size, + ) + + async def create_campaign( + self, + *, + create: CampaignCreate, + from_report_ids: list[str] | None = None, + from_run_ids: list[str] | None = None, + from_campaign_id: str | None = None, + ) -> Campaign: + """Create a campaign, optionally seeded with reports. + + At most one seed may be given. `from_run_ids` lets the server collect the + reports those runs generated. + + Args: + create: The campaign definition. + from_report_ids: Seed with these reports. + from_run_ids: Seed with the reports these runs generated. + from_campaign_id: Duplicate this campaign. + + Returns: + The created Campaign. + + Raises: + ValueError: If more than one seed is provided. + """ + seeds = [s for s in (from_report_ids, from_run_ids, from_campaign_id) if s] + if len(seeds) > 1: + raise ValueError( + "At most one of from_report_ids, from_run_ids, or from_campaign_id may be provided" + ) + + request_proto = create.to_proto() + if create.tags: + request_proto.tags.CopyFrom( + NamedResources(names=Names(names=tag_names(create.tags))) # type: ignore[arg-type] + ) + if from_report_ids: + request_proto.create_from.CopyFrom( + CreateCampaignFrom(reports=ResourceIdentifiers(ids=Ids(ids=from_report_ids))) + ) + elif from_run_ids: + request_proto.create_from.CopyFrom( + CreateCampaignFrom(runs=ResourceIdentifiers(ids=Ids(ids=from_run_ids))) + ) + elif from_campaign_id: + request_proto.create_from.other_campaign.id = from_campaign_id + + response = await self._grpc_client.get_stub(CampaignServiceStub).CreateCampaign( + request_proto + ) + return Campaign._from_proto(cast("CreateCampaignResponse", response).campaign) + + async def update_campaign(self, update: CampaignUpdate) -> Campaign: + """Update select fields of a campaign. + + Args: + update: The updates to apply. Its `resource_id` must be set. + + Returns: + The updated Campaign. + """ + grpc_campaign, update_mask = update.to_proto_with_mask() + if "tags" in update.model_fields_set: + grpc_campaign.tags.extend(TagRef(name=name) for name in tag_names(update.tags)) + update_mask.paths.append("tags") + request = UpdateCampaignRequest(campaign=grpc_campaign, update_mask=update_mask) + response = await self._grpc_client.get_stub(CampaignServiceStub).UpdateCampaign(request) + return Campaign._from_proto(cast("UpdateCampaignResponse", response).campaign) + + async def get_report_summaries( + self, *, campaign_ids: list[str], organization_id: str | None = None + ) -> dict[str, list[CampaignReport]]: + """Get per-report rule counts for several campaigns at once. + + Args: + campaign_ids: The campaigns to summarize. + organization_id: Required if you belong to several organizations. + + Returns: + A mapping of campaign ID to its reports, with counts populated. + """ + request_kwargs: dict[str, Any] = {"campaign_ids": campaign_ids} + if organization_id is not None: + request_kwargs["organization_id"] = organization_id + + request = GetCampaignReportSummariesRequest(**request_kwargs) + response = await self._grpc_client.get_stub(CampaignServiceStub).GetCampaignReportSummaries( + request + ) + response = cast("GetCampaignReportSummariesResponse", response) + + return { + campaign_id: [CampaignReport._from_proto(r) for r in reports.reports] + for campaign_id, reports in response.summaries_by_campaign_id.items() + } diff --git a/python/lib/sift_client/_tests/conftest.py b/python/lib/sift_client/_tests/conftest.py index 28d3ee11d2..e3bfaba776 100644 --- a/python/lib/sift_client/_tests/conftest.py +++ b/python/lib/sift_client/_tests/conftest.py @@ -85,6 +85,7 @@ def mock_client(): client.runs = MagicMock() client.channels = MagicMock() client.calculated_channels = MagicMock() + client.campaigns = MagicMock() client.rules = MagicMock() client.access_control = MagicMock() client.tags = MagicMock() diff --git a/python/lib/sift_client/_tests/resources/test_campaigns.py b/python/lib/sift_client/_tests/resources/test_campaigns.py new file mode 100644 index 0000000000..3b6931b967 --- /dev/null +++ b/python/lib/sift_client/_tests/resources/test_campaigns.py @@ -0,0 +1,327 @@ +"""Pytest tests for the Campaigns API. + +These tests cover get, list, find, create, get_or_create, update, add_reports, +add_runs, archive/unarchive, and report_summaries. +""" + +from datetime import datetime, timezone + +import pytest + +from sift_client import SiftClient +from sift_client.resources import CampaignsAPI, CampaignsAPIAsync +from sift_client.sift_types import Campaign +from sift_client.sift_types.campaign import CampaignCreate, CampaignUpdate + +pytestmark = pytest.mark.integration + + +def test_client_binding(sift_client): + assert sift_client.campaigns + assert isinstance(sift_client.campaigns, CampaignsAPI) + assert sift_client.async_.campaigns + assert isinstance(sift_client.async_.campaigns, CampaignsAPIAsync) + + +@pytest.fixture +def campaigns_api_async(sift_client: SiftClient): + """Get the async campaigns API instance.""" + return sift_client.async_.campaigns + + +@pytest.fixture +def campaigns_api_sync(sift_client: SiftClient): + """Get the synchronous campaigns API instance.""" + return sift_client.campaigns + + +@pytest.fixture(scope="session") +def test_timestamp_str(): + """A per-session suffix so campaign names stay unique across runs.""" + return datetime.now(timezone.utc).isoformat() + + +@pytest.fixture(scope="session") +def campaign_run(sift_client, test_timestamp_str): + """A run with a default report, so it can join a campaign.""" + from sift_client.sift_types.run import RunCreate + + return sift_client.runs.create( + RunCreate( + name=f"test_campaign_run_{test_timestamp_str}", + description="sift_client campaign tests", + create_default_report=True, + ) + ) + + +@pytest.fixture(scope="session") +def new_campaign(sift_client, test_timestamp_str): + """Create a campaign for the session and archive it on teardown.""" + created = sift_client.campaigns.create( + CampaignCreate( + name=f"test_campaign_{test_timestamp_str}", + description="sift_client campaign tests", + client_key=f"test_campaign_key_{test_timestamp_str}", + tags=["sift-client-pytest"], + metadata={"suite": "campaigns"}, + ) + ) + yield created + sift_client.campaigns.archive(created) + + +class TestCampaigns: + """Tests for the Campaigns API.""" + + def test_create(self, new_campaign, test_timestamp_str): + """Test that create returns a fully populated campaign.""" + assert isinstance(new_campaign, Campaign) + assert new_campaign.id_ is not None + assert new_campaign.name == f"test_campaign_{test_timestamp_str}" + assert new_campaign.description == "sift_client campaign tests" + assert new_campaign.client_key == f"test_campaign_key_{test_timestamp_str}" + assert new_campaign.tags == ["sift-client-pytest"] + assert new_campaign.metadata == {"suite": "campaigns"} + assert new_campaign.is_archived is False + assert new_campaign.organization_id + + def test_get_by_id(self, campaigns_api_sync, new_campaign): + """Test getting a campaign by ID.""" + fetched = campaigns_api_sync.get(new_campaign._id_or_error) + + assert isinstance(fetched, Campaign) + assert fetched.id_ == new_campaign.id_ + + def test_get_by_client_key(self, campaigns_api_sync, new_campaign): + """Test getting a campaign by client key.""" + fetched = campaigns_api_sync.get(client_key=new_campaign.client_key) + + assert fetched.id_ == new_campaign.id_ + + def test_get_requires_an_identifier(self, campaigns_api_sync): + """Test that get rejects a call with neither identifier.""" + with pytest.raises(ValueError, match="Either campaign_id or client_key"): + campaigns_api_sync.get() + + def test_basic_list(self, campaigns_api_sync, new_campaign): + """Test basic campaign listing functionality.""" + campaigns = campaigns_api_sync.list_(limit=5) + + assert isinstance(campaigns, list) + assert len(campaigns) >= 1 + for campaign in campaigns: + assert isinstance(campaign, Campaign) + assert campaign.id_ is not None + + def test_list_with_name_filter(self, campaigns_api_sync, new_campaign): + """Test campaign listing with name filtering.""" + by_name = campaigns_api_sync.list_(name=new_campaign.name) + by_contains = campaigns_api_sync.list_(name_contains=new_campaign.name) + + assert len(by_name) == 1 + assert by_name[0].id_ == new_campaign.id_ + assert by_contains[0].id_ == new_campaign.id_ + + def test_list_with_id_filter(self, campaigns_api_sync, new_campaign): + """Test campaign listing filtered to specific IDs.""" + campaigns = campaigns_api_sync.list_(campaign_ids=[new_campaign._id_or_error]) + + assert len(campaigns) == 1 + assert campaigns[0].id_ == new_campaign.id_ + + def test_list_with_client_key_filter(self, campaigns_api_sync, new_campaign): + """Test campaign listing filtered by client key.""" + campaigns = campaigns_api_sync.list_(client_keys=[new_campaign.client_key]) + + assert len(campaigns) == 1 + assert campaigns[0].id_ == new_campaign.id_ + + def test_list_with_tag_filter(self, campaigns_api_sync, new_campaign): + """Test campaign listing filtered by tag.""" + campaigns = campaigns_api_sync.list_( + campaign_ids=[new_campaign._id_or_error], tags=["sift-client-pytest"] + ) + + assert len(campaigns) == 1 + + def test_find(self, campaigns_api_sync, new_campaign): + """Test finding a single campaign.""" + found = campaigns_api_sync.find(name=new_campaign.name) + + assert found is not None + assert found.id_ == new_campaign.id_ + + def test_find_nonexistent(self, campaigns_api_sync): + """Test finding a non-existent campaign returns None.""" + found = campaigns_api_sync.find( + name=f"nonexistent_campaign_{datetime.now(timezone.utc).timestamp()}" + ) + assert found is None + + def test_update(self, campaigns_api_sync, new_campaign, test_timestamp_str): + """Test updating a campaign's description.""" + updated = campaigns_api_sync.update( + new_campaign, CampaignUpdate(description="updated description") + ) + + assert updated.id_ == new_campaign.id_ + assert updated.description == "updated description" + # The name was not in the mask, so it is unchanged. + assert updated.name == new_campaign.name + + campaigns_api_sync.update(new_campaign, {"description": "sift_client campaign tests"}) + + def test_update_accepts_dict(self, campaigns_api_sync, new_campaign, test_timestamp_str): + """Test that update accepts a plain dict.""" + renamed = f"test_campaign_renamed_{test_timestamp_str}" + updated = campaigns_api_sync.update(new_campaign._id_or_error, {"name": renamed}) + + assert updated.name == renamed + + campaigns_api_sync.update(new_campaign, {"name": new_campaign.name}) + + def test_get_or_create(self, campaigns_api_sync, new_campaign): + """Test that get_or_create returns the existing campaign for a known key.""" + existing = campaigns_api_sync.get_or_create( + CampaignCreate(name="ignored", client_key=new_campaign.client_key) + ) + + assert existing.id_ == new_campaign.id_ + + def test_get_or_create_creates(self, campaigns_api_sync, test_timestamp_str): + """Test that get_or_create creates a campaign for an unknown key.""" + key = f"test_campaign_goc_{test_timestamp_str}" + created = campaigns_api_sync.get_or_create( + CampaignCreate(name=f"test_campaign_goc_{test_timestamp_str}", client_key=key) + ) + + assert created.client_key == key + assert campaigns_api_sync.get_or_create({"name": "ignored", "client_key": key}).id_ == ( + created.id_ + ) + + campaigns_api_sync.archive(created) + + def test_get_or_create_requires_client_key(self, campaigns_api_sync): + """Test that get_or_create rejects a create with no client key.""" + with pytest.raises(ValueError, match="requires a client_key"): + campaigns_api_sync.get_or_create(CampaignCreate(name="no key")) + + def test_add_reports(self, campaigns_api_sync, campaign_run, test_timestamp_str): + """Test adding reports to a campaign without dropping the existing ones.""" + campaign = campaigns_api_sync.create( + CampaignCreate(name=f"test_campaign_reports_{test_timestamp_str}") + ) + report_id = campaign_run.default_report_id + + added = campaigns_api_sync.add_reports(campaign, [report_id]) + assert added.report_ids == [report_id] + + # Adding the same report again is a no-op, not a duplicate. + again = campaigns_api_sync.add_reports(added, [report_id]) + assert again.report_ids == [report_id] + + campaigns_api_sync.archive(campaign) + + def test_add_runs(self, campaigns_api_sync, campaign_run, test_timestamp_str): + """Test adding runs to a campaign through their default reports.""" + campaign = campaigns_api_sync.create( + CampaignCreate(name=f"test_campaign_runs_{test_timestamp_str}") + ) + + added = campaigns_api_sync.add_runs(campaign, [campaign_run]) + assert added.report_ids == [campaign_run.default_report_id] + + campaigns_api_sync.archive(campaign) + + def test_add_runs_rejects_run_without_report( + self, campaigns_api_sync, sift_client, new_campaign, test_timestamp_str + ): + """Test that add_runs fails clearly when a run has no default report.""" + from sift_client.sift_types.run import RunCreate + + run = sift_client.runs.create( + RunCreate(name=f"test_campaign_no_report_{test_timestamp_str}") + ) + + with pytest.raises(ValueError, match="no default report"): + campaigns_api_sync.add_runs(new_campaign, [run]) + + def test_create_from_runs(self, campaigns_api_sync, campaign_run, test_timestamp_str): + """Test seeding a new campaign from a run.""" + campaign = campaigns_api_sync.create( + CampaignCreate(name=f"test_campaign_from_runs_{test_timestamp_str}"), + runs=[campaign_run], + ) + + assert campaign.report_ids == [campaign_run.default_report_id] + + campaigns_api_sync.archive(campaign) + + def test_create_rejects_multiple_seeds(self, campaigns_api_sync, campaign_run): + """Test that create rejects more than one seed.""" + with pytest.raises(ValueError, match="At most one of"): + campaigns_api_sync.create( + CampaignCreate(name="ignored"), + runs=[campaign_run], + reports=[campaign_run.default_report_id], + ) + + def test_report_summaries(self, campaigns_api_sync, campaign_run, test_timestamp_str): + """Test fetching per-report rule counts for a campaign.""" + campaign = campaigns_api_sync.create( + CampaignCreate(name=f"test_campaign_summaries_{test_timestamp_str}"), + runs=[campaign_run], + ) + + summaries = campaigns_api_sync.report_summaries([campaign]) + + assert campaign._id_or_error in summaries + assert [r.report_id for r in summaries[campaign._id_or_error]] == campaign.report_ids + + campaigns_api_sync.archive(campaign) + + def test_archive_and_unarchive(self, campaigns_api_sync, test_timestamp_str): + """Test archiving and unarchiving a campaign.""" + campaign = campaigns_api_sync.create( + CampaignCreate(name=f"test_campaign_archive_{test_timestamp_str}") + ) + + archived = campaigns_api_sync.archive(campaign) + assert archived.is_archived is True + + # Archived campaigns are excluded from list_ by default. + assert campaigns_api_sync.find(name=campaign.name) is None + assert ( + campaigns_api_sync.find(name=campaign.name, include_archived=True).id_ == campaign.id_ + ) + + unarchived = campaigns_api_sync.unarchive(campaign) + assert unarchived.is_archived is False + + campaigns_api_sync.archive(campaign) + + def test_instance_methods(self, campaigns_api_sync, test_timestamp_str): + """Test the update and archive methods on the Campaign instance itself.""" + campaign = campaigns_api_sync.create( + CampaignCreate(name=f"test_campaign_instance_{test_timestamp_str}") + ) + + campaign.update({"description": "set from the instance"}) + assert campaign.description == "set from the instance" + + campaign.archive() + assert campaign.is_archived is True + + campaign.unarchive() + assert campaign.is_archived is False + + campaign.archive() + + @pytest.mark.asyncio + async def test_async_api(self, campaigns_api_async, new_campaign): + """Test that the async API returns the same campaign.""" + fetched = await campaigns_api_async.get(new_campaign._id_or_error) + + assert fetched.id_ == new_campaign.id_ diff --git a/python/lib/sift_client/_tests/sift_types/test_campaign.py b/python/lib/sift_client/_tests/sift_types/test_campaign.py new file mode 100644 index 0000000000..f0fd0709c4 --- /dev/null +++ b/python/lib/sift_client/_tests/sift_types/test_campaign.py @@ -0,0 +1,195 @@ +"""Tests for sift_types.Campaign model.""" + +from datetime import datetime, timezone +from unittest.mock import MagicMock + +import pytest + +from sift_client.sift_types import Campaign +from sift_client.sift_types.campaign import ( + CampaignCreate, + CampaignReport, + CampaignUpdate, +) + +NOW = datetime(2026, 1, 1, tzinfo=timezone.utc) + + +class TestCampaignReport: + """Unit tests for CampaignReport.""" + + def test_counts_default_to_zero(self): + report = CampaignReport(report_id="r-1") + + assert report.num_annotations == 0 + assert report.report_name == "" + + def test_from_proto(self): + from sift.campaigns.v1.campaigns_pb2 import CampaignReport as Proto + + report = CampaignReport._from_proto( + Proto(report_id="r-1", report_name="nightly", num_failed_rules=2) + ) + + assert report.report_id == "r-1" + assert report.report_name == "nightly" + assert report.num_failed_rules == 2 + + +class TestCampaignCreate: + """Unit tests for CampaignCreate - tests _to_proto_helpers.""" + + def test_minimal_create(self): + proto = CampaignCreate(name="Q1 regression").to_proto() + + assert proto.name == "Q1 regression" + + def test_client_key(self): + proto = CampaignCreate(name="Q1", client_key="q1_2026").to_proto() + + assert proto.client_key == "q1_2026" + + def test_metadata_converter(self): + proto = CampaignCreate(name="Q1", metadata={"owner": "ops", "n": 3.0}).to_proto() + + by_key = {m.key.name: m for m in proto.metadata} + assert by_key["owner"].string_value == "ops" + assert by_key["n"].number_value == 3.0 + + +class TestCampaignUpdate: + """Unit tests for CampaignUpdate - tests field masks.""" + + def test_update_mask_only_includes_set_fields(self): + update = CampaignUpdate(name="renamed", description="why") + update.resource_id = "c-1" + + proto, mask = update.to_proto_with_mask() + + assert proto.campaign_id == "c-1" + assert proto.name == "renamed" + assert set(mask.paths) == {"name", "description"} + + def test_reports_converter_sends_only_ids(self): + update = CampaignUpdate( + reports=[ + CampaignReport(report_id="r-1", report_name="ignored", num_failed_rules=9), + CampaignReport(report_id="r-2"), + ] + ) + update.resource_id = "c-1" + + proto, mask = update.to_proto_with_mask() + + assert [r.report_id for r in proto.reports] == ["r-1", "r-2"] + # Name and counts are output only, so they must not be sent. + assert proto.reports[0].report_name == "" + assert proto.reports[0].num_failed_rules == 0 + assert mask.paths == ["reports"] + + def test_archive_update(self): + update = CampaignUpdate(is_archived=True) + update.resource_id = "c-1" + + proto, mask = update.to_proto_with_mask() + + assert proto.is_archived is True + assert mask.paths == ["is_archived"] + + def test_requires_resource_id(self): + with pytest.raises(ValueError, match="Resource ID must be set"): + CampaignUpdate(name="renamed").to_proto_with_mask() + + +@pytest.fixture +def mock_campaign(mock_client): + """Create a mock Campaign instance for testing.""" + campaign = Campaign( + proto=MagicMock(), + id_="c-1", + name="Q1 regression", + organization_id="org1", + reports=[CampaignReport(report_id="r-1"), CampaignReport(report_id="r-2")], + tags=[], + metadata={}, + created_date=NOW, + modified_date=NOW, + created_by_user_id="user1", + modified_by_user_id="user1", + is_archived=False, + reports_include_summaries=False, + description=None, + client_key="q1_2026", + created_from_campaign_id=None, + archived_date=None, + ) + campaign._apply_client_to_instance(mock_client) + return campaign + + +class TestCampaign: + """Unit tests for Campaign model - tests properties and methods.""" + + def test_report_ids(self, mock_campaign): + assert mock_campaign.report_ids == ["r-1", "r-2"] + + def test_resolved_reports_calls_client(self, mock_campaign, mock_client): + mock_client.reports.list_.return_value = [] + + _ = mock_campaign.resolved_reports + + mock_client.reports.list_.assert_called_once_with(report_ids=["r-1", "r-2"]) + + def test_update_calls_client_and_updates_self(self, mock_campaign, mock_client): + updated = MagicMock() + mock_client.campaigns.update.return_value = updated + + with MagicMock() as mock_update: + mock_campaign._update = mock_update + + update = CampaignUpdate(name="renamed") + result = mock_campaign.update(update) + + mock_client.campaigns.update.assert_called_once_with( + campaign=mock_campaign, update=update + ) + mock_update.assert_called_once_with(updated) + assert result is mock_campaign + + def test_archive_calls_client(self, mock_campaign, mock_client): + mock_client.campaigns.archive.return_value = MagicMock() + + with MagicMock() as mock_update: + mock_campaign._update = mock_update + result = mock_campaign.archive() + + mock_client.campaigns.archive.assert_called_once_with(campaign=mock_campaign) + assert result is mock_campaign + + def test_unarchive_calls_client(self, mock_campaign, mock_client): + mock_client.campaigns.unarchive.return_value = MagicMock() + + with MagicMock() as mock_update: + mock_campaign._update = mock_update + result = mock_campaign.unarchive() + + mock_client.campaigns.unarchive.assert_called_once_with(campaign=mock_campaign) + assert result is mock_campaign + + +class TestRunCreateDefaultReport: + """A run needs a default report before it can join a campaign.""" + + def test_create_default_report_reaches_the_proto(self): + from sift_client.sift_types.run import RunCreate + + proto = RunCreate(name="r", create_default_report=True).to_proto() + + assert proto.create_default_report is True + + def test_unset_by_default(self): + from sift_client.sift_types.run import RunCreate + + proto = RunCreate(name="r").to_proto() + + assert proto.HasField("create_default_report") is False diff --git a/python/lib/sift_client/client.py b/python/lib/sift_client/client.py index dc1a46e8ba..0625293150 100644 --- a/python/lib/sift_client/client.py +++ b/python/lib/sift_client/client.py @@ -15,6 +15,8 @@ AssetsAPIAsync, CalculatedChannelsAPI, CalculatedChannelsAPIAsync, + CampaignsAPI, + CampaignsAPIAsync, ChannelsAPI, ChannelsAPIAsync, DataExportAPI, @@ -112,6 +114,9 @@ class SiftClient( calculated_channels: CalculatedChannelsAPI """Instance of the Calculated Channels API for making synchronous requests.""" + campaigns: CampaignsAPI + """Instance of the Campaigns API for making synchronous requests.""" + channels: ChannelsAPI """Instance of the Channels API for making synchronous requests.""" @@ -242,6 +247,7 @@ def __init__( self.annotations = AnnotationsAPI(self) self.assets = AssetsAPI(self) self.calculated_channels = CalculatedChannelsAPI(self) + self.campaigns = CampaignsAPI(self) self.channels = ChannelsAPI(self) self.file_attachments = FileAttachmentsAPI(self) self.jobs = JobsAPI(self) @@ -264,6 +270,7 @@ def __init__( annotations=AnnotationsAPIAsync(self), assets=AssetsAPIAsync(self), calculated_channels=CalculatedChannelsAPIAsync(self), + campaigns=CampaignsAPIAsync(self), channels=ChannelsAPIAsync(self), file_attachments=FileAttachmentsAPIAsync(self), ingestion=IngestionAPIAsync(self), diff --git a/python/lib/sift_client/resources/__init__.py b/python/lib/sift_client/resources/__init__.py index 99861cd669..6885f31e4e 100644 --- a/python/lib/sift_client/resources/__init__.py +++ b/python/lib/sift_client/resources/__init__.py @@ -153,6 +153,7 @@ async def main(): from sift_client.resources.annotations import AnnotationLogsAPIAsync, AnnotationsAPIAsync from sift_client.resources.assets import AssetsAPIAsync from sift_client.resources.calculated_channels import CalculatedChannelsAPIAsync +from sift_client.resources.campaigns import CampaignsAPIAsync from sift_client.resources.channels import ChannelsAPIAsync from sift_client.resources.file_attachments import FileAttachmentsAPIAsync from sift_client.resources.ingestion import ( @@ -191,6 +192,7 @@ async def main(): AnnotationsAPI, AssetsAPI, CalculatedChannelsAPI, + CampaignsAPI, ChannelsAPI, JobsAPI, PingAPI, @@ -232,6 +234,8 @@ async def main(): "AutoRegisterStreamingClient", "CalculatedChannelsAPI", "CalculatedChannelsAPIAsync", + "CampaignsAPI", + "CampaignsAPIAsync", "ChannelsAPI", "ChannelsAPIAsync", "FileAttachmentsAPI", diff --git a/python/lib/sift_client/resources/campaigns.py b/python/lib/sift_client/resources/campaigns.py new file mode 100644 index 0000000000..d27bc45a93 --- /dev/null +++ b/python/lib/sift_client/resources/campaigns.py @@ -0,0 +1,340 @@ +from __future__ import annotations + +from typing import TYPE_CHECKING, Any + +from sift_client._internal.low_level_wrappers.campaigns import CampaignsLowLevelClient +from sift_client.resources._base import ResourceBase +from sift_client.sift_types.campaign import ( + Campaign, + CampaignCreate, + CampaignReport, + CampaignUpdate, +) +from sift_client.sift_types.run import Run +from sift_client.util import cel_utils as cel + +if TYPE_CHECKING: + import re + + from sift_client.client import SiftClient + from sift_client.sift_types.report import Report + from sift_client.sift_types.tag import Tag + + +class CampaignsAPIAsync(ResourceBase): + """High-level API for interacting with campaigns. + + A campaign is a named list of reports. Runs join a campaign through the reports they + generate, so a run must be created with `create_default_report=True` to be added. + """ + + def __init__(self, sift_client: SiftClient): + """Initialize the CampaignsAPI. + + Args: + sift_client: The Sift client to use. + """ + super().__init__(sift_client) + self._low_level_client = CampaignsLowLevelClient(grpc_client=self.client.grpc_client) + + async def get( + self, + campaign_id: str | None = None, + *, + client_key: str | None = None, + organization_id: str | None = None, + skip_report_summaries: bool = False, + ) -> Campaign: + """Get a Campaign by ID or client key. + + Args: + campaign_id: The ID of the campaign. + client_key: The client key, as an alternative to the ID. + organization_id: Required with `client_key` if you belong to several orgs. + skip_report_summaries: Omit the per-report counts. Much faster for large campaigns. + + Returns: + The Campaign. + """ + campaign = await self._low_level_client.get_campaign( + campaign_id=campaign_id, + client_key=client_key, + organization_id=organization_id, + skip_report_summaries=skip_report_summaries, + ) + return self._apply_client_to_instance(campaign) + + async def list_( + self, + *, + name: str | None = None, + names: list[str] | None = None, + name_contains: str | None = None, + name_regex: str | re.Pattern | None = None, + # self ids + campaign_ids: list[str] | None = None, + client_keys: list[str] | None = None, + # created/modified users + created_by: Any | str | None = None, + # tags and metadata + tags: list[str] | list[Tag] | None = None, + metadata: dict[str, Any] | None = None, + # campaign specific + reports: list[Report] | list[str] | None = None, + runs: list[Run] | list[str] | None = None, + # common filters + description_contains: str | None = None, + include_archived: bool = False, + filter_query: str | None = None, + order_by: str | None = None, + limit: int | None = None, + page_size: int | None = None, + ) -> list[Campaign]: + """List campaigns. + + Args: + name: Exact name of the campaign. + names: List of campaign names to filter by. + name_contains: Partial name of the campaign. + name_regex: Regular expression to filter campaigns by name. + campaign_ids: Filter to campaigns with any of these IDs. + client_keys: Filter to campaigns with any of these client keys. + created_by: Filter campaigns created by this user ID. + tags: Filter campaigns with any of these Tags or tag names. + metadata: Filter campaigns by metadata criteria. + reports: Filter campaigns containing any of these Reports or report IDs. + runs: Filter campaigns containing any of these Runs or run IDs. + description_contains: Partial description of the campaign. + include_archived: If True, include archived campaigns in results. + filter_query: Explicit CEL query to filter campaigns. + order_by: Field and direction to order results by. + limit: Maximum number of campaigns to return. If None, returns all matches. + page_size: Number of results to fetch per request. + + Returns: + A list of Campaign objects that match the filter criteria. + """ + filter_parts = [ + *self._build_name_cel_filters( + name=name, names=names, name_contains=name_contains, name_regex=name_regex + ), + *self._build_time_cel_filters(created_by=created_by), + *self._build_tags_metadata_cel_filters(tag_names=tags, metadata=metadata), + *self._build_common_cel_filters( + description_contains=description_contains, + filter_query=filter_query, + ), + ] + if campaign_ids: + filter_parts.append(cel.in_("campaign_id", campaign_ids)) + if client_keys: + filter_parts.append(cel.in_("client_key", client_keys)) + if reports: + filter_parts.append(cel.in_("report_id", [self._report_id(r) for r in reports])) + if runs: + run_ids = [r._id_or_error if isinstance(r, Run) else r for r in runs] + filter_parts.append(cel.in_("run_id", run_ids)) + query_filter = cel.and_(*filter_parts) + + campaigns = await self._low_level_client.list_all_campaigns( + query_filter=query_filter or None, + order_by=order_by, + max_results=limit, + include_archived=include_archived, + **({"page_size": page_size} if page_size is not None else {}), + ) + return self._apply_client_to_instances(campaigns) + + async def find(self, **kwargs) -> Campaign | None: + """Find one campaign. Takes the same arguments as `list_`. + + Raises if more than one matches. + + Args: + **kwargs: Keyword arguments to pass to `list_`. + + Returns: + The Campaign found or None. + """ + campaigns = await self.list_(**kwargs) + if len(campaigns) > 1: + raise ValueError(f"Multiple ({len(campaigns)}) campaigns found for query") + elif len(campaigns) == 1: + return campaigns[0] + return None + + async def create( + self, + create: CampaignCreate | dict, + *, + reports: list[Report] | list[str] | None = None, + runs: list[Run] | list[str] | None = None, + from_campaign: str | Campaign | None = None, + ) -> Campaign: + """Create a new campaign, optionally seeded with reports. + + At most one seed may be given. `runs` lets the server collect the reports those + runs generated, so the runs need a default report. + + Args: + create: The campaign definition. + reports: Seed with these Reports or report IDs. + runs: Seed with the reports these Runs generated. + from_campaign: Duplicate this Campaign or campaign ID. + + Returns: + The created Campaign. + """ + if isinstance(create, dict): + create = CampaignCreate.model_validate(create) + created = await self._low_level_client.create_campaign( + create=create, + from_report_ids=[self._report_id(r) for r in reports] if reports else None, + from_run_ids=( + [r._id_or_error if isinstance(r, Run) else r for r in runs] if runs else None + ), + from_campaign_id=( + from_campaign._id_or_error if isinstance(from_campaign, Campaign) else from_campaign + ), + ) + return self._apply_client_to_instance(created) + + async def get_or_create(self, create: CampaignCreate | dict) -> Campaign: + """Get the campaign with this client key, or create it. + + Args: + create: The campaign definition. Its `client_key` is required. + + Returns: + The existing or newly created Campaign. + + Raises: + ValueError: If `client_key` is not set. + """ + if isinstance(create, dict): + create = CampaignCreate.model_validate(create) + if not create.client_key: + raise ValueError("get_or_create requires a client_key") + + existing = await self.find(client_keys=[create.client_key], include_archived=True) + if existing is not None: + return existing + return await self.create(create) + + async def update(self, campaign: str | Campaign, update: CampaignUpdate | dict) -> Campaign: + """Update a Campaign. + + `reports`, `tags`, and `metadata` are replaced, not merged. Prefer `add_reports` + or `add_runs` to grow the report list. + + Args: + campaign: The Campaign or campaign ID to update. + update: Updates to apply to the Campaign. + + Returns: + The updated Campaign. + """ + campaign_id = campaign._id_or_error if isinstance(campaign, Campaign) else campaign + if isinstance(update, dict): + update = CampaignUpdate.model_validate(update) + update.resource_id = campaign_id + updated = await self._low_level_client.update_campaign(update) + return self._apply_client_to_instance(updated) + + async def add_reports( + self, campaign: str | Campaign, reports: list[Report] | list[str] + ) -> Campaign: + """Add reports to a campaign, keeping the ones already there. + + Args: + campaign: The Campaign or campaign ID to add to. + reports: The Reports or report IDs to add. + + Returns: + The updated Campaign. + """ + current = ( + campaign + if isinstance(campaign, Campaign) + else await self.get(campaign, skip_report_summaries=True) + ) + existing_ids = current.report_ids + merged = existing_ids + [ + report_id + for report_id in (self._report_id(r) for r in reports) + if report_id not in existing_ids + ] + return await self.update( + current, CampaignUpdate(reports=[CampaignReport(report_id=r) for r in merged]) + ) + + async def add_runs(self, campaign: str | Campaign, runs: list[Run] | list[str]) -> Campaign: + """Add runs to a campaign through the reports they generated. + + A campaign holds reports, not runs, so each run must have a default report. Pass + `create_default_report=True` to `RunCreate` to get one. + + Args: + campaign: The Campaign or campaign ID to add to. + runs: The Runs or run IDs to add. + + Returns: + The updated Campaign. + + Raises: + ValueError: If any run has no default report. + """ + resolved = [ + r if isinstance(r, Run) else await self.client.async_.runs.get(run_id=r) for r in runs + ] + missing = [r.name for r in resolved if not r.default_report_id] + if missing: + raise ValueError( + f"These runs have no default report, so they cannot join a campaign: {missing}. " + "Create runs with create_default_report=True." + ) + report_ids = [r.default_report_id for r in resolved if r.default_report_id] + return await self.add_reports(campaign, report_ids) + + async def archive(self, campaign: str | Campaign) -> Campaign: + """Archive a campaign. + + Args: + campaign: The Campaign or campaign ID to archive. + + Returns: + The archived Campaign. + """ + return await self.update(campaign, CampaignUpdate(is_archived=True)) + + async def unarchive(self, campaign: str | Campaign) -> Campaign: + """Unarchive a campaign. + + Args: + campaign: The Campaign or campaign ID to unarchive. + + Returns: + The unarchived Campaign. + """ + return await self.update(campaign, CampaignUpdate(is_archived=False)) + + async def report_summaries( + self, campaigns: list[str | Campaign], *, organization_id: str | None = None + ) -> dict[str, list[CampaignReport]]: + """Get per-report rule counts for several campaigns at once. + + Args: + campaigns: The Campaigns or campaign IDs to summarize. + organization_id: Required if you belong to several organizations. + + Returns: + A mapping of campaign ID to its reports, with counts populated. + """ + ids = [c._id_or_error if isinstance(c, Campaign) else c for c in campaigns] + return await self._low_level_client.get_report_summaries( + campaign_ids=ids, organization_id=organization_id + ) + + @staticmethod + def _report_id(report: Report | str) -> str: + return report if isinstance(report, str) else report._id_or_error diff --git a/python/lib/sift_client/resources/sync_stubs/__init__.py b/python/lib/sift_client/resources/sync_stubs/__init__.py index 64206b780c..8dea371b39 100644 --- a/python/lib/sift_client/resources/sync_stubs/__init__.py +++ b/python/lib/sift_client/resources/sync_stubs/__init__.py @@ -8,6 +8,7 @@ AnnotationsAPIAsync, AssetsAPIAsync, CalculatedChannelsAPIAsync, + CampaignsAPIAsync, ChannelsAPIAsync, DataExportAPIAsync, DataImportAPIAsync, @@ -39,6 +40,7 @@ ) AssetsAPI = generate_sync_api(AssetsAPIAsync, "AssetsAPI") CalculatedChannelsAPI = generate_sync_api(CalculatedChannelsAPIAsync, "CalculatedChannelsAPI") +CampaignsAPI = generate_sync_api(CampaignsAPIAsync, "CampaignsAPI") ChannelsAPI = generate_sync_api(ChannelsAPIAsync, "ChannelsAPI") FileAttachmentsAPI = generate_sync_api(FileAttachmentsAPIAsync, "FileAttachmentsAPI") JobsAPI = generate_sync_api(JobsAPIAsync, "JobsAPI") @@ -98,6 +100,7 @@ "AnnotationsAPI", "AssetsAPI", "CalculatedChannelsAPI", + "CampaignsAPI", "ChannelsAPI", "DataExportAPI", "DataImportAPI", diff --git a/python/lib/sift_client/resources/sync_stubs/__init__.pyi b/python/lib/sift_client/resources/sync_stubs/__init__.pyi index 8275d1c63a..5dd39b7305 100644 --- a/python/lib/sift_client/resources/sync_stubs/__init__.pyi +++ b/python/lib/sift_client/resources/sync_stubs/__init__.pyi @@ -34,6 +34,12 @@ if TYPE_CHECKING: CalculatedChannelCreate, CalculatedChannelUpdate, ) + from sift_client.sift_types.campaign import ( + Campaign, + CampaignCreate, + CampaignReport, + CampaignUpdate, + ) from sift_client.sift_types.channel import Channel, ChannelUpdate from sift_client.sift_types.data_import import ( DataImport, @@ -754,6 +760,224 @@ class CalculatedChannelsAPI: """ ... +class CampaignsAPI: + """Sync counterpart to `CampaignsAPIAsync`. + + High-level API for interacting with campaigns. + + A campaign is a named list of reports. Runs join a campaign through the reports they + generate, so a run must be created with `create_default_report=True` to be added. + """ + + def __init__(self, sift_client: SiftClient): + """Initialize the CampaignsAPI. + + Args: + sift_client: The Sift client to use. + """ + ... + + def _run(self, coro): ... + def add_reports(self, campaign: str | Campaign, reports: list[Report] | list[str]) -> Campaign: + """Add reports to a campaign, keeping the ones already there. + + Args: + campaign: The Campaign or campaign ID to add to. + reports: The Reports or report IDs to add. + + Returns: + The updated Campaign. + """ + ... + + def add_runs(self, campaign: str | Campaign, runs: list[Run] | list[str]) -> Campaign: + """Add runs to a campaign through the reports they generated. + + A campaign holds reports, not runs, so each run must have a default report. Pass + `create_default_report=True` to `RunCreate` to get one. + + Args: + campaign: The Campaign or campaign ID to add to. + runs: The Runs or run IDs to add. + + Returns: + The updated Campaign. + + Raises: + ValueError: If any run has no default report. + """ + ... + + def archive(self, campaign: str | Campaign) -> Campaign: + """Archive a campaign. + + Args: + campaign: The Campaign or campaign ID to archive. + + Returns: + The archived Campaign. + """ + ... + + def create( + self, + create: CampaignCreate | dict, + *, + reports: list[Report] | list[str] | None = None, + runs: list[Run] | list[str] | None = None, + from_campaign: str | Campaign | None = None, + ) -> Campaign: + """Create a new campaign, optionally seeded with reports. + + At most one seed may be given. `runs` lets the server collect the reports those + runs generated, so the runs need a default report. + + Args: + create: The campaign definition. + reports: Seed with these Reports or report IDs. + runs: Seed with the reports these Runs generated. + from_campaign: Duplicate this Campaign or campaign ID. + + Returns: + The created Campaign. + """ + ... + + def find(self, **kwargs) -> Campaign | None: + """Find one campaign. Takes the same arguments as `list_`. + + Raises if more than one matches. + + Args: + **kwargs: Keyword arguments to pass to `list_`. + + Returns: + The Campaign found or None. + """ + ... + + def get( + self, + campaign_id: str | None = None, + *, + client_key: str | None = None, + organization_id: str | None = None, + skip_report_summaries: bool = False, + ) -> Campaign: + """Get a Campaign by ID or client key. + + Args: + campaign_id: The ID of the campaign. + client_key: The client key, as an alternative to the ID. + organization_id: Required with `client_key` if you belong to several orgs. + skip_report_summaries: Omit the per-report counts. Much faster for large campaigns. + + Returns: + The Campaign. + """ + ... + + def get_or_create(self, create: CampaignCreate | dict) -> Campaign: + """Get the campaign with this client key, or create it. + + Args: + create: The campaign definition. Its `client_key` is required. + + Returns: + The existing or newly created Campaign. + + Raises: + ValueError: If `client_key` is not set. + """ + ... + + def list_( + self, + *, + name: str | None = None, + names: list[str] | None = None, + name_contains: str | None = None, + name_regex: str | re.Pattern | None = None, + campaign_ids: list[str] | None = None, + client_keys: list[str] | None = None, + created_by: Any | str | None = None, + tags: list[str] | list[Tag] | None = None, + metadata: dict[str, Any] | None = None, + reports: list[Report] | list[str] | None = None, + runs: list[Run] | list[str] | None = None, + description_contains: str | None = None, + include_archived: bool = False, + filter_query: str | None = None, + order_by: str | None = None, + limit: int | None = None, + page_size: int | None = None, + ) -> list[Campaign]: + """List campaigns. + + Args: + name: Exact name of the campaign. + names: List of campaign names to filter by. + name_contains: Partial name of the campaign. + name_regex: Regular expression to filter campaigns by name. + campaign_ids: Filter to campaigns with any of these IDs. + client_keys: Filter to campaigns with any of these client keys. + created_by: Filter campaigns created by this user ID. + tags: Filter campaigns with any of these Tags or tag names. + metadata: Filter campaigns by metadata criteria. + reports: Filter campaigns containing any of these Reports or report IDs. + runs: Filter campaigns containing any of these Runs or run IDs. + description_contains: Partial description of the campaign. + include_archived: If True, include archived campaigns in results. + filter_query: Explicit CEL query to filter campaigns. + order_by: Field and direction to order results by. + limit: Maximum number of campaigns to return. If None, returns all matches. + page_size: Number of results to fetch per request. + + Returns: + A list of Campaign objects that match the filter criteria. + """ + ... + + def report_summaries( + self, campaigns: list[str | Campaign], *, organization_id: str | None = None + ) -> dict[str, list[CampaignReport]]: + """Get per-report rule counts for several campaigns at once. + + Args: + campaigns: The Campaigns or campaign IDs to summarize. + organization_id: Required if you belong to several organizations. + + Returns: + A mapping of campaign ID to its reports, with counts populated. + """ + ... + + def unarchive(self, campaign: str | Campaign) -> Campaign: + """Unarchive a campaign. + + Args: + campaign: The Campaign or campaign ID to unarchive. + + Returns: + The unarchived Campaign. + """ + ... + + def update(self, campaign: str | Campaign, update: CampaignUpdate | dict) -> Campaign: + """Update a Campaign. + + `reports`, `tags`, and `metadata` are replaced, not merged. Prefer `add_reports` + or `add_runs` to grow the report list. + + Args: + campaign: The Campaign or campaign ID to update. + update: Updates to apply to the Campaign. + + Returns: + The updated Campaign. + """ + ... + class ChannelsAPI: """Sync counterpart to `ChannelsAPIAsync`. diff --git a/python/lib/sift_client/sift_types/__init__.py b/python/lib/sift_client/sift_types/__init__.py index f844cbc25f..4e4084d118 100644 --- a/python/lib/sift_client/sift_types/__init__.py +++ b/python/lib/sift_client/sift_types/__init__.py @@ -150,6 +150,12 @@ CalculatedChannelCreate, CalculatedChannelUpdate, ) +from sift_client.sift_types.campaign import ( + Campaign, + CampaignCreate, + CampaignReport, + CampaignUpdate, +) from sift_client.sift_types.channel import ( Channel, ChannelBitFieldElement, @@ -262,6 +268,10 @@ "CalculatedChannel", "CalculatedChannelCreate", "CalculatedChannelUpdate", + "Campaign", + "CampaignCreate", + "CampaignReport", + "CampaignUpdate", "Channel", "ChannelBitFieldElement", "ChannelConfig", diff --git a/python/lib/sift_client/sift_types/campaign.py b/python/lib/sift_client/sift_types/campaign.py new file mode 100644 index 0000000000..13862dd960 --- /dev/null +++ b/python/lib/sift_client/sift_types/campaign.py @@ -0,0 +1,234 @@ +from __future__ import annotations + +from datetime import datetime, timezone +from typing import TYPE_CHECKING, ClassVar + +from pydantic import BaseModel, Field +from sift.campaigns.v1.campaigns_pb2 import ( + Campaign as CampaignProto, +) +from sift.campaigns.v1.campaigns_pb2 import ( + CampaignReport as CampaignReportProto, +) +from sift.campaigns.v1.campaigns_pb2 import ( + CreateCampaignRequest as CreateCampaignRequestProto, +) + +from sift_client.sift_types._base import ( + BaseType, + MappingHelper, + ModelCreate, + ModelCreateUpdateBase, + ModelUpdate, +) +from sift_client.sift_types.tag import Tag +from sift_client.util.metadata import metadata_dict_to_proto, metadata_proto_to_dict + +if TYPE_CHECKING: + from sift_client.client import SiftClient + from sift_client.sift_types.report import Report + + +class CampaignReport(BaseModel): + """A report in a campaign, with a rollup of its rule outcomes. + + The counts are only populated when the campaign was fetched with summaries. + + Attributes: + report_id: The report this entry refers to. + report_name: The report's name. + num_annotations: Annotations across the report's rules. + num_passed_rules: Rules that never triggered. + num_accepted_rules: Rules whose annotations are all accepted. + num_failed_rules: Rules with any failed annotation. + num_open_rules: Rules with open annotations and none failed. + """ + + report_id: str + report_name: str = "" + num_annotations: int = 0 + num_passed_rules: int = 0 + num_accepted_rules: int = 0 + num_failed_rules: int = 0 + num_open_rules: int = 0 + + @classmethod + def _from_proto(cls, proto: CampaignReportProto) -> CampaignReport: + return cls( + report_id=proto.report_id, + report_name=proto.report_name, + num_annotations=proto.num_annotations, + num_passed_rules=proto.num_passed_rules, + num_accepted_rules=proto.num_accepted_rules, + num_failed_rules=proto.num_failed_rules, + num_open_rules=proto.num_open_rules, + ) + + +def _campaign_report_to_proto(**kwargs) -> CampaignReportProto: + """Convert a campaign report dict (from model_dump) into its proto form. + + Only `report_id` is sent; the name and counts are output only. + """ + return CampaignReportProto(report_id=kwargs["report_id"]) + + +def tag_names(tags: list[str] | list[Tag] | None) -> list[str]: + """Reduce Tags or tag names to plain names.""" + return [tag.name if isinstance(tag, Tag) else tag for tag in tags or []] + + +class Campaign(BaseType[CampaignProto, "Campaign"]): + """A campaign, which is a named list of reports. + + Runs are attached through the reports they generate, not directly. + """ + + # Required fields + name: str + organization_id: str + reports: list[CampaignReport] + tags: list[str] + metadata: dict[str, str | float | bool] + created_date: datetime + modified_date: datetime + created_by_user_id: str + modified_by_user_id: str + is_archived: bool + reports_include_summaries: bool + + # Optional fields + description: str | None + client_key: str | None + created_from_campaign_id: str | None + archived_date: datetime | None + + @classmethod + def _from_proto(cls, proto: CampaignProto, sift_client: SiftClient | None = None) -> Campaign: + return cls( + proto=proto, + id_=proto.campaign_id, + name=proto.name, + organization_id=proto.organization_id, + reports=[CampaignReport._from_proto(r) for r in proto.reports], + tags=[t.name or t.tag_id for t in proto.tags], + metadata=metadata_proto_to_dict(proto.metadata), # type: ignore + created_date=proto.created_date.ToDatetime(tzinfo=timezone.utc), + modified_date=proto.modified_date.ToDatetime(tzinfo=timezone.utc), + created_by_user_id=proto.created_by_user_id, + modified_by_user_id=proto.modified_by_user_id, + is_archived=proto.is_archived, + reports_include_summaries=proto.reports_include_summaries, + description=proto.description if proto.HasField("description") else None, + client_key=proto.client_key if proto.HasField("client_key") else None, + created_from_campaign_id=( + proto.created_from_campaign_id + if proto.HasField("created_from_campaign_id") + else None + ), + archived_date=( + proto.archived_date.ToDatetime(tzinfo=timezone.utc) + if proto.HasField("archived_date") + else None + ), + _client=sift_client, + ) + + @property + def report_ids(self) -> list[str]: + """The IDs of the reports in this campaign.""" + return [r.report_id for r in self.reports] + + @property + def resolved_reports(self) -> list[Report]: + """Fetch the full Reports in this campaign.""" + if not self.reports: + return [] + return self.client.reports.list_(report_ids=self.report_ids) + + def update(self, update: CampaignUpdate | dict) -> Campaign: + """Update the Campaign. + + Args: + update: The update to apply. See CampaignUpdate for updatable fields. + + Returns: + The updated campaign. + """ + updated = self.client.campaigns.update(campaign=self, update=update) + self._update(updated) + return self + + def archive(self) -> Campaign: + """Archive the campaign.""" + updated = self.client.campaigns.archive(campaign=self) + self._update(updated) + return self + + def unarchive(self) -> Campaign: + """Unarchive the campaign.""" + updated = self.client.campaigns.unarchive(campaign=self) + self._update(updated) + return self + + +class CampaignBase(ModelCreateUpdateBase): + """Base class for Campaign create and update models.""" + + description: str | None = None + # Create takes NamedResources and update takes repeated TagRef, so the low-level + # client sets tags on the proto rather than the shared builder. + tags: list[str] | list[Tag] | None = Field(default=None, exclude=True) + metadata: dict[str, str | float | bool] | None = None + + _to_proto_helpers: ClassVar[dict[str, MappingHelper]] = { + "metadata": MappingHelper( + proto_attr_path="metadata", + update_field="metadata", + converter=metadata_dict_to_proto, + ), + } + + +class CampaignCreate(CampaignBase, ModelCreate[CreateCampaignRequestProto]): + """Create model for Campaign. + + Pass `client_key` to make the campaign addressable by your own identifier, which is + what `get_or_create` matches on. + """ + + name: str + client_key: str | None = None + organization_id: str | None = None + + def _get_proto_class(self) -> type[CreateCampaignRequestProto]: + return CreateCampaignRequestProto + + +class CampaignUpdate(CampaignBase, ModelUpdate[CampaignProto]): + """Update model for Campaign. + + `reports` replaces the campaign's report list. Prefer `campaigns.add_reports` or + `campaigns.add_runs`, which read the current list first. + """ + + name: str | None = None + reports: list[CampaignReport] | None = None + is_archived: bool | None = None + + _to_proto_helpers: ClassVar[dict[str, MappingHelper]] = { + **CampaignBase._to_proto_helpers, + "reports": MappingHelper( + proto_attr_path="reports", + update_field="reports", + converter=_campaign_report_to_proto, # type: ignore[arg-type] + ), + } + + def _get_proto_class(self) -> type[CampaignProto]: + return CampaignProto + + def _add_resource_id_to_proto(self, proto_msg: CampaignProto): + if self._resource_id is None: + raise ValueError("Resource ID must be set before adding to proto") + proto_msg.campaign_id = self._resource_id diff --git a/python/lib/sift_client/sift_types/run.py b/python/lib/sift_client/sift_types/run.py index b61968d7b8..7f70047167 100644 --- a/python/lib/sift_client/sift_types/run.py +++ b/python/lib/sift_client/sift_types/run.py @@ -223,6 +223,8 @@ class RunCreate(RunBase, ModelCreate[CreateRunRequestProto]): name: str client_key: str | None = None organization_id: str | None = None + create_default_report: bool | None = None + """Create a report for the run, so it can be added to a campaign.""" def _get_proto_class(self) -> type[CreateRunRequestProto]: return CreateRunRequestProto diff --git a/python/lib/sift_client/util/util.py b/python/lib/sift_client/util/util.py index 73253bcd3b..b999e0ae63 100644 --- a/python/lib/sift_client/util/util.py +++ b/python/lib/sift_client/util/util.py @@ -7,6 +7,7 @@ AnnotationsAPIAsync, AssetsAPIAsync, CalculatedChannelsAPIAsync, + CampaignsAPIAsync, ChannelsAPIAsync, DataExportAPIAsync, DataImportAPIAsync, @@ -39,6 +40,9 @@ class AsyncAPIs(NamedTuple): calculated_channels: CalculatedChannelsAPIAsync """Instance of the Calculated Channels API for making asynchronous requests.""" + campaigns: CampaignsAPIAsync + """Instance of the Campaigns API for making asynchronous requests.""" + channels: ChannelsAPIAsync """Instance of the Channels API for making asynchronous requests.""" From d62c6f48f099c440007ce0bab8e33e32cad8dfb8 Mon Sep 17 00:00:00 2001 From: Brandon Shippy Date: Thu, 17 Sep 2026 15:57:21 -0700 Subject: [PATCH 2/8] campaigns: add runs and report_summaries, fix stale add_reports --- .../_internal/low_level_wrappers/campaigns.py | 3 +- .../_tests/resources/test_campaigns.py | 67 +++++++++++++++++++ python/lib/sift_client/resources/campaigns.py | 16 +++-- .../resources/sync_stubs/__init__.pyi | 4 +- python/lib/sift_client/sift_types/campaign.py | 23 ++++++- 5 files changed, 101 insertions(+), 12 deletions(-) diff --git a/python/lib/sift_client/_internal/low_level_wrappers/campaigns.py b/python/lib/sift_client/_internal/low_level_wrappers/campaigns.py index e1d20c8c4e..efa9a3647f 100644 --- a/python/lib/sift_client/_internal/low_level_wrappers/campaigns.py +++ b/python/lib/sift_client/_internal/low_level_wrappers/campaigns.py @@ -113,7 +113,6 @@ async def list_campaigns( Returns: A tuple of (campaigns, next_page_token). """ - # ListCampaigns gates archived rows on its own field, not on the CEL filter. request_kwargs: dict[str, Any] = {"include_archived": include_archived} if page_size is not None: request_kwargs["page_size"] = page_size @@ -225,6 +224,8 @@ async def update_campaign(self, update: CampaignUpdate) -> Campaign: if "tags" in update.model_fields_set: grpc_campaign.tags.extend(TagRef(name=name) for name in tag_names(update.tags)) update_mask.paths.append("tags") + if not update_mask.paths: + return await self.get_campaign(campaign_id=grpc_campaign.campaign_id) request = UpdateCampaignRequest(campaign=grpc_campaign, update_mask=update_mask) response = await self._grpc_client.get_stub(CampaignServiceStub).UpdateCampaign(request) return Campaign._from_proto(cast("UpdateCampaignResponse", response).campaign) diff --git a/python/lib/sift_client/_tests/resources/test_campaigns.py b/python/lib/sift_client/_tests/resources/test_campaigns.py index 3b6931b967..837b4b88bc 100644 --- a/python/lib/sift_client/_tests/resources/test_campaigns.py +++ b/python/lib/sift_client/_tests/resources/test_campaigns.py @@ -325,3 +325,70 @@ async def test_async_api(self, campaigns_api_async, new_campaign): fetched = await campaigns_api_async.get(new_campaign._id_or_error) assert fetched.id_ == new_campaign.id_ + + def test_add_reports_reads_the_server_list( + self, campaigns_api_sync, campaign_run, test_timestamp_str + ): + """A stale Campaign handle must not drop reports added since it was fetched.""" + campaign = campaigns_api_sync.create( + CampaignCreate(name=f"test_campaign_stale_{test_timestamp_str}") + ) + stale = campaign + + # Another caller adds a report that `stale` knows nothing about. + campaigns_api_sync.add_reports(campaign._id_or_error, [campaign_run.default_report_id]) + assert stale.report_ids == [] + + merged = campaigns_api_sync.add_reports(stale, [campaign_run.default_report_id]) + + assert merged.report_ids == [campaign_run.default_report_id] + + campaigns_api_sync.archive(campaign) + + def test_campaign_runs(self, campaigns_api_sync, campaign_run, test_timestamp_str): + """Test reading back the runs behind a campaign's reports.""" + campaign = campaigns_api_sync.create( + CampaignCreate(name=f"test_campaign_runs_prop_{test_timestamp_str}"), + runs=[campaign_run], + ) + + assert [r.id_ for r in campaign.runs] == [campaign_run.id_] + + campaigns_api_sync.archive(campaign) + + def test_campaign_report_summaries(self, campaigns_api_sync, campaign_run, test_timestamp_str): + """Test the rollup from the Campaign instance.""" + campaign = campaigns_api_sync.create( + CampaignCreate(name=f"test_campaign_rollup_{test_timestamp_str}"), + runs=[campaign_run], + ) + + # The service returns these in no fixed order; the instance method sorts them. + assert [s.report_id for s in campaign.report_summaries()] == campaign.report_ids + + campaigns_api_sync.archive(campaign) + + def test_update_with_nothing_set_is_a_noop(self, campaigns_api_sync, new_campaign): + """The service rejects an empty update mask, so the client must not send one.""" + unchanged = campaigns_api_sync.update(new_campaign, CampaignUpdate()) + + assert unchanged.id_ == new_campaign.id_ + assert unchanged.name == new_campaign.name + + def test_report_summaries_with_no_campaigns(self, campaigns_api_sync): + """An empty request must not reach the service, which rejects it.""" + assert campaigns_api_sync.report_summaries([]) == {} + + def test_report_summaries_includes_empty_campaigns( + self, campaigns_api_sync, test_timestamp_str + ): + """The service omits campaigns with no reports; every ID asked for is returned.""" + campaign = campaigns_api_sync.create( + CampaignCreate(name=f"test_campaign_empty_rollup_{test_timestamp_str}") + ) + + summaries = campaigns_api_sync.report_summaries([campaign]) + + assert summaries[campaign._id_or_error] == [] + + campaigns_api_sync.archive(campaign) diff --git a/python/lib/sift_client/resources/campaigns.py b/python/lib/sift_client/resources/campaigns.py index d27bc45a93..e1776c476f 100644 --- a/python/lib/sift_client/resources/campaigns.py +++ b/python/lib/sift_client/resources/campaigns.py @@ -253,11 +253,8 @@ async def add_reports( Returns: The updated Campaign. """ - current = ( - campaign - if isinstance(campaign, Campaign) - else await self.get(campaign, skip_report_summaries=True) - ) + campaign_id = campaign._id_or_error if isinstance(campaign, Campaign) else campaign + current = await self.get(campaign_id, skip_report_summaries=True) existing_ids = current.report_ids merged = existing_ids + [ report_id @@ -328,12 +325,17 @@ async def report_summaries( organization_id: Required if you belong to several organizations. Returns: - A mapping of campaign ID to its reports, with counts populated. + A mapping of campaign ID to its reports, with counts populated. The service + returns each campaign's reports in no fixed order; `Campaign.report_summaries` + orders them to match the campaign. """ ids = [c._id_or_error if isinstance(c, Campaign) else c for c in campaigns] - return await self._low_level_client.get_report_summaries( + if not ids: + return {} + found = await self._low_level_client.get_report_summaries( campaign_ids=ids, organization_id=organization_id ) + return {campaign_id: found.get(campaign_id, []) for campaign_id in ids} @staticmethod def _report_id(report: Report | str) -> str: diff --git a/python/lib/sift_client/resources/sync_stubs/__init__.pyi b/python/lib/sift_client/resources/sync_stubs/__init__.pyi index 5dd39b7305..90755ecdbc 100644 --- a/python/lib/sift_client/resources/sync_stubs/__init__.pyi +++ b/python/lib/sift_client/resources/sync_stubs/__init__.pyi @@ -948,7 +948,9 @@ class CampaignsAPI: organization_id: Required if you belong to several organizations. Returns: - A mapping of campaign ID to its reports, with counts populated. + A mapping of campaign ID to its reports, with counts populated. The service + returns each campaign's reports in no fixed order; `Campaign.report_summaries` + orders them to match the campaign. """ ... diff --git a/python/lib/sift_client/sift_types/campaign.py b/python/lib/sift_client/sift_types/campaign.py index 13862dd960..609c4a46b5 100644 --- a/python/lib/sift_client/sift_types/campaign.py +++ b/python/lib/sift_client/sift_types/campaign.py @@ -27,6 +27,7 @@ if TYPE_CHECKING: from sift_client.client import SiftClient from sift_client.sift_types.report import Report + from sift_client.sift_types.run import Run class CampaignReport(BaseModel): @@ -146,6 +147,23 @@ def resolved_reports(self) -> list[Report]: return [] return self.client.reports.list_(report_ids=self.report_ids) + @property + def runs(self) -> list[Run]: + """Fetch the Runs behind this campaign's reports. Reports with no run are skipped.""" + run_ids = [r.run_id for r in self.resolved_reports if r.run_id] + if not run_ids: + return [] + return self.client.runs.list_(run_ids=run_ids) + + def report_summaries(self) -> list[CampaignReport]: + """Get this campaign's reports with their rule counts, ordered to match `report_ids`. + + The service returns them in no fixed order, so this would otherwise vary. + """ + summaries = self.client.campaigns.report_summaries([self]) + by_id = {s.report_id: s for s in summaries.get(self._id_or_error, [])} + return [by_id[report_id] for report_id in self.report_ids if report_id in by_id] + def update(self, update: CampaignUpdate | dict) -> Campaign: """Update the Campaign. @@ -176,8 +194,6 @@ class CampaignBase(ModelCreateUpdateBase): """Base class for Campaign create and update models.""" description: str | None = None - # Create takes NamedResources and update takes repeated TagRef, so the low-level - # client sets tags on the proto rather than the shared builder. tags: list[str] | list[Tag] | None = Field(default=None, exclude=True) metadata: dict[str, str | float | bool] | None = None @@ -209,7 +225,8 @@ class CampaignUpdate(CampaignBase, ModelUpdate[CampaignProto]): """Update model for Campaign. `reports` replaces the campaign's report list. Prefer `campaigns.add_reports` or - `campaigns.add_runs`, which read the current list first. + `campaigns.add_runs`, which read the current list first. `tags` is unordered, so it + may come back in a different order. """ name: str | None = None From 33544b57fd9d874bc09b668311465ffee06ab345 Mon Sep 17 00:00:00 2001 From: Brandon Shippy Date: Thu, 24 Sep 2026 01:15:37 -0400 Subject: [PATCH 3/8] python: campaigns pr comments updated --- .../_internal/low_level_wrappers/campaigns.py | 15 ++- .../_tests/resources/test_campaigns.py | 72 +++++------ .../_tests/sift_types/test_campaign.py | 44 +++---- python/lib/sift_client/resources/campaigns.py | 65 ++++------ .../resources/sync_stubs/__init__.pyi | 30 ++--- python/lib/sift_client/sift_types/__init__.py | 6 +- python/lib/sift_client/sift_types/campaign.py | 120 +++++++++--------- 7 files changed, 165 insertions(+), 187 deletions(-) diff --git a/python/lib/sift_client/_internal/low_level_wrappers/campaigns.py b/python/lib/sift_client/_internal/low_level_wrappers/campaigns.py index efa9a3647f..cb9699d639 100644 --- a/python/lib/sift_client/_internal/low_level_wrappers/campaigns.py +++ b/python/lib/sift_client/_internal/low_level_wrappers/campaigns.py @@ -3,6 +3,9 @@ import logging from typing import TYPE_CHECKING, Any, cast +from sift.campaigns.v1.campaigns_pb2 import ( + CampaignReport as CampaignReportProto, +) from sift.campaigns.v1.campaigns_pb2 import ( CreateCampaignFrom, CreateCampaignResponse, @@ -28,8 +31,9 @@ from sift_client.sift_types.campaign import ( Campaign, CampaignCreate, - CampaignReport, + CampaignReportSummary, CampaignUpdate, + report_ids, tag_names, ) from sift_client.transport import WithGrpcClient @@ -221,6 +225,11 @@ async def update_campaign(self, update: CampaignUpdate) -> Campaign: The updated Campaign. """ grpc_campaign, update_mask = update.to_proto_with_mask() + if "reports" in update.model_fields_set: + grpc_campaign.reports.extend( + CampaignReportProto(report_id=r) for r in report_ids(update.reports) + ) + update_mask.paths.append("reports") if "tags" in update.model_fields_set: grpc_campaign.tags.extend(TagRef(name=name) for name in tag_names(update.tags)) update_mask.paths.append("tags") @@ -232,7 +241,7 @@ async def update_campaign(self, update: CampaignUpdate) -> Campaign: async def get_report_summaries( self, *, campaign_ids: list[str], organization_id: str | None = None - ) -> dict[str, list[CampaignReport]]: + ) -> dict[str, list[CampaignReportSummary]]: """Get per-report rule counts for several campaigns at once. Args: @@ -253,6 +262,6 @@ async def get_report_summaries( response = cast("GetCampaignReportSummariesResponse", response) return { - campaign_id: [CampaignReport._from_proto(r) for r in reports.reports] + campaign_id: [CampaignReportSummary._from_proto(r) for r in reports.reports] for campaign_id, reports in response.summaries_by_campaign_id.items() } diff --git a/python/lib/sift_client/_tests/resources/test_campaigns.py b/python/lib/sift_client/_tests/resources/test_campaigns.py index 837b4b88bc..40b6262e26 100644 --- a/python/lib/sift_client/_tests/resources/test_campaigns.py +++ b/python/lib/sift_client/_tests/resources/test_campaigns.py @@ -1,6 +1,6 @@ """Pytest tests for the Campaigns API. -These tests cover get, list, find, create, get_or_create, update, add_reports, +These tests cover get, list, find, create, update, add_reports, add_runs, archive/unarchive, and report_summaries. """ @@ -181,33 +181,6 @@ def test_update_accepts_dict(self, campaigns_api_sync, new_campaign, test_timest campaigns_api_sync.update(new_campaign, {"name": new_campaign.name}) - def test_get_or_create(self, campaigns_api_sync, new_campaign): - """Test that get_or_create returns the existing campaign for a known key.""" - existing = campaigns_api_sync.get_or_create( - CampaignCreate(name="ignored", client_key=new_campaign.client_key) - ) - - assert existing.id_ == new_campaign.id_ - - def test_get_or_create_creates(self, campaigns_api_sync, test_timestamp_str): - """Test that get_or_create creates a campaign for an unknown key.""" - key = f"test_campaign_goc_{test_timestamp_str}" - created = campaigns_api_sync.get_or_create( - CampaignCreate(name=f"test_campaign_goc_{test_timestamp_str}", client_key=key) - ) - - assert created.client_key == key - assert campaigns_api_sync.get_or_create({"name": "ignored", "client_key": key}).id_ == ( - created.id_ - ) - - campaigns_api_sync.archive(created) - - def test_get_or_create_requires_client_key(self, campaigns_api_sync): - """Test that get_or_create rejects a create with no client key.""" - with pytest.raises(ValueError, match="requires a client_key"): - campaigns_api_sync.get_or_create(CampaignCreate(name="no key")) - def test_add_reports(self, campaigns_api_sync, campaign_run, test_timestamp_str): """Test adding reports to a campaign without dropping the existing ones.""" campaign = campaigns_api_sync.create( @@ -216,11 +189,11 @@ def test_add_reports(self, campaigns_api_sync, campaign_run, test_timestamp_str) report_id = campaign_run.default_report_id added = campaigns_api_sync.add_reports(campaign, [report_id]) - assert added.report_ids == [report_id] + assert [r.report_id for r in added.report_summaries] == [report_id] # Adding the same report again is a no-op, not a duplicate. again = campaigns_api_sync.add_reports(added, [report_id]) - assert again.report_ids == [report_id] + assert [r.report_id for r in again.report_summaries] == [report_id] campaigns_api_sync.archive(campaign) @@ -231,23 +204,38 @@ def test_add_runs(self, campaigns_api_sync, campaign_run, test_timestamp_str): ) added = campaigns_api_sync.add_runs(campaign, [campaign_run]) - assert added.report_ids == [campaign_run.default_report_id] + assert [r.report_id for r in added.report_summaries] == [campaign_run.default_report_id] campaigns_api_sync.archive(campaign) def test_add_runs_rejects_run_without_report( self, campaigns_api_sync, sift_client, new_campaign, test_timestamp_str ): - """Test that add_runs fails clearly when a run has no default report.""" + """Test that add_runs fails clearly when a run has no report at all.""" from sift_client.sift_types.run import RunCreate run = sift_client.runs.create( RunCreate(name=f"test_campaign_no_report_{test_timestamp_str}") ) - with pytest.raises(ValueError, match="no default report"): + with pytest.raises(ValueError, match="no report"): campaigns_api_sync.add_runs(new_campaign, [run]) + def test_add_runs_falls_back_to_a_report_over_the_run( + self, campaigns_api_sync, campaign_run, test_timestamp_str + ): + """Test that a run with no default report joins through a report search.""" + # Hide the default report so add_runs must search for one over the run. + run = campaign_run.model_copy(update={"default_report_id": None}) + campaign = campaigns_api_sync.create( + CampaignCreate(name=f"test_campaign_fallback_{test_timestamp_str}") + ) + + added = campaigns_api_sync.add_runs(campaign, [run]) + + assert [r.report_id for r in added.report_summaries] == [campaign_run.default_report_id] + campaigns_api_sync.archive(campaign) + def test_create_from_runs(self, campaigns_api_sync, campaign_run, test_timestamp_str): """Test seeding a new campaign from a run.""" campaign = campaigns_api_sync.create( @@ -255,7 +243,7 @@ def test_create_from_runs(self, campaigns_api_sync, campaign_run, test_timestamp runs=[campaign_run], ) - assert campaign.report_ids == [campaign_run.default_report_id] + assert [r.report_id for r in campaign.report_summaries] == [campaign_run.default_report_id] campaigns_api_sync.archive(campaign) @@ -278,7 +266,9 @@ def test_report_summaries(self, campaigns_api_sync, campaign_run, test_timestamp summaries = campaigns_api_sync.report_summaries([campaign]) assert campaign._id_or_error in summaries - assert [r.report_id for r in summaries[campaign._id_or_error]] == campaign.report_ids + assert [r.report_id for r in summaries[campaign._id_or_error]] == [ + r.report_id for r in campaign.report_summaries + ] campaigns_api_sync.archive(campaign) @@ -337,11 +327,11 @@ def test_add_reports_reads_the_server_list( # Another caller adds a report that `stale` knows nothing about. campaigns_api_sync.add_reports(campaign._id_or_error, [campaign_run.default_report_id]) - assert stale.report_ids == [] + assert [r.report_id for r in stale.report_summaries] == [] merged = campaigns_api_sync.add_reports(stale, [campaign_run.default_report_id]) - assert merged.report_ids == [campaign_run.default_report_id] + assert [r.report_id for r in merged.report_summaries] == [campaign_run.default_report_id] campaigns_api_sync.archive(campaign) @@ -363,8 +353,12 @@ def test_campaign_report_summaries(self, campaigns_api_sync, campaign_run, test_ runs=[campaign_run], ) - # The service returns these in no fixed order; the instance method sorts them. - assert [s.report_id for s in campaign.report_summaries()] == campaign.report_ids + # The service returns these in no fixed order; _from_proto sorts them. + ids = [r.report_id for r in campaign.report_summaries] + assert ids == sorted(ids) + assert ids == [ + r.report_id for r in campaigns_api_sync.get(campaign._id_or_error).report_summaries + ] campaigns_api_sync.archive(campaign) diff --git a/python/lib/sift_client/_tests/sift_types/test_campaign.py b/python/lib/sift_client/_tests/sift_types/test_campaign.py index f0fd0709c4..98934b0eb7 100644 --- a/python/lib/sift_client/_tests/sift_types/test_campaign.py +++ b/python/lib/sift_client/_tests/sift_types/test_campaign.py @@ -8,32 +8,32 @@ from sift_client.sift_types import Campaign from sift_client.sift_types.campaign import ( CampaignCreate, - CampaignReport, + CampaignReportSummary, CampaignUpdate, ) NOW = datetime(2026, 1, 1, tzinfo=timezone.utc) -class TestCampaignReport: - """Unit tests for CampaignReport.""" +class TestCampaignReportSummary: + """Unit tests for CampaignReportSummary.""" def test_counts_default_to_zero(self): - report = CampaignReport(report_id="r-1") + report = CampaignReportSummary(report_id="r-1") - assert report.num_annotations == 0 + assert report.rule_statistics.annotations == 0 assert report.report_name == "" def test_from_proto(self): from sift.campaigns.v1.campaigns_pb2 import CampaignReport as Proto - report = CampaignReport._from_proto( + report = CampaignReportSummary._from_proto( Proto(report_id="r-1", report_name="nightly", num_failed_rules=2) ) assert report.report_id == "r-1" assert report.report_name == "nightly" - assert report.num_failed_rules == 2 + assert report.rule_statistics.failed == 2 class TestCampaignCreate: @@ -70,22 +70,15 @@ def test_update_mask_only_includes_set_fields(self): assert proto.name == "renamed" assert set(mask.paths) == {"name", "description"} - def test_reports_converter_sends_only_ids(self): - update = CampaignUpdate( - reports=[ - CampaignReport(report_id="r-1", report_name="ignored", num_failed_rules=9), - CampaignReport(report_id="r-2"), - ] - ) + def test_reports_takes_ids(self): + """`reports` takes report IDs; the read-only summary type never appears here.""" + update = CampaignUpdate(reports=["r-1", "r-2"]) update.resource_id = "c-1" proto, mask = update.to_proto_with_mask() - assert [r.report_id for r in proto.reports] == ["r-1", "r-2"] - # Name and counts are output only, so they must not be sent. - assert proto.reports[0].report_name == "" - assert proto.reports[0].num_failed_rules == 0 - assert mask.paths == ["reports"] + assert mask.paths == [] + assert update.reports == ["r-1", "r-2"] def test_archive_update(self): update = CampaignUpdate(is_archived=True) @@ -109,7 +102,10 @@ def mock_campaign(mock_client): id_="c-1", name="Q1 regression", organization_id="org1", - reports=[CampaignReport(report_id="r-1"), CampaignReport(report_id="r-2")], + report_summaries=[ + CampaignReportSummary(report_id="r-1"), + CampaignReportSummary(report_id="r-2"), + ], tags=[], metadata={}, created_date=NOW, @@ -130,13 +126,13 @@ def mock_campaign(mock_client): class TestCampaign: """Unit tests for Campaign model - tests properties and methods.""" - def test_report_ids(self, mock_campaign): - assert mock_campaign.report_ids == ["r-1", "r-2"] + def test_report_summaries(self, mock_campaign): + assert [r.report_id for r in mock_campaign.report_summaries] == ["r-1", "r-2"] - def test_resolved_reports_calls_client(self, mock_campaign, mock_client): + def test_reports_calls_client(self, mock_campaign, mock_client): mock_client.reports.list_.return_value = [] - _ = mock_campaign.resolved_reports + _ = mock_campaign.reports mock_client.reports.list_.assert_called_once_with(report_ids=["r-1", "r-2"]) diff --git a/python/lib/sift_client/resources/campaigns.py b/python/lib/sift_client/resources/campaigns.py index e1776c476f..5fa6d2d5c7 100644 --- a/python/lib/sift_client/resources/campaigns.py +++ b/python/lib/sift_client/resources/campaigns.py @@ -7,7 +7,7 @@ from sift_client.sift_types.campaign import ( Campaign, CampaignCreate, - CampaignReport, + CampaignReportSummary, CampaignUpdate, ) from sift_client.sift_types.run import Run @@ -25,7 +25,7 @@ class CampaignsAPIAsync(ResourceBase): """High-level API for interacting with campaigns. A campaign is a named list of reports. Runs join a campaign through the reports they - generate, so a run must be created with `create_default_report=True` to be added. + generate. """ def __init__(self, sift_client: SiftClient): @@ -169,7 +169,7 @@ async def create( *, reports: list[Report] | list[str] | None = None, runs: list[Run] | list[str] | None = None, - from_campaign: str | Campaign | None = None, + campaign: str | Campaign | None = None, ) -> Campaign: """Create a new campaign, optionally seeded with reports. @@ -180,7 +180,7 @@ async def create( create: The campaign definition. reports: Seed with these Reports or report IDs. runs: Seed with the reports these Runs generated. - from_campaign: Duplicate this Campaign or campaign ID. + campaign: Duplicate this Campaign or campaign ID. Returns: The created Campaign. @@ -194,33 +194,11 @@ async def create( [r._id_or_error if isinstance(r, Run) else r for r in runs] if runs else None ), from_campaign_id=( - from_campaign._id_or_error if isinstance(from_campaign, Campaign) else from_campaign + campaign._id_or_error if isinstance(campaign, Campaign) else campaign ), ) return self._apply_client_to_instance(created) - async def get_or_create(self, create: CampaignCreate | dict) -> Campaign: - """Get the campaign with this client key, or create it. - - Args: - create: The campaign definition. Its `client_key` is required. - - Returns: - The existing or newly created Campaign. - - Raises: - ValueError: If `client_key` is not set. - """ - if isinstance(create, dict): - create = CampaignCreate.model_validate(create) - if not create.client_key: - raise ValueError("get_or_create requires a client_key") - - existing = await self.find(client_keys=[create.client_key], include_archived=True) - if existing is not None: - return existing - return await self.create(create) - async def update(self, campaign: str | Campaign, update: CampaignUpdate | dict) -> Campaign: """Update a Campaign. @@ -255,21 +233,19 @@ async def add_reports( """ campaign_id = campaign._id_or_error if isinstance(campaign, Campaign) else campaign current = await self.get(campaign_id, skip_report_summaries=True) - existing_ids = current.report_ids + existing_ids = [r.report_id for r in current.report_summaries] merged = existing_ids + [ report_id for report_id in (self._report_id(r) for r in reports) if report_id not in existing_ids ] - return await self.update( - current, CampaignUpdate(reports=[CampaignReport(report_id=r) for r in merged]) - ) + return await self.update(current, CampaignUpdate(reports=merged)) async def add_runs(self, campaign: str | Campaign, runs: list[Run] | list[str]) -> Campaign: """Add runs to a campaign through the reports they generated. - A campaign holds reports, not runs, so each run must have a default report. Pass - `create_default_report=True` to `RunCreate` to get one. + A campaign holds reports, not runs. Each run contributes its default report, or + its most recent report if it has none. Args: campaign: The Campaign or campaign ID to add to. @@ -279,18 +255,29 @@ async def add_runs(self, campaign: str | Campaign, runs: list[Run] | list[str]) The updated Campaign. Raises: - ValueError: If any run has no default report. + ValueError: If any run has no report. """ resolved = [ r if isinstance(r, Run) else await self.client.async_.runs.get(run_id=r) for r in runs ] - missing = [r.name for r in resolved if not r.default_report_id] + report_ids = [] + missing = [] + for run in resolved: + report_id = run.default_report_id + if not report_id: + found = await self.client.async_.reports.list_( + run=run, order_by="created_date desc", limit=1 + ) + report_id = found[0]._id_or_error if found else None + if report_id: + report_ids.append(report_id) + else: + missing.append(run.name) if missing: raise ValueError( - f"These runs have no default report, so they cannot join a campaign: {missing}. " - "Create runs with create_default_report=True." + f"These runs have no report, so they cannot join a campaign: {missing}. " + "Create runs with create_default_report=True, or build a report over the run." ) - report_ids = [r.default_report_id for r in resolved if r.default_report_id] return await self.add_reports(campaign, report_ids) async def archive(self, campaign: str | Campaign) -> Campaign: @@ -317,7 +304,7 @@ async def unarchive(self, campaign: str | Campaign) -> Campaign: async def report_summaries( self, campaigns: list[str | Campaign], *, organization_id: str | None = None - ) -> dict[str, list[CampaignReport]]: + ) -> dict[str, list[CampaignReportSummary]]: """Get per-report rule counts for several campaigns at once. Args: diff --git a/python/lib/sift_client/resources/sync_stubs/__init__.pyi b/python/lib/sift_client/resources/sync_stubs/__init__.pyi index 90755ecdbc..3383c0ab7f 100644 --- a/python/lib/sift_client/resources/sync_stubs/__init__.pyi +++ b/python/lib/sift_client/resources/sync_stubs/__init__.pyi @@ -37,7 +37,7 @@ if TYPE_CHECKING: from sift_client.sift_types.campaign import ( Campaign, CampaignCreate, - CampaignReport, + CampaignReportSummary, CampaignUpdate, ) from sift_client.sift_types.channel import Channel, ChannelUpdate @@ -766,7 +766,7 @@ class CampaignsAPI: High-level API for interacting with campaigns. A campaign is a named list of reports. Runs join a campaign through the reports they - generate, so a run must be created with `create_default_report=True` to be added. + generate. """ def __init__(self, sift_client: SiftClient): @@ -793,8 +793,8 @@ class CampaignsAPI: def add_runs(self, campaign: str | Campaign, runs: list[Run] | list[str]) -> Campaign: """Add runs to a campaign through the reports they generated. - A campaign holds reports, not runs, so each run must have a default report. Pass - `create_default_report=True` to `RunCreate` to get one. + A campaign holds reports, not runs. Each run contributes its default report, or + its most recent report if it has none. Args: campaign: The Campaign or campaign ID to add to. @@ -804,7 +804,7 @@ class CampaignsAPI: The updated Campaign. Raises: - ValueError: If any run has no default report. + ValueError: If any run has no report. """ ... @@ -825,7 +825,7 @@ class CampaignsAPI: *, reports: list[Report] | list[str] | None = None, runs: list[Run] | list[str] | None = None, - from_campaign: str | Campaign | None = None, + campaign: str | Campaign | None = None, ) -> Campaign: """Create a new campaign, optionally seeded with reports. @@ -836,7 +836,7 @@ class CampaignsAPI: create: The campaign definition. reports: Seed with these Reports or report IDs. runs: Seed with the reports these Runs generated. - from_campaign: Duplicate this Campaign or campaign ID. + campaign: Duplicate this Campaign or campaign ID. Returns: The created Campaign. @@ -877,20 +877,6 @@ class CampaignsAPI: """ ... - def get_or_create(self, create: CampaignCreate | dict) -> Campaign: - """Get the campaign with this client key, or create it. - - Args: - create: The campaign definition. Its `client_key` is required. - - Returns: - The existing or newly created Campaign. - - Raises: - ValueError: If `client_key` is not set. - """ - ... - def list_( self, *, @@ -940,7 +926,7 @@ class CampaignsAPI: def report_summaries( self, campaigns: list[str | Campaign], *, organization_id: str | None = None - ) -> dict[str, list[CampaignReport]]: + ) -> dict[str, list[CampaignReportSummary]]: """Get per-report rule counts for several campaigns at once. Args: diff --git a/python/lib/sift_client/sift_types/__init__.py b/python/lib/sift_client/sift_types/__init__.py index 4e4084d118..c22fa1363c 100644 --- a/python/lib/sift_client/sift_types/__init__.py +++ b/python/lib/sift_client/sift_types/__init__.py @@ -153,8 +153,9 @@ from sift_client.sift_types.campaign import ( Campaign, CampaignCreate, - CampaignReport, + CampaignReportSummary, CampaignUpdate, + RuleStatistics, ) from sift_client.sift_types.channel import ( Channel, @@ -270,7 +271,7 @@ "CalculatedChannelUpdate", "Campaign", "CampaignCreate", - "CampaignReport", + "CampaignReportSummary", "CampaignUpdate", "Channel", "ChannelBitFieldElement", @@ -325,6 +326,7 @@ "RuleCreate", "RuleEvaluationDetails", "RuleEvaluationStatusDetails", + "RuleStatistics", "RuleUpdate", "RuleVersion", "Run", diff --git a/python/lib/sift_client/sift_types/campaign.py b/python/lib/sift_client/sift_types/campaign.py index 609c4a46b5..7b4295d8a3 100644 --- a/python/lib/sift_client/sift_types/campaign.py +++ b/python/lib/sift_client/sift_types/campaign.py @@ -21,64 +21,63 @@ ModelCreateUpdateBase, ModelUpdate, ) +from sift_client.sift_types.report import Report # noqa: TC001 from sift_client.sift_types.tag import Tag from sift_client.util.metadata import metadata_dict_to_proto, metadata_proto_to_dict if TYPE_CHECKING: from sift_client.client import SiftClient - from sift_client.sift_types.report import Report from sift_client.sift_types.run import Run -class CampaignReport(BaseModel): +class RuleStatistics(BaseModel): + """How a report's rules came out. + + Attributes: + annotations: Annotations across the report's rules. + passed: Rules that never triggered. + accepted: Rules whose annotations are all accepted. + failed: Rules with any failed annotation. + open: Rules with open annotations and none failed. + """ + + annotations: int = 0 + passed: int = 0 + accepted: int = 0 + failed: int = 0 + open: int = 0 + + +class CampaignReportSummary(BaseModel): """A report in a campaign, with a rollup of its rule outcomes. - The counts are only populated when the campaign was fetched with summaries. + Read only. The counts are populated only when the campaign was fetched with summaries. Attributes: report_id: The report this entry refers to. report_name: The report's name. - num_annotations: Annotations across the report's rules. - num_passed_rules: Rules that never triggered. - num_accepted_rules: Rules whose annotations are all accepted. - num_failed_rules: Rules with any failed annotation. - num_open_rules: Rules with open annotations and none failed. + rule_statistics: The rollup of the report's rule outcomes. """ report_id: str report_name: str = "" - num_annotations: int = 0 - num_passed_rules: int = 0 - num_accepted_rules: int = 0 - num_failed_rules: int = 0 - num_open_rules: int = 0 + rule_statistics: RuleStatistics = RuleStatistics() @classmethod - def _from_proto(cls, proto: CampaignReportProto) -> CampaignReport: + def _from_proto(cls, proto: CampaignReportProto) -> CampaignReportSummary: return cls( report_id=proto.report_id, report_name=proto.report_name, - num_annotations=proto.num_annotations, - num_passed_rules=proto.num_passed_rules, - num_accepted_rules=proto.num_accepted_rules, - num_failed_rules=proto.num_failed_rules, - num_open_rules=proto.num_open_rules, + rule_statistics=RuleStatistics( + annotations=proto.num_annotations, + passed=proto.num_passed_rules, + accepted=proto.num_accepted_rules, + failed=proto.num_failed_rules, + open=proto.num_open_rules, + ), ) -def _campaign_report_to_proto(**kwargs) -> CampaignReportProto: - """Convert a campaign report dict (from model_dump) into its proto form. - - Only `report_id` is sent; the name and counts are output only. - """ - return CampaignReportProto(report_id=kwargs["report_id"]) - - -def tag_names(tags: list[str] | list[Tag] | None) -> list[str]: - """Reduce Tags or tag names to plain names.""" - return [tag.name if isinstance(tag, Tag) else tag for tag in tags or []] - - class Campaign(BaseType[CampaignProto, "Campaign"]): """A campaign, which is a named list of reports. @@ -88,7 +87,7 @@ class Campaign(BaseType[CampaignProto, "Campaign"]): # Required fields name: str organization_id: str - reports: list[CampaignReport] + report_summaries: list[CampaignReportSummary] tags: list[str] metadata: dict[str, str | float | bool] created_date: datetime @@ -111,7 +110,10 @@ def _from_proto(cls, proto: CampaignProto, sift_client: SiftClient | None = None id_=proto.campaign_id, name=proto.name, organization_id=proto.organization_id, - reports=[CampaignReport._from_proto(r) for r in proto.reports], + report_summaries=sorted( + (CampaignReportSummary._from_proto(r) for r in proto.reports), + key=lambda r: r.report_id, + ), tags=[t.name or t.tag_id for t in proto.tags], metadata=metadata_proto_to_dict(proto.metadata), # type: ignore created_date=proto.created_date.ToDatetime(tzinfo=timezone.utc), @@ -136,33 +138,31 @@ def _from_proto(cls, proto: CampaignProto, sift_client: SiftClient | None = None ) @property - def report_ids(self) -> list[str]: - """The IDs of the reports in this campaign.""" - return [r.report_id for r in self.reports] - - @property - def resolved_reports(self) -> list[Report]: + def reports(self) -> list[Report]: """Fetch the full Reports in this campaign.""" - if not self.reports: + if not self.report_summaries: return [] - return self.client.reports.list_(report_ids=self.report_ids) + return self.client.reports.list_(report_ids=[r.report_id for r in self.report_summaries]) @property def runs(self) -> list[Run]: """Fetch the Runs behind this campaign's reports. Reports with no run are skipped.""" - run_ids = [r.run_id for r in self.resolved_reports if r.run_id] + run_ids = [r.run_id for r in self.reports if r.run_id] if not run_ids: return [] return self.client.runs.list_(run_ids=run_ids) - def report_summaries(self) -> list[CampaignReport]: - """Get this campaign's reports with their rule counts, ordered to match `report_ids`. + def add_reports(self, reports: list[Report] | list[str]) -> Campaign: + """Add reports to the campaign, keeping the ones already there.""" + updated = self.client.campaigns.add_reports(campaign=self, reports=reports) + self._update(updated) + return self - The service returns them in no fixed order, so this would otherwise vary. - """ - summaries = self.client.campaigns.report_summaries([self]) - by_id = {s.report_id: s for s in summaries.get(self._id_or_error, [])} - return [by_id[report_id] for report_id in self.report_ids if report_id in by_id] + def add_runs(self, runs: list[Run] | list[str]) -> Campaign: + """Add runs to the campaign through the reports they generated.""" + updated = self.client.campaigns.add_runs(campaign=self, runs=runs) + self._update(updated) + return self def update(self, update: CampaignUpdate | dict) -> Campaign: """Update the Campaign. @@ -190,6 +190,16 @@ def unarchive(self) -> Campaign: return self +def tag_names(tags: list[str] | list[Tag] | None) -> list[str]: + """Reduce Tags or tag names to plain names.""" + return [tag.name if isinstance(tag, Tag) else tag for tag in tags or []] + + +def report_ids(reports: list[Report] | list[str] | None) -> list[str]: + """Reduce Reports or report IDs to plain IDs.""" + return [r if isinstance(r, str) else r._id_or_error for r in reports or []] + + class CampaignBase(ModelCreateUpdateBase): """Base class for Campaign create and update models.""" @@ -209,8 +219,7 @@ class CampaignBase(ModelCreateUpdateBase): class CampaignCreate(CampaignBase, ModelCreate[CreateCampaignRequestProto]): """Create model for Campaign. - Pass `client_key` to make the campaign addressable by your own identifier, which is - what `get_or_create` matches on. + Pass `client_key` to make the campaign addressable by your own identifier. """ name: str @@ -230,16 +239,11 @@ class CampaignUpdate(CampaignBase, ModelUpdate[CampaignProto]): """ name: str | None = None - reports: list[CampaignReport] | None = None + reports: list[Report] | list[str] | None = Field(default=None, exclude=True) is_archived: bool | None = None _to_proto_helpers: ClassVar[dict[str, MappingHelper]] = { **CampaignBase._to_proto_helpers, - "reports": MappingHelper( - proto_attr_path="reports", - update_field="reports", - converter=_campaign_report_to_proto, # type: ignore[arg-type] - ), } def _get_proto_class(self) -> type[CampaignProto]: From 96d5d7886d93f8c7e48c272d4283f1f33441c661 Mon Sep 17 00:00:00 2001 From: Brandon Shippy Date: Thu, 24 Sep 2026 01:54:49 -0400 Subject: [PATCH 4/8] python: resolve runs to reports in create --- python/lib/sift_client/resources/campaigns.py | 65 +++++++++++-------- .../resources/sync_stubs/__init__.pyi | 6 +- 2 files changed, 41 insertions(+), 30 deletions(-) diff --git a/python/lib/sift_client/resources/campaigns.py b/python/lib/sift_client/resources/campaigns.py index 5fa6d2d5c7..213dfed463 100644 --- a/python/lib/sift_client/resources/campaigns.py +++ b/python/lib/sift_client/resources/campaigns.py @@ -173,8 +173,7 @@ async def create( ) -> Campaign: """Create a new campaign, optionally seeded with reports. - At most one seed may be given. `runs` lets the server collect the reports those - runs generated, so the runs need a default report. + At most one seed may be given. Args: create: The campaign definition. @@ -184,15 +183,21 @@ async def create( Returns: The created Campaign. + + Raises: + ValueError: If any run has no report. """ if isinstance(create, dict): create = CampaignCreate.model_validate(create) + if len([seed for seed in (reports, runs, campaign) if seed]) > 1: + raise ValueError("At most one of reports, runs, or campaign may be provided") + seed_report_ids = [self._report_id(r) for r in reports] if reports else None + if runs: + # The service silently skips runs with no default report, so resolve here. + seed_report_ids = await self._run_report_ids(runs) created = await self._low_level_client.create_campaign( create=create, - from_report_ids=[self._report_id(r) for r in reports] if reports else None, - from_run_ids=( - [r._id_or_error if isinstance(r, Run) else r for r in runs] if runs else None - ), + from_report_ids=seed_report_ids, from_campaign_id=( campaign._id_or_error if isinstance(campaign, Campaign) else campaign ), @@ -257,28 +262,7 @@ async def add_runs(self, campaign: str | Campaign, runs: list[Run] | list[str]) Raises: ValueError: If any run has no report. """ - resolved = [ - r if isinstance(r, Run) else await self.client.async_.runs.get(run_id=r) for r in runs - ] - report_ids = [] - missing = [] - for run in resolved: - report_id = run.default_report_id - if not report_id: - found = await self.client.async_.reports.list_( - run=run, order_by="created_date desc", limit=1 - ) - report_id = found[0]._id_or_error if found else None - if report_id: - report_ids.append(report_id) - else: - missing.append(run.name) - if missing: - raise ValueError( - f"These runs have no report, so they cannot join a campaign: {missing}. " - "Create runs with create_default_report=True, or build a report over the run." - ) - return await self.add_reports(campaign, report_ids) + return await self.add_reports(campaign, await self._run_report_ids(runs)) async def archive(self, campaign: str | Campaign) -> Campaign: """Archive a campaign. @@ -324,6 +308,31 @@ async def report_summaries( ) return {campaign_id: found.get(campaign_id, []) for campaign_id in ids} + async def _run_report_ids(self, runs: list[Run] | list[str]) -> list[str]: + """Resolve runs to the report each one contributes to a campaign.""" + resolved = [ + r if isinstance(r, Run) else await self.client.async_.runs.get(run_id=r) for r in runs + ] + report_ids = [] + missing = [] + for run in resolved: + report_id = run.default_report_id + if not report_id: + found = await self.client.async_.reports.list_( + run=run, order_by="created_date desc", limit=1 + ) + report_id = found[0]._id_or_error if found else None + if report_id: + report_ids.append(report_id) + else: + missing.append(run.name) + if missing: + raise ValueError( + f"These runs have no report, so they cannot join a campaign: {missing}. " + "Create runs with create_default_report=True, or build a report over the run." + ) + return report_ids + @staticmethod def _report_id(report: Report | str) -> str: return report if isinstance(report, str) else report._id_or_error diff --git a/python/lib/sift_client/resources/sync_stubs/__init__.pyi b/python/lib/sift_client/resources/sync_stubs/__init__.pyi index 3383c0ab7f..4e8321429f 100644 --- a/python/lib/sift_client/resources/sync_stubs/__init__.pyi +++ b/python/lib/sift_client/resources/sync_stubs/__init__.pyi @@ -829,8 +829,7 @@ class CampaignsAPI: ) -> Campaign: """Create a new campaign, optionally seeded with reports. - At most one seed may be given. `runs` lets the server collect the reports those - runs generated, so the runs need a default report. + At most one seed may be given. Args: create: The campaign definition. @@ -840,6 +839,9 @@ class CampaignsAPI: Returns: The created Campaign. + + Raises: + ValueError: If any run has no report. """ ... From f96db86408c744345b9ebebcf33f35ac915d69ac Mon Sep 17 00:00:00 2001 From: Brandon Shippy Date: Thu, 24 Sep 2026 02:17:40 -0400 Subject: [PATCH 5/8] python: renamed to add_reports_to_campaign --- .../_tests/resources/test_campaigns.py | 20 ++++++++++--------- python/lib/sift_client/resources/campaigns.py | 12 ++++++----- .../resources/sync_stubs/__init__.pyi | 12 +++++++---- python/lib/sift_client/sift_types/campaign.py | 8 ++++---- 4 files changed, 30 insertions(+), 22 deletions(-) diff --git a/python/lib/sift_client/_tests/resources/test_campaigns.py b/python/lib/sift_client/_tests/resources/test_campaigns.py index 40b6262e26..d54f1b1e5d 100644 --- a/python/lib/sift_client/_tests/resources/test_campaigns.py +++ b/python/lib/sift_client/_tests/resources/test_campaigns.py @@ -1,7 +1,7 @@ """Pytest tests for the Campaigns API. -These tests cover get, list, find, create, update, add_reports, -add_runs, archive/unarchive, and report_summaries. +These tests cover get, list, find, create, update, add_reports_to_campaign, +add_runs_to_campaign, archive/unarchive, and report_summaries. """ from datetime import datetime, timezone @@ -188,11 +188,11 @@ def test_add_reports(self, campaigns_api_sync, campaign_run, test_timestamp_str) ) report_id = campaign_run.default_report_id - added = campaigns_api_sync.add_reports(campaign, [report_id]) + added = campaigns_api_sync.add_reports_to_campaign(campaign, [report_id]) assert [r.report_id for r in added.report_summaries] == [report_id] # Adding the same report again is a no-op, not a duplicate. - again = campaigns_api_sync.add_reports(added, [report_id]) + again = campaigns_api_sync.add_reports_to_campaign(added, [report_id]) assert [r.report_id for r in again.report_summaries] == [report_id] campaigns_api_sync.archive(campaign) @@ -203,7 +203,7 @@ def test_add_runs(self, campaigns_api_sync, campaign_run, test_timestamp_str): CampaignCreate(name=f"test_campaign_runs_{test_timestamp_str}") ) - added = campaigns_api_sync.add_runs(campaign, [campaign_run]) + added = campaigns_api_sync.add_runs_to_campaign(campaign, [campaign_run]) assert [r.report_id for r in added.report_summaries] == [campaign_run.default_report_id] campaigns_api_sync.archive(campaign) @@ -219,7 +219,7 @@ def test_add_runs_rejects_run_without_report( ) with pytest.raises(ValueError, match="no report"): - campaigns_api_sync.add_runs(new_campaign, [run]) + campaigns_api_sync.add_runs_to_campaign(new_campaign, [run]) def test_add_runs_falls_back_to_a_report_over_the_run( self, campaigns_api_sync, campaign_run, test_timestamp_str @@ -231,7 +231,7 @@ def test_add_runs_falls_back_to_a_report_over_the_run( CampaignCreate(name=f"test_campaign_fallback_{test_timestamp_str}") ) - added = campaigns_api_sync.add_runs(campaign, [run]) + added = campaigns_api_sync.add_runs_to_campaign(campaign, [run]) assert [r.report_id for r in added.report_summaries] == [campaign_run.default_report_id] campaigns_api_sync.archive(campaign) @@ -326,10 +326,12 @@ def test_add_reports_reads_the_server_list( stale = campaign # Another caller adds a report that `stale` knows nothing about. - campaigns_api_sync.add_reports(campaign._id_or_error, [campaign_run.default_report_id]) + campaigns_api_sync.add_reports_to_campaign( + campaign._id_or_error, [campaign_run.default_report_id] + ) assert [r.report_id for r in stale.report_summaries] == [] - merged = campaigns_api_sync.add_reports(stale, [campaign_run.default_report_id]) + merged = campaigns_api_sync.add_reports_to_campaign(stale, [campaign_run.default_report_id]) assert [r.report_id for r in merged.report_summaries] == [campaign_run.default_report_id] diff --git a/python/lib/sift_client/resources/campaigns.py b/python/lib/sift_client/resources/campaigns.py index 213dfed463..427bcaae82 100644 --- a/python/lib/sift_client/resources/campaigns.py +++ b/python/lib/sift_client/resources/campaigns.py @@ -207,8 +207,8 @@ async def create( async def update(self, campaign: str | Campaign, update: CampaignUpdate | dict) -> Campaign: """Update a Campaign. - `reports`, `tags`, and `metadata` are replaced, not merged. Prefer `add_reports` - or `add_runs` to grow the report list. + `reports`, `tags`, and `metadata` are replaced, not merged. Prefer + `add_reports_to_campaign` or `add_runs_to_campaign` to grow the report list. Args: campaign: The Campaign or campaign ID to update. @@ -224,7 +224,7 @@ async def update(self, campaign: str | Campaign, update: CampaignUpdate | dict) updated = await self._low_level_client.update_campaign(update) return self._apply_client_to_instance(updated) - async def add_reports( + async def add_reports_to_campaign( self, campaign: str | Campaign, reports: list[Report] | list[str] ) -> Campaign: """Add reports to a campaign, keeping the ones already there. @@ -246,7 +246,9 @@ async def add_reports( ] return await self.update(current, CampaignUpdate(reports=merged)) - async def add_runs(self, campaign: str | Campaign, runs: list[Run] | list[str]) -> Campaign: + async def add_runs_to_campaign( + self, campaign: str | Campaign, runs: list[Run] | list[str] + ) -> Campaign: """Add runs to a campaign through the reports they generated. A campaign holds reports, not runs. Each run contributes its default report, or @@ -262,7 +264,7 @@ async def add_runs(self, campaign: str | Campaign, runs: list[Run] | list[str]) Raises: ValueError: If any run has no report. """ - return await self.add_reports(campaign, await self._run_report_ids(runs)) + return await self.add_reports_to_campaign(campaign, await self._run_report_ids(runs)) async def archive(self, campaign: str | Campaign) -> Campaign: """Archive a campaign. diff --git a/python/lib/sift_client/resources/sync_stubs/__init__.pyi b/python/lib/sift_client/resources/sync_stubs/__init__.pyi index 4e8321429f..39c70f1a00 100644 --- a/python/lib/sift_client/resources/sync_stubs/__init__.pyi +++ b/python/lib/sift_client/resources/sync_stubs/__init__.pyi @@ -778,7 +778,9 @@ class CampaignsAPI: ... def _run(self, coro): ... - def add_reports(self, campaign: str | Campaign, reports: list[Report] | list[str]) -> Campaign: + def add_reports_to_campaign( + self, campaign: str | Campaign, reports: list[Report] | list[str] + ) -> Campaign: """Add reports to a campaign, keeping the ones already there. Args: @@ -790,7 +792,9 @@ class CampaignsAPI: """ ... - def add_runs(self, campaign: str | Campaign, runs: list[Run] | list[str]) -> Campaign: + def add_runs_to_campaign( + self, campaign: str | Campaign, runs: list[Run] | list[str] + ) -> Campaign: """Add runs to a campaign through the reports they generated. A campaign holds reports, not runs. Each run contributes its default report, or @@ -956,8 +960,8 @@ class CampaignsAPI: def update(self, campaign: str | Campaign, update: CampaignUpdate | dict) -> Campaign: """Update a Campaign. - `reports`, `tags`, and `metadata` are replaced, not merged. Prefer `add_reports` - or `add_runs` to grow the report list. + `reports`, `tags`, and `metadata` are replaced, not merged. Prefer + `add_reports_to_campaign` or `add_runs_to_campaign` to grow the report list. Args: campaign: The Campaign or campaign ID to update. diff --git a/python/lib/sift_client/sift_types/campaign.py b/python/lib/sift_client/sift_types/campaign.py index 7b4295d8a3..76823dd2d8 100644 --- a/python/lib/sift_client/sift_types/campaign.py +++ b/python/lib/sift_client/sift_types/campaign.py @@ -154,13 +154,13 @@ def runs(self) -> list[Run]: def add_reports(self, reports: list[Report] | list[str]) -> Campaign: """Add reports to the campaign, keeping the ones already there.""" - updated = self.client.campaigns.add_reports(campaign=self, reports=reports) + updated = self.client.campaigns.add_reports_to_campaign(campaign=self, reports=reports) self._update(updated) return self def add_runs(self, runs: list[Run] | list[str]) -> Campaign: """Add runs to the campaign through the reports they generated.""" - updated = self.client.campaigns.add_runs(campaign=self, runs=runs) + updated = self.client.campaigns.add_runs_to_campaign(campaign=self, runs=runs) self._update(updated) return self @@ -233,8 +233,8 @@ def _get_proto_class(self) -> type[CreateCampaignRequestProto]: class CampaignUpdate(CampaignBase, ModelUpdate[CampaignProto]): """Update model for Campaign. - `reports` replaces the campaign's report list. Prefer `campaigns.add_reports` or - `campaigns.add_runs`, which read the current list first. `tags` is unordered, so it + `reports` replaces the campaign's report list. Prefer `Campaign.add_reports` or + `Campaign.add_runs`, which read the current list first. `tags` is unordered, so it may come back in a different order. """ From 378f81c036c34a53c921bd6b9ec21694904402f6 Mon Sep 17 00:00:00 2001 From: Brandon Shippy Date: Thu, 24 Sep 2026 02:51:17 -0400 Subject: [PATCH 6/8] remove create_default_report and allow for create with runs --- .../_tests/resources/test_campaigns.py | 104 +++++++++--------- .../_tests/sift_types/test_campaign.py | 18 --- python/lib/sift_client/resources/campaigns.py | 64 ++--------- .../resources/sync_stubs/__init__.pyi | 30 +---- python/lib/sift_client/sift_types/campaign.py | 10 +- python/lib/sift_client/sift_types/run.py | 2 - 6 files changed, 69 insertions(+), 159 deletions(-) diff --git a/python/lib/sift_client/_tests/resources/test_campaigns.py b/python/lib/sift_client/_tests/resources/test_campaigns.py index d54f1b1e5d..4ad9599195 100644 --- a/python/lib/sift_client/_tests/resources/test_campaigns.py +++ b/python/lib/sift_client/_tests/resources/test_campaigns.py @@ -1,7 +1,7 @@ """Pytest tests for the Campaigns API. These tests cover get, list, find, create, update, add_reports_to_campaign, -add_runs_to_campaign, archive/unarchive, and report_summaries. +archive/unarchive, and report_summaries. """ from datetime import datetime, timezone @@ -43,18 +43,40 @@ def test_timestamp_str(): @pytest.fixture(scope="session") def campaign_run(sift_client, test_timestamp_str): - """A run with a default report, so it can join a campaign.""" + """A bounded run, so rules can be evaluated over it.""" + from datetime import timedelta + from sift_client.sift_types.run import RunCreate + stop = datetime.now(timezone.utc) return sift_client.runs.create( RunCreate( name=f"test_campaign_run_{test_timestamp_str}", description="sift_client campaign tests", - create_default_report=True, + start_time=stop - timedelta(minutes=5), + stop_time=stop, ) ) +@pytest.fixture(scope="session") +def campaign_report(sift_client, campaign_run, test_timestamp_str): + """A report over `campaign_run`, which is what a campaign holds.""" + rules = sift_client.rules.list_(limit=1) + if not rules: + pytest.skip("no rules in this organization") + try: + sift_client.reports.create_from_rules( + name=f"test_campaign_report_{test_timestamp_str}", run=campaign_run, rules=rules + ) + except ValueError: + # The report is created, but the call raises looking up a job ListJobs never returns. + pass + reports = sift_client.reports.list_(run=campaign_run) + assert reports, "no report was created over the run" + return reports[0] + + @pytest.fixture(scope="session") def new_campaign(sift_client, test_timestamp_str): """Create a campaign for the session and archive it on teardown.""" @@ -181,12 +203,12 @@ def test_update_accepts_dict(self, campaigns_api_sync, new_campaign, test_timest campaigns_api_sync.update(new_campaign, {"name": new_campaign.name}) - def test_add_reports(self, campaigns_api_sync, campaign_run, test_timestamp_str): + def test_add_reports(self, campaigns_api_sync, campaign_report, test_timestamp_str): """Test adding reports to a campaign without dropping the existing ones.""" campaign = campaigns_api_sync.create( CampaignCreate(name=f"test_campaign_reports_{test_timestamp_str}") ) - report_id = campaign_run.default_report_id + report_id = campaign_report._id_or_error added = campaigns_api_sync.add_reports_to_campaign(campaign, [report_id]) assert [r.report_id for r in added.report_summaries] == [report_id] @@ -197,70 +219,46 @@ def test_add_reports(self, campaigns_api_sync, campaign_run, test_timestamp_str) campaigns_api_sync.archive(campaign) - def test_add_runs(self, campaigns_api_sync, campaign_run, test_timestamp_str): - """Test adding runs to a campaign through their default reports.""" + def test_create_from_reports(self, campaigns_api_sync, campaign_report, test_timestamp_str): + """Test seeding a new campaign from a report.""" campaign = campaigns_api_sync.create( - CampaignCreate(name=f"test_campaign_runs_{test_timestamp_str}") + CampaignCreate(name=f"test_campaign_from_reports_{test_timestamp_str}"), + reports=[campaign_report], ) - added = campaigns_api_sync.add_runs_to_campaign(campaign, [campaign_run]) - assert [r.report_id for r in added.report_summaries] == [campaign_run.default_report_id] + assert [r.report_id for r in campaign.report_summaries] == [campaign_report._id_or_error] campaigns_api_sync.archive(campaign) - def test_add_runs_rejects_run_without_report( - self, campaigns_api_sync, sift_client, new_campaign, test_timestamp_str - ): - """Test that add_runs fails clearly when a run has no report at all.""" - from sift_client.sift_types.run import RunCreate - - run = sift_client.runs.create( - RunCreate(name=f"test_campaign_no_report_{test_timestamp_str}") - ) - - with pytest.raises(ValueError, match="no report"): - campaigns_api_sync.add_runs_to_campaign(new_campaign, [run]) - - def test_add_runs_falls_back_to_a_report_over_the_run( - self, campaigns_api_sync, campaign_run, test_timestamp_str + def test_create_from_runs( + self, campaigns_api_sync, sift_client, campaign_run, campaign_report, test_timestamp_str ): - """Test that a run with no default report joins through a report search.""" - # Hide the default report so add_runs must search for one over the run. - run = campaign_run.model_copy(update={"default_report_id": None}) - campaign = campaigns_api_sync.create( - CampaignCreate(name=f"test_campaign_fallback_{test_timestamp_str}") - ) - - added = campaigns_api_sync.add_runs_to_campaign(campaign, [run]) - - assert [r.report_id for r in added.report_summaries] == [campaign_run.default_report_id] - campaigns_api_sync.archive(campaign) + """Test that seeding from a run collects every report over it.""" + expected = {r._id_or_error for r in sift_client.reports.list_(run=campaign_run)} - def test_create_from_runs(self, campaigns_api_sync, campaign_run, test_timestamp_str): - """Test seeding a new campaign from a run.""" campaign = campaigns_api_sync.create( CampaignCreate(name=f"test_campaign_from_runs_{test_timestamp_str}"), runs=[campaign_run], ) - assert [r.report_id for r in campaign.report_summaries] == [campaign_run.default_report_id] + assert {r.report_id for r in campaign.report_summaries} == expected campaigns_api_sync.archive(campaign) - def test_create_rejects_multiple_seeds(self, campaigns_api_sync, campaign_run): + def test_create_rejects_multiple_seeds(self, campaigns_api_sync, campaign_report, campaign_run): """Test that create rejects more than one seed.""" with pytest.raises(ValueError, match="At most one of"): campaigns_api_sync.create( CampaignCreate(name="ignored"), + reports=[campaign_report], runs=[campaign_run], - reports=[campaign_run.default_report_id], ) - def test_report_summaries(self, campaigns_api_sync, campaign_run, test_timestamp_str): + def test_report_summaries(self, campaigns_api_sync, campaign_report, test_timestamp_str): """Test fetching per-report rule counts for a campaign.""" campaign = campaigns_api_sync.create( CampaignCreate(name=f"test_campaign_summaries_{test_timestamp_str}"), - runs=[campaign_run], + reports=[campaign_report], ) summaries = campaigns_api_sync.report_summaries([campaign]) @@ -317,7 +315,7 @@ async def test_async_api(self, campaigns_api_async, new_campaign): assert fetched.id_ == new_campaign.id_ def test_add_reports_reads_the_server_list( - self, campaigns_api_sync, campaign_run, test_timestamp_str + self, campaigns_api_sync, campaign_report, test_timestamp_str ): """A stale Campaign handle must not drop reports added since it was fetched.""" campaign = campaigns_api_sync.create( @@ -327,32 +325,36 @@ def test_add_reports_reads_the_server_list( # Another caller adds a report that `stale` knows nothing about. campaigns_api_sync.add_reports_to_campaign( - campaign._id_or_error, [campaign_run.default_report_id] + campaign._id_or_error, [campaign_report._id_or_error] ) assert [r.report_id for r in stale.report_summaries] == [] - merged = campaigns_api_sync.add_reports_to_campaign(stale, [campaign_run.default_report_id]) + merged = campaigns_api_sync.add_reports_to_campaign(stale, [campaign_report._id_or_error]) - assert [r.report_id for r in merged.report_summaries] == [campaign_run.default_report_id] + assert [r.report_id for r in merged.report_summaries] == [campaign_report._id_or_error] campaigns_api_sync.archive(campaign) - def test_campaign_runs(self, campaigns_api_sync, campaign_run, test_timestamp_str): + def test_campaign_runs( + self, campaigns_api_sync, campaign_run, campaign_report, test_timestamp_str + ): """Test reading back the runs behind a campaign's reports.""" campaign = campaigns_api_sync.create( CampaignCreate(name=f"test_campaign_runs_prop_{test_timestamp_str}"), - runs=[campaign_run], + reports=[campaign_report], ) assert [r.id_ for r in campaign.runs] == [campaign_run.id_] campaigns_api_sync.archive(campaign) - def test_campaign_report_summaries(self, campaigns_api_sync, campaign_run, test_timestamp_str): + def test_campaign_report_summaries( + self, campaigns_api_sync, campaign_report, test_timestamp_str + ): """Test the rollup from the Campaign instance.""" campaign = campaigns_api_sync.create( CampaignCreate(name=f"test_campaign_rollup_{test_timestamp_str}"), - runs=[campaign_run], + reports=[campaign_report], ) # The service returns these in no fixed order; _from_proto sorts them. diff --git a/python/lib/sift_client/_tests/sift_types/test_campaign.py b/python/lib/sift_client/_tests/sift_types/test_campaign.py index 98934b0eb7..479369cef4 100644 --- a/python/lib/sift_client/_tests/sift_types/test_campaign.py +++ b/python/lib/sift_client/_tests/sift_types/test_campaign.py @@ -171,21 +171,3 @@ def test_unarchive_calls_client(self, mock_campaign, mock_client): mock_client.campaigns.unarchive.assert_called_once_with(campaign=mock_campaign) assert result is mock_campaign - - -class TestRunCreateDefaultReport: - """A run needs a default report before it can join a campaign.""" - - def test_create_default_report_reaches_the_proto(self): - from sift_client.sift_types.run import RunCreate - - proto = RunCreate(name="r", create_default_report=True).to_proto() - - assert proto.create_default_report is True - - def test_unset_by_default(self): - from sift_client.sift_types.run import RunCreate - - proto = RunCreate(name="r").to_proto() - - assert proto.HasField("create_default_report") is False diff --git a/python/lib/sift_client/resources/campaigns.py b/python/lib/sift_client/resources/campaigns.py index 427bcaae82..7301076a93 100644 --- a/python/lib/sift_client/resources/campaigns.py +++ b/python/lib/sift_client/resources/campaigns.py @@ -24,8 +24,8 @@ class CampaignsAPIAsync(ResourceBase): """High-level API for interacting with campaigns. - A campaign is a named list of reports. Runs join a campaign through the reports they - generate. + A campaign is a named list of reports. Seeding from runs collects every report + those runs generated. """ def __init__(self, sift_client: SiftClient): @@ -178,26 +178,25 @@ async def create( Args: create: The campaign definition. reports: Seed with these Reports or report IDs. - runs: Seed with the reports these Runs generated. + runs: Seed with every report these Runs generated. campaign: Duplicate this Campaign or campaign ID. Returns: The created Campaign. Raises: - ValueError: If any run has no report. + ValueError: If more than one seed is provided. """ if isinstance(create, dict): create = CampaignCreate.model_validate(create) if len([seed for seed in (reports, runs, campaign) if seed]) > 1: raise ValueError("At most one of reports, runs, or campaign may be provided") - seed_report_ids = [self._report_id(r) for r in reports] if reports else None - if runs: - # The service silently skips runs with no default report, so resolve here. - seed_report_ids = await self._run_report_ids(runs) created = await self._low_level_client.create_campaign( create=create, - from_report_ids=seed_report_ids, + from_report_ids=[self._report_id(r) for r in reports] if reports else None, + from_run_ids=( + [r._id_or_error if isinstance(r, Run) else r for r in runs] if runs else None + ), from_campaign_id=( campaign._id_or_error if isinstance(campaign, Campaign) else campaign ), @@ -208,7 +207,7 @@ async def update(self, campaign: str | Campaign, update: CampaignUpdate | dict) """Update a Campaign. `reports`, `tags`, and `metadata` are replaced, not merged. Prefer - `add_reports_to_campaign` or `add_runs_to_campaign` to grow the report list. + `add_reports_to_campaign` to grow the report list. Args: campaign: The Campaign or campaign ID to update. @@ -246,26 +245,6 @@ async def add_reports_to_campaign( ] return await self.update(current, CampaignUpdate(reports=merged)) - async def add_runs_to_campaign( - self, campaign: str | Campaign, runs: list[Run] | list[str] - ) -> Campaign: - """Add runs to a campaign through the reports they generated. - - A campaign holds reports, not runs. Each run contributes its default report, or - its most recent report if it has none. - - Args: - campaign: The Campaign or campaign ID to add to. - runs: The Runs or run IDs to add. - - Returns: - The updated Campaign. - - Raises: - ValueError: If any run has no report. - """ - return await self.add_reports_to_campaign(campaign, await self._run_report_ids(runs)) - async def archive(self, campaign: str | Campaign) -> Campaign: """Archive a campaign. @@ -310,31 +289,6 @@ async def report_summaries( ) return {campaign_id: found.get(campaign_id, []) for campaign_id in ids} - async def _run_report_ids(self, runs: list[Run] | list[str]) -> list[str]: - """Resolve runs to the report each one contributes to a campaign.""" - resolved = [ - r if isinstance(r, Run) else await self.client.async_.runs.get(run_id=r) for r in runs - ] - report_ids = [] - missing = [] - for run in resolved: - report_id = run.default_report_id - if not report_id: - found = await self.client.async_.reports.list_( - run=run, order_by="created_date desc", limit=1 - ) - report_id = found[0]._id_or_error if found else None - if report_id: - report_ids.append(report_id) - else: - missing.append(run.name) - if missing: - raise ValueError( - f"These runs have no report, so they cannot join a campaign: {missing}. " - "Create runs with create_default_report=True, or build a report over the run." - ) - return report_ids - @staticmethod def _report_id(report: Report | str) -> str: return report if isinstance(report, str) else report._id_or_error diff --git a/python/lib/sift_client/resources/sync_stubs/__init__.pyi b/python/lib/sift_client/resources/sync_stubs/__init__.pyi index 39c70f1a00..6638ec33a4 100644 --- a/python/lib/sift_client/resources/sync_stubs/__init__.pyi +++ b/python/lib/sift_client/resources/sync_stubs/__init__.pyi @@ -765,8 +765,8 @@ class CampaignsAPI: High-level API for interacting with campaigns. - A campaign is a named list of reports. Runs join a campaign through the reports they - generate. + A campaign is a named list of reports. Seeding from runs collects every report + those runs generated. """ def __init__(self, sift_client: SiftClient): @@ -792,26 +792,6 @@ class CampaignsAPI: """ ... - def add_runs_to_campaign( - self, campaign: str | Campaign, runs: list[Run] | list[str] - ) -> Campaign: - """Add runs to a campaign through the reports they generated. - - A campaign holds reports, not runs. Each run contributes its default report, or - its most recent report if it has none. - - Args: - campaign: The Campaign or campaign ID to add to. - runs: The Runs or run IDs to add. - - Returns: - The updated Campaign. - - Raises: - ValueError: If any run has no report. - """ - ... - def archive(self, campaign: str | Campaign) -> Campaign: """Archive a campaign. @@ -838,14 +818,14 @@ class CampaignsAPI: Args: create: The campaign definition. reports: Seed with these Reports or report IDs. - runs: Seed with the reports these Runs generated. + runs: Seed with every report these Runs generated. campaign: Duplicate this Campaign or campaign ID. Returns: The created Campaign. Raises: - ValueError: If any run has no report. + ValueError: If more than one seed is provided. """ ... @@ -961,7 +941,7 @@ class CampaignsAPI: """Update a Campaign. `reports`, `tags`, and `metadata` are replaced, not merged. Prefer - `add_reports_to_campaign` or `add_runs_to_campaign` to grow the report list. + `add_reports_to_campaign` to grow the report list. Args: campaign: The Campaign or campaign ID to update. diff --git a/python/lib/sift_client/sift_types/campaign.py b/python/lib/sift_client/sift_types/campaign.py index 76823dd2d8..102ff96f9f 100644 --- a/python/lib/sift_client/sift_types/campaign.py +++ b/python/lib/sift_client/sift_types/campaign.py @@ -158,12 +158,6 @@ def add_reports(self, reports: list[Report] | list[str]) -> Campaign: self._update(updated) return self - def add_runs(self, runs: list[Run] | list[str]) -> Campaign: - """Add runs to the campaign through the reports they generated.""" - updated = self.client.campaigns.add_runs_to_campaign(campaign=self, runs=runs) - self._update(updated) - return self - def update(self, update: CampaignUpdate | dict) -> Campaign: """Update the Campaign. @@ -233,8 +227,8 @@ def _get_proto_class(self) -> type[CreateCampaignRequestProto]: class CampaignUpdate(CampaignBase, ModelUpdate[CampaignProto]): """Update model for Campaign. - `reports` replaces the campaign's report list. Prefer `Campaign.add_reports` or - `Campaign.add_runs`, which read the current list first. `tags` is unordered, so it + `reports` replaces the campaign's report list. Prefer `Campaign.add_reports`, + which reads the current list first. `tags` is unordered, so it may come back in a different order. """ diff --git a/python/lib/sift_client/sift_types/run.py b/python/lib/sift_client/sift_types/run.py index 7f70047167..b61968d7b8 100644 --- a/python/lib/sift_client/sift_types/run.py +++ b/python/lib/sift_client/sift_types/run.py @@ -223,8 +223,6 @@ class RunCreate(RunBase, ModelCreate[CreateRunRequestProto]): name: str client_key: str | None = None organization_id: str | None = None - create_default_report: bool | None = None - """Create a report for the run, so it can be added to a campaign.""" def _get_proto_class(self) -> type[CreateRunRequestProto]: return CreateRunRequestProto From 75bb81719f3b610807fba27d00ddbd267228fec4 Mon Sep 17 00:00:00 2001 From: Brandon Shippy Date: Fri, 25 Sep 2026 18:42:32 -0400 Subject: [PATCH 7/8] python: address pr comments --- python/lib/sift_client/resources/campaigns.py | 3 +++ .../lib/sift_client/resources/sync_stubs/__init__.pyi | 3 +++ python/lib/sift_client/sift_types/campaign.py | 11 +++++++++++ 3 files changed, 17 insertions(+) diff --git a/python/lib/sift_client/resources/campaigns.py b/python/lib/sift_client/resources/campaigns.py index 7301076a93..f3fe2fbb2a 100644 --- a/python/lib/sift_client/resources/campaigns.py +++ b/python/lib/sift_client/resources/campaigns.py @@ -228,6 +228,9 @@ async def add_reports_to_campaign( ) -> Campaign: """Add reports to a campaign, keeping the ones already there. + `CampaignService` has no append RPC, so this reads the report list, merges, and + writes it back. Two concurrent calls drop one side's reports. + Args: campaign: The Campaign or campaign ID to add to. reports: The Reports or report IDs to add. diff --git a/python/lib/sift_client/resources/sync_stubs/__init__.pyi b/python/lib/sift_client/resources/sync_stubs/__init__.pyi index 6638ec33a4..8a5520e5b4 100644 --- a/python/lib/sift_client/resources/sync_stubs/__init__.pyi +++ b/python/lib/sift_client/resources/sync_stubs/__init__.pyi @@ -783,6 +783,9 @@ class CampaignsAPI: ) -> Campaign: """Add reports to a campaign, keeping the ones already there. + `CampaignService` has no append RPC, so this reads the report list, merges, and + writes it back. Two concurrent calls drop one side's reports. + Args: campaign: The Campaign or campaign ID to add to. reports: The Reports or report IDs to add. diff --git a/python/lib/sift_client/sift_types/campaign.py b/python/lib/sift_client/sift_types/campaign.py index 102ff96f9f..effc9f177f 100644 --- a/python/lib/sift_client/sift_types/campaign.py +++ b/python/lib/sift_client/sift_types/campaign.py @@ -28,6 +28,7 @@ if TYPE_CHECKING: from sift_client.client import SiftClient from sift_client.sift_types.run import Run + from sift_client.sift_types.user import User class RuleStatistics(BaseModel): @@ -144,6 +145,16 @@ def reports(self) -> list[Report]: return [] return self.client.reports.list_(report_ids=[r.report_id for r in self.report_summaries]) + @property + def created_by(self) -> User: + """Fetch the User that created this campaign.""" + return self.client.users.get(user_id=self.created_by_user_id) + + @property + def modified_by(self) -> User: + """Fetch the User that last modified this campaign.""" + return self.client.users.get(user_id=self.modified_by_user_id) + @property def runs(self) -> list[Run]: """Fetch the Runs behind this campaign's reports. Reports with no run are skipped.""" From ff32a37300ffe3e665cba0f2dadbd6403b4175ff Mon Sep 17 00:00:00 2001 From: Brandon Shippy Date: Fri, 25 Sep 2026 21:15:56 -0400 Subject: [PATCH 8/8] python: pr comments --- .../_internal/low_level_wrappers/campaigns.py | 15 +++++++++++++-- python/lib/sift_client/resources/campaigns.py | 4 ++++ .../sift_client/resources/sync_stubs/__init__.pyi | 3 +++ 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/python/lib/sift_client/_internal/low_level_wrappers/campaigns.py b/python/lib/sift_client/_internal/low_level_wrappers/campaigns.py index cb9699d639..223f4a72a8 100644 --- a/python/lib/sift_client/_internal/low_level_wrappers/campaigns.py +++ b/python/lib/sift_client/_internal/low_level_wrappers/campaigns.py @@ -104,6 +104,7 @@ async def list_campaigns( query_filter: str | None = None, order_by: str | None = None, include_archived: bool = False, + skip_report_summaries: bool = False, ) -> tuple[list[Campaign], str]: """List one page of campaigns. @@ -113,11 +114,15 @@ async def list_campaigns( query_filter: A CEL filter string. order_by: How to order the retrieved campaigns. include_archived: Include archived campaigns in the results. + skip_report_summaries: Omit the per-report counts. Much faster for large campaigns. Returns: A tuple of (campaigns, next_page_token). """ - request_kwargs: dict[str, Any] = {"include_archived": include_archived} + request_kwargs: dict[str, Any] = { + "include_archived": include_archived, + "skip_report_summaries": skip_report_summaries, + } if page_size is not None: request_kwargs["page_size"] = page_size if page_token is not None: @@ -142,6 +147,7 @@ async def list_all_campaigns( page_size: int | None = DEFAULT_PAGE_SIZE, max_results: int | None = None, include_archived: bool = False, + skip_report_summaries: bool = False, ) -> list[Campaign]: """List every matching campaign. @@ -151,13 +157,18 @@ async def list_all_campaigns( page_size: The number of results to fetch per request. max_results: Maximum number of results to return. include_archived: Include archived campaigns in the results. + skip_report_summaries: Omit the per-report counts. Much faster for large campaigns. Returns: A list of all matching campaigns. """ return await self._handle_pagination( self.list_campaigns, - kwargs={"query_filter": query_filter, "include_archived": include_archived}, + kwargs={ + "query_filter": query_filter, + "include_archived": include_archived, + "skip_report_summaries": skip_report_summaries, + }, order_by=order_by, max_results=max_results, page_size=page_size, diff --git a/python/lib/sift_client/resources/campaigns.py b/python/lib/sift_client/resources/campaigns.py index f3fe2fbb2a..34acc7b5a7 100644 --- a/python/lib/sift_client/resources/campaigns.py +++ b/python/lib/sift_client/resources/campaigns.py @@ -85,6 +85,7 @@ async def list_( # common filters description_contains: str | None = None, include_archived: bool = False, + skip_report_summaries: bool = False, filter_query: str | None = None, order_by: str | None = None, limit: int | None = None, @@ -106,6 +107,8 @@ async def list_( runs: Filter campaigns containing any of these Runs or run IDs. description_contains: Partial description of the campaign. include_archived: If True, include archived campaigns in results. + skip_report_summaries: Omit the per-report counts. Much faster over many + campaigns; fetch counts for the ones you want with `report_summaries`. filter_query: Explicit CEL query to filter campaigns. order_by: Field and direction to order results by. limit: Maximum number of campaigns to return. If None, returns all matches. @@ -141,6 +144,7 @@ async def list_( order_by=order_by, max_results=limit, include_archived=include_archived, + skip_report_summaries=skip_report_summaries, **({"page_size": page_size} if page_size is not None else {}), ) return self._apply_client_to_instances(campaigns) diff --git a/python/lib/sift_client/resources/sync_stubs/__init__.pyi b/python/lib/sift_client/resources/sync_stubs/__init__.pyi index 8a5520e5b4..6c64e3e099 100644 --- a/python/lib/sift_client/resources/sync_stubs/__init__.pyi +++ b/python/lib/sift_client/resources/sync_stubs/__init__.pyi @@ -882,6 +882,7 @@ class CampaignsAPI: runs: list[Run] | list[str] | None = None, description_contains: str | None = None, include_archived: bool = False, + skip_report_summaries: bool = False, filter_query: str | None = None, order_by: str | None = None, limit: int | None = None, @@ -903,6 +904,8 @@ class CampaignsAPI: runs: Filter campaigns containing any of these Runs or run IDs. description_contains: Partial description of the campaign. include_archived: If True, include archived campaigns in results. + skip_report_summaries: Omit the per-report counts. Much faster over many + campaigns; fetch counts for the ones you want with `report_summaries`. filter_query: Explicit CEL query to filter campaigns. order_by: Field and direction to order results by. limit: Maximum number of campaigns to return. If None, returns all matches.