Repository navigation
test(reporting): run the spec's canonical coverage vectors and align the vocabularies - #1738
Merged
Merged
Conversation
…the vocabularies The parity test now runs the spec repository's coverage vectors (adcontextprotocol/adcp#8616, 023bbd26e: 113 cases), vendored under tests/fixtures/reporting_subscriptions/spec/ and pinned by SHA-256, in place of the vendored TypeScript table. Rows are derived with IANA tzdata 2026f, so the dev extra pins tzdata==2026.6 and the test resolves zones from it, never the host's zoneinfo. All 113 cases pass. Two disagreements with the vectors are fixed: - excluded[].offering_id was ignored, so an offering-scoped capacity exclusion under match "all" removed every offering's periods (max-accounts-capacity-per-offering; core/reporting-subscription-coverage.json excluded[].offering_id). - A selector period_duration was never validated, so P1W resolved to no offering instead of failing (p1w-is-not-a-schedule-duration; core/reporting-schedule.json period_duration has no W designator). Coverage input errors are typed: CoverageInputError (an ADCPError and a ValueError) carries code COVERAGE_INPUT_INCOMPLETE or COVERAGE_INPUT_INVALID, the detail from error-codes.json (missing_account_timezone, invalid_period_duration, ...) and recovery "correctable". The vocabularies are pinned against the vendored files: REPORTING_SUBSCRIPTION_ERROR_CODES and COVERAGE_INPUT_DETAILS against error-codes.json, and the new COVERAGE_RULE_ORDER and COVERAGE_REASON_CODES against reason-codes.json. The buyer facade follows error-codes.json: SUBSCRIPTION_OPERATOR_SCOPE_REQUIRED applies when an all-accounts subscription names a destination without operator_domains, and the errors carry their context (would_revoke, delivery_config_id and account_id, nearest with every failed attribute). Refs #1695 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — clean diff, no blocking or medium findings.
Checked:
- No findings at any severity; reviewer verified the change end-to-end.
CoverageInputErrorsubclasses bothADCPErrorandValueError, preserving existingexcept ValueErrorcatch sites (_clock.py:115).excluded[].offering_idfix flows correctly through pre-existingSubscriptionException.removes(..., offering_id)scoping (coverage.py:331,341,825)._mismatches+SUBSCRIPTION_OPERATOR_SCOPE_REQUIREDgating ondestination is not Noneper spec, both covered by updated facade tests (facade.py:519-542).- Source changes backed by SHA-256-pinned canonical coverage vectors; tzdata==2026.6 zoneinfo reset fails closed on any other release.
- No stale references to renamed symbols.
Gate checks: high_risk=false; gated_paths=false; no no-auto-approve team match; no prior decision. Decision table rows 1–8 do not fire → row 9 approve.
Non-blocking note (not a finding): new public exports (CoverageInputError, COVERAGE_REASON_CODES/RULE_ORDER/VOCABULARY_VERSION/INPUT_DETAILS) and the ValueError→CoverageInputError change ship under a test(reporting): prefix, which release-please will not release. If these are meant to reach PyPI, a feat in the stack must carry them — worth confirming before merge, but does not block.
This was referenced Oct 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Folds the spec's canonical reporting-subscription vectors (adcontextprotocol/adcp#8616,
023bbd26e) into the Python SDK. Stacked on #1737.Parity test on the spec vectors
tests/fixtures/reporting_subscriptions/spec/vendorsvectors.json(113 cases),case.schema.json,error-codes.jsonandreason-codes.json. Each is pinned by SHA-256 in the test. The vendored TypeScript table is removed.tzdata==2026.6(2026f). The test resets zoneinfo to that distribution and refuses to run on any other release, so the host's zoneinfo can't drift the results.expected_errorcases, each assertingcodeanddetail.Disagreements found and fixed
max-accounts-capacity-per-offeringexcluded[].offering_id, so an offering-scoped capacity exclusion removed every offering's periodscore/reporting-subscription-coverage.jsonexcluded[].offering_id: each entry removes only its own offering's periodsp1w-is-not-a-schedule-durationperiod_duration; P1W simply matched no offeringcore/reporting-schedule.jsonperiod_durationpattern has noW(a week isP7D):COVERAGE_INPUT_INVALID/invalid_period_durationNo other Python result disagrees with a vector.
Vocabularies (
error-codes.json,reason-codes.json)Typed coverage errors.
CoverageInputError(anADCPErrorand aValueError) carries:code,COVERAGE_INPUT_INCOMPLETEorCOVERAGE_INPUT_INVALID;detail, one of the 11 spec details;recovery="correctable", withdispatchedunset.Every coverage raise site maps to its detail.
Pins. Tests pin the constants to the vendored files:
REPORTING_SUBSCRIPTION_ERROR_CODES(P3) equalserror-codes.json's buyer codes, recovery and dispatch;COVERAGE_INPUT_DETAILSequals its detail list;COVERAGE_RULE_ORDER,COVERAGE_REASON_CODES(20 reasons) andCOVERAGE_VOCABULARY_VERSIONequalreason-codes.json.Facade (P4) changes to match the spec:
SUBSCRIPTION_OPERATOR_SCOPE_REQUIREDnow applies exactly when an all-accounts subscription names a destination withoutoperator_domains;would_revoke,delivery_config_idandaccount_id, andnearest(offerings failing on the fewest attributes, with every failed attribute).Gaps to note
COVERAGE_BUDGET_EXCEEDED.REPORTING_STORAGE_NOT_DURABLE,REPORTING_SCHEMA_BEHIND,SUBSCRIPTIONS_REQUIRE_STANDARD_DIMENSIONS,SUBSCRIPTION_ROSTER_UNAVAILABLE) land with P5 and P7.Tests
test_reporting_subscription_parity.py: 113 vectors, the hash pin, both vocabulary pins, and schema validity, skipping the onewire_valid: falsecase.Refs #1695
🤖 Generated with Claude Code