Repository navigation
feat(reporting): use owed and list_subscription_principals, as the TypeScript SDK does - #1746
Conversation
There was a problem hiding this comment.
Ladon verdict: Approve
Approve.
Mechanical naming-freeze rename matching adcp-client#3546: expected_subscription_periods→owed_subscription_periods, ExpectedSubscriptionPeriod→OwedSubscriptionPeriod, and ReportingSubscriptionEngine.list_principals→list_subscription_principals. All three renamed public exports retain working deprecation aliases (coverage.py:321,324; engine.py:397), every internal caller is updated to the new name so no deprecation warning trips, and the intentionally-preserved ReportingSubscriptionServiceOptions.list_principals field plus the state-store port's list_principals are correctly left untouched. Docs, tests, and __all__ are consistent. Non-breaking (aliases preserved), so the feat(reporting): semver signal is correct; no wire-shape or discriminated-union change.
No Critical/High/Medium findings. No gated paths, no high-risk flag, no author-team gate, no prior decision — falls through to row 9.
d12b524 to
8d5c038
Compare
8eecfcc to
18b8e9f
Compare
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — clean naming-freeze rename with backward-compatible deprecation aliases.
What I checked:
- Pure rename (
expected_subscription_periods→owed_subscription_periods,ExpectedSubscriptionPeriod→OwedSubscriptionPeriod, enginelist_principals→list_subscription_principals) plus ruff/black formatting churn. - Old names retained as deprecation-warning aliases and still in
__all__, so no public export is removed —feat:is the correct non-breaking semver signal. - All internal callers rewired to new names; the SDK never warns on itself. The unaliased
ManagedSubscriptions.list_principalsrename is not public surface. - No wire field/enum/variant touched, so DR-0032/0033 translation pathway is not implicated.
- New deprecation test covers the alias path; test files updated accordingly.
Gates: high_risk false, gated_paths false (and review_decision is APPROVED regardless), no no-auto-approve team match. Prior decision was approve with no findings; this run is consistent.
No critical/high/medium findings. Falls through to row 9 → approve.
8d5c038 to
55fc790
Compare
18b8e9f to
c1a6cd8
Compare
There was a problem hiding this comment.
Ladon verdict: Approve
Approve.
Clean pass with no blocking or medium findings. The PR implements a naming freeze (DX plan §3.1) renaming three public symbols — expected_subscription_periods→owed_subscription_periods, ExpectedSubscriptionPeriod→OwedSubscriptionPeriod, and ReportingSubscriptionEngine.list_principals→list_subscription_principals — each paired with a deprecation alias/shim kept for one beta, so the change is backward-compatible and the feat(reporting): (minor) semver signal is correct; no break ships. Internal callers, tests, and docs all move to the new names (no spurious DeprecationWarnings), the store port deliberately keeps list_principals per §2.4, and ManagedSubscriptions.list_principals is renamed without an alias but is internal (not in any __all__). Remaining hunks across projection.py, reconciliation_projection.py, worker.py, service.py, conformance.py, consumer_status.py, and _reconcile.py are black/ruff reflows with identical semantics.
Decision-table trace: no critical/high/medium findings (row 1 n/a); gated_paths false and review_decision APPROVED (rows 2 n/a); high_risk false (rows 3/5 n/a); no medium data-loss/schema/infra (row 4 n/a); prior decision was approve (row 6 n/a); no no-auto-approve team match (row 7 n/a); zero medium findings (row 8 n/a). Falls through to row 9 → approve.
55fc790 to
69906d2
Compare
c1a6cd8 to
62c7ca6
Compare
69906d2 to
2a8a119
Compare
62c7ca6 to
0f89d2a
Compare
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — clean pass, no blocking findings.
Checked: Python naming freeze matching adcp-client#3546 — expected_subscription_periods/ExpectedSubscriptionPeriod renamed to owed_*/Owed*, and ReportingSubscriptionEngine.list_principals renamed to list_subscription_principals. All three public renames retain working deprecation shims (type alias; warn-and-delegate function; warn-and-delegate engine method), so the public surface is additive and feat: is the correct semver signal, consistent with the sibling TS SDK. The only un-aliased rename (ManagedSubscriptions.list_principals) is on an internal class reachable only via private service._managed, so it does not break the public surface. Remaining files are formatter reflow, semantically unchanged. No wire-shape change, so DR-0032/0033 translation mandate does not apply.
No Critical/High/Medium findings. Author team gate not triggered. gated_paths is false and review_decision is APPROVED. high_risk is false. Decision table falls through to row 9 → approve.
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — no blocking or medium findings.
Delta since the prior approve is a backward-compatible deprecating rename plus formatter reflow across the reporting modules:
expected_subscription_periods→owed_subscription_periodsExpectedSubscriptionPeriod→OwedSubscriptionPeriodReportingSubscriptionEngine.list_principals→list_subscription_principals
Old names are retained as a warning-emitting shim, a type alias, and a delegating engine method, with a new test pinning the aliases — so the change is non-breaking and feat(reporting): is the correct semver signal. New names remain reachable at the documented adcp.reporting.subscriptions path. The ManagedSubscriptions.list_principals rename has no alias, but that class is internal with no public export, so no public-surface break.
No critical/high/medium findings. high_risk=false, gated_paths=false, review_decision=APPROVED, no no-auto-approve team match. Decision table rows 1–8 do not fire; falls through to row 9 (approve).
2a8a119 to
e74becd
Compare
0f89d2a to
105089f
Compare
…peScript SDK does The naming freeze for the Python low-level surface (DX plan §3.1), matching adcp-client#3546: - expected_subscription_periods is renamed owed_subscription_periods and ExpectedSubscriptionPeriod is renamed OwedSubscriptionPeriod. "Owed" is the one word for what a seller must deliver on both sides; "expected" already means something else in Reliable Reporting. The old names stay for one beta: the function warns with DeprecationWarning and delegates, and the type is an alias. - ReportingSubscriptionEngine.list_principals is renamed list_subscription_principals; the old name warns and delegates. The state-store port keeps list_principals, as §2.4 specifies. Internal callers, tests and docs use the new names. Refs #1695 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
105089f to
268789b
Compare
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — clean pass, no blocking findings.
Checked: this PR is the Python half of the naming freeze — expected_subscription_periods/ExpectedSubscriptionPeriod → owed_* and list_principals → list_subscription_principals. Every renamed public export keeps a backward-compat pathway (function and engine method warn-and-delegate via DeprecationWarning; the type is kept as an alias), so the feat: (minor) semver signal is correct and no feat!: is required. ManagedSubscriptions.list_principals drops its alias but the class is SDK-owned, not public surface. Remaining files are pure ruff/black reformatting with no logic change. A test was added for the deprecated aliases, and docs plus llms.txt were updated.
No Critical/High/Medium findings. high_risk is false; gated_paths is false and review_decision is APPROVED; no no-auto-approve team match. Prior decision was approve with no findings — consistent with this run. Falls through to row 9.
There was a problem hiding this comment.
Ladon verdict: Approve
Approve.
Clean pass — no Critical/High/Medium findings. This is a non-breaking "owed" naming freeze aligning with the TypeScript SDK (adcp-client#3546): expected_subscription_periods→owed_subscription_periods, ExpectedSubscriptionPeriod→OwedSubscriptionPeriod, and ReportingSubscriptionEngine.list_principals→list_subscription_principals.
Verified semver-signal correctness: public-facing names retain deprecation shims (warn+delegate / type alias), the state-store port keeps list_principals per §2.4, and the internal-only ManagedSubscriptions.list_principals (not re-exported from adcp.*) is renamed directly — so feat: is the correct signal, not feat!:. A new test asserts alias identity and the DeprecationWarning. All call sites, tests, and docs updated; remaining hunks are ruff/black churn.
Decision table: no critical/high (row 1 n/a); gated_paths false and review_decision APPROVED (rows 2 n/a); high_risk false (rows 3/5 n/a); no medium findings (rows 4/8 n/a); prior decision was approve (row 6 n/a); no no-auto-approve team match (row 7 n/a). Falls through to row 9 — approve.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — clean pass, no blocking findings.
This is a rename + deprecation + formatting-churn PR. The reviewer confirmed each rename (expected_subscription_periods→owed_subscription_periods, ExpectedSubscriptionPeriod→OwedSubscriptionPeriod, list_principals→list_subscription_principals) ships a retained compat path (warning wrapper, type alias, delegating method), so the additive feat(reporting): without ! is the correct semver signal — not a breaking change. The one unaliased managed-class method rename is on an internal class; the state-store port and ReportingSubscriptionServiceOptions field correctly retain list_principals. Remaining files are logic-identical ruff/black reflow. Docs, llms.txt, and a new deprecated-alias test track the rename.
No critical/high/medium findings. high_risk is false, gated_paths is false (review_decision is APPROVED anyway), no no-auto-approve team match. Prior decision was also a clean approve. None of rows 1–8 fire; falls through to row 9 → approve.
Summary
The Python half of the naming freeze (DX plan §3.1), stacked on #1745, matching the TypeScript SDK's adcp-client#3546. Review only
feat(reporting): use "owed" and list_subscription_principals, as the TypeScript SDK does.expected_subscription_periodsowed_subscription_periodsowedSubscriptionPeriodsV1ExpectedSubscriptionPeriodOwedSubscriptionPeriodOwedSubscriptionPeriodV1ReportingSubscriptionEngine.list_principalslist_subscription_principalsengine.listSubscriptionPrincipalsDeprecationWarningand delegate; the type is an alias, because it still subclasses the consumer'sExpectedReportingPeriod.list_principals, as §2.4 specifies.PrincipalRef(tenant_id, principal_id)already landed in feat(reporting): serve reporting subscriptions from one SubscriptionOptions block #1739.Not renamed
ReportingSubscriptionServiceOptions.list_principals(the advanced composition's hook field) keeps its name. Renaming a dataclass field breaks callers without an alias path, and the managed path (SubscriptionOptions) never asks for it.recordReportingSubscriptionCoverage→recordSubscriptionReadbackhas no Python counterpart to rename: Python writes the readback throughPrincipalService.update_subscription_status.Tests
Refs #1695
🤖 Generated with Claude Code