Skip to content

python(feat): campaigns resource - #795

Open
Brandon-Shippy wants to merge 2 commits into
mainfrom
python/campaigns-resource
Open

Brandon-Shippy wants to merge 2 commits into
mainfrom
python/campaigns-resource

Conversation

@Brandon-Shippy

@Brandon-Shippy Brandon-Shippy commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Description

Adds a campaigns resource to sift_client. A campaign groups the reports from a series of runs, so a whole test program can be reviewed at once.

  • CampaignsAPIAsync with the usual get, list_, find, create, update, archive, unarchive, plus get_or_create. Wired up as
    client.campaigns
  • CampaignsLowLevelClient wraps CampaignService; the high-level client has zero proto imports
  • Pydantic models in sift_types/campaign.py
  • add_runs(...) and add_reports(...) grow a campaign without replacing its report list
  • report_summaries(...) returns the per-report rule rollup; Campaign.report_summaries() and Campaign.runs read it back from the campaign
  • RunCreate.create_default_report so a run generates the report a campaign holds
  • Sync wrappers and type stubs registered

Validation

  • End to End testing with manual script written to test each new API feature.
  • Verified against a local stack, including a multi-run test program collected into one campaign

@github-actions

github-actions Bot commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Python docs preview: https://sift-stack.github.io/sift/python/pr-795/

Deployed from 735d09d. The link may take up to a minute to become live as GitHub Pages propagates.

@Brandon-Shippy
Brandon-Shippy force-pushed the python/campaigns-resource branch from 3b1af5d to 41c3e0e Compare September 17, 2026 23:57
@Brandon-Shippy
Brandon-Shippy marked this pull request as ready for review September 17, 2026 23:57

report_id: str
report_name: str = ""
num_annotations: int = 0

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we make this cleaner by moving this into a structure like:

rule_statistics: passed_count: .., open_count} etc

return CampaignReportProto(report_id=kwargs["report_id"])


def tag_names(tags: list[str] | list[Tag] | None) -> list[str]:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like an odd place for this function

)

@property
def report_ids(self) -> list[str]:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need this - a user can do the list comprehension themselves if they really need this.

return [r.report_id for r in self.reports]

@property
def resolved_reports(self) -> list[Report]:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this just be reports and the existing reports to report_summaries

from sift_client.sift_types.run import Run


class CampaignReport(BaseModel):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CampaignReportSummary would be more descriptive

name: str
client_key: str | None = None
organization_id: str | None = None
create_default_report: bool | None = None

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really want to support this? Feels like a bodge and not first class. Would prefer a different method.

return [tag.name if isinstance(tag, Tag) else tag for tag in tags or []]


class Campaign(BaseType[CampaignProto, "Campaign"]):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some helpers around "add_reports`, etc. would be nice.

*,
reports: list[Report] | list[str] | None = None,
runs: list[Run] | list[str] | None = None,
from_campaign: str | Campaign | None = None,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can this just be "campaign"?

)
return self._apply_client_to_instance(created)

async def get_or_create(self, create: CampaignCreate | dict) -> Campaign:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is generally something we should avoid. Any specific reason we have this for campaigns?

updated = await self._low_level_client.update_campaign(update)
return self._apply_client_to_instance(updated)

async def add_reports(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add_reports_to_campaign... etc

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants