Repository navigation
feat(reporting-storage): let the Postgres service factory build a subclass - #1589
Conversation
…class postgres_reliable_reporting_service always constructed the base ReliableReportingService, while ReliableReportingService.memory builds cls. Hosts override the installed tool methods on a subclass, so the Embedded Sales Agent had to copy the factory body to keep its subclass. Add a service_class keyword, typed so the call returns the subclass. Fixes #1588 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
![aao-secretariat[bot]](https://avatars.githubusercontent.com/u/221690821?s=60&v=4){kind=small}
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — clean, additive PR with no blocking or medium findings.
Checked: adds a service_class keyword to postgres_reliable_reporting_service so hosts build a ReliableReportingService subclass through the factory (cls(...)) rather than copying its composition body, mirroring the existing memory() classmethod pattern. Overloads type the subclass/default return paths correctly and are covered by both a runtime test and a mypy/pyright type-check fixture. Correct feat(reporting-storage) semver for a non-breaking addition; no wire shape, generated types, credentials, or operational paths touched.
Decision-table walk: no critical/high findings (row 1 n/a); gated_paths is false (row 2 n/a); high_risk false (rows 3/5 n/a); no medium findings (rows 4/8 n/a); no no-auto-approve team match (row 7 n/a); no prior decision (row 6 n/a). Falls through to row 9 → approve. Note review_decision is REVIEW_REQUIRED, but row 2 only gates when gated_paths is true, which it is not here.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| production: ReportingProductionOptions | None = ..., | ||
| service_class: None = ..., | ||
| **kwargs: Any, | ||
| ) -> ReliableReportingService: ... |
| production: ReportingProductionOptions | None = ..., | ||
| service_class: type[_ServiceT], | ||
| **kwargs: Any, | ||
| ) -> _ServiceT: ... |
…st (#1601) The IPR push policy maps every main commit to the PR whose merge commit it is. #1589 and #1595 were rebase-merged, so their first commits match no PR and main's IPR check has failed since. List those two exact commits with their PR numbers; authorship is still checked. Document squash-only merges in the releasing guide. #1591's test expected ext={'note': None} to dump as {} by default, but #1600 deliberately keeps nulls inside opaque ext and context objects. Expect the null. Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Fixes #1588.
postgres_reliable_reporting_service(...), which replacedReliableReportingService.postgres(...)in 9.0.0 (#1557), always constructed the baseReliableReportingService, whileSubClass.memory(...)builds the subclass. Hosts override the installed tool methods (get_reporting_status,sync_reporting_status,get_revision_content) on a subclass. Without this, the Embedded Sales Agent copies the factory body, and that copy drifts when the factory gains a store.service_class: type[ReliableReportingService] | None = None.service_class=HostReportingServicereturnsHostReportingService; the default returnsReliableReportingService.service_classmatchesmemory()(fails without the fix). New adopter type-check fixturetests/type_checks/reporting_storage_service_class.py, clean under mypy --strict and pyright.Merge with rebase, not squash. The first commit touches only
packages/adcp-reporting-storage/, so it releases only the storage package (0.1.0b1 is already on PyPI; this goes in 0.1.0b2). The second is a tests-onlytest:commit. A squashed commit touching both trees would count for both components, and afeatwould proposeadcp9.1.0.🤖 Generated with Claude Code