From 4b4e532c56605a78653d296e4a6445db38673c58 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sun, 13 Sep 2026 16:07:26 -0400 Subject: [PATCH] fix: the membership status vocabulary moves beside Membership, so the Patreon client never imports the roster 240f11c made PatreonClient._membership import has_paid_access from membership_roster. test_gated_reason::test_no_fetch_path_can_read_the_roster failed on it, correctly: native_ingest_common is a fetch root, patreon_client is reachable from it, and no fetch path may be able to reach the roster. The roster is allowed to explain a skip, never to cause one. MEMBERSHIP_STATUS and has_paid_access are pure platform knowledge with no database behind them. They move to native_ingest_common, next to the Membership type they interpret (the same move C7 made for Membership itself). membership_roster, membership_reconcile, patreon_client and the tests import them from there. There is no re-export from membership_roster. The guard is unchanged. The lapsed-orphan skip from 240f11c stays as it was. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SHQB1YukL3VyvMK8rcbmV9 --- backend/app/services/membership_reconcile.py | 2 +- backend/app/services/membership_roster.py | 76 ++----------------- backend/app/services/native_ingest_common.py | 80 +++++++++++++++++++- backend/app/services/patreon_client.py | 3 +- backend/app/services/subscribestar_client.py | 2 +- tests/test_membership_roster.py | 7 +- tests/test_subscribestar_memberships.py | 4 +- 7 files changed, 91 insertions(+), 83 deletions(-) diff --git a/backend/app/services/membership_reconcile.py b/backend/app/services/membership_reconcile.py index c6a0d44..ce5b057 100644 --- a/backend/app/services/membership_reconcile.py +++ b/backend/app/services/membership_reconcile.py @@ -47,12 +47,12 @@ from sqlalchemy.ext.asyncio import AsyncSession from ..models import Artist, MembershipSync, PlatformMembership, Source from .membership_roster import ( get_sync_state, - has_paid_access, identity_keys_for_source, pair_sources_with_memberships, roster_is_fresh, url_tail, ) +from .native_ingest_common import has_paid_access # Why a source appears in `tracked_not_subscribed`. Ordered strongest first — # the UI renders a different sentence per basis, because collapsing them into diff --git a/backend/app/services/membership_roster.py b/backend/app/services/membership_roster.py index 8cf4945..e36cc50 100644 --- a/backend/app/services/membership_roster.py +++ b/backend/app/services/membership_roster.py @@ -20,8 +20,10 @@ own word — `active_patron`, not some normalised FC value. The mapping from those words to FC's meaning is a read-site concern and belongs in code that can be corrected without a migration, because the vocabulary comes from whatever each platform says and will be discovered per platform rather than designed up -front. `MEMBERSHIP_STATUS` below is a place for that knowledge to accumulate as -platforms are characterised; it is deliberately empty of guesses today. +front. `native_ingest_common.MEMBERSHIP_STATUS` is where that knowledge +accumulates as platforms are characterised, and it holds no guesses. It lives +there rather than here because platform clients need it, and a client may not +import this module (test_gated_reason.py). """ from __future__ import annotations @@ -35,78 +37,10 @@ from sqlalchemy.dialects.postgresql import insert as pg_insert from sqlalchemy.ext.asyncio import AsyncSession from ..models import MembershipSync, PlatformMembership, Source +from .native_ingest_common import has_paid_access log = logging.getLogger(__name__) -# Platform word -> whether the account currently has paid access. -# -# Every entry here must come from a CHARACTERISED response, never from API docs -# or a plausible guess — project rule 130, and inventing a status before seeing -# it in a real payload is exactly the failure it names. -# -# patreon: from a live capture of the operator's own session, 2026-09-10 -# (Scribe note #3886). Only two values were OBSERVED in `patron_status` and -# only those two are here. -# -# `declined_patron` is deliberately ABSENT even though it looks obviously -# right. It appears in the request's `filter[membership_type]`, and the capture -# proved that filter is NOT the same vocabulary as the attribute — a row -# selected by the filter as `free_member` came back with -# `patron_status: former_patron`, a word the filter does not contain. Reading -# the filter as an enum is the specific mistake the capture caught; adding -# `declined_patron` on the strength of it would be repeating that mistake one -# step later. -# -# Unknown words are NOT an error: an unrecognised status means the roster -# records evidence it cannot yet interpret, which is a better state than -# dropping the row or asserting a meaning for it. -# -# subscribestar: from a live capture of the account's /subscriptions page, -# 2026-09-13 (Scribe note #3989). SubscribeStar gives NO per-row status word — -# a membership's state is which of two tables it sits in — so the "word" stored -# is the table card's own `data-identifier`, verbatim. Those two identifiers are -# the whole vocabulary; there is nothing further to characterise later. -MEMBERSHIP_STATUS: dict[str, dict[str, bool]] = { - "patreon": { - "active_patron": True, - "former_patron": False, - }, - "subscribestar": { - "active_subscriptions": True, - "cancelled_subscriptions": False, - }, -} - - -def has_paid_access( - platform: str, status: str | None, *, is_free_member: bool = False, -) -> bool | None: - """Does this membership mean the account currently PAYS for access? - - Returns None for a status this code has not been taught, which callers must - treat as "unknown" rather than as False. The difference matters: False says - the operator has lost access, and asserting that from an unrecognised word - would tell them to cancel a source they are still paying for. - - `is_free_member` is a second axis, not a status, and that is Patreon's - design rather than ours: the capture shows a free follow expressed as a - boolean alongside `patron_status`, so a "current" membership can still be - one nobody is paying for. Taking status alone would report a free follower - as a paying patron, and C4 would then never offer to clean it up. - - (Honest limit: the capture contains no ACTIVE free member, so it cannot - demonstrate the two axes coming apart. The separation is what the payload's - shape says; the sample only shows it is possible, not that it happens.) - """ - if status is None: - return None - known = MEMBERSHIP_STATUS.get(platform, {}).get(status) - if known is None: - return None - if not known: - return False - return not is_free_member - async def touch_membership( session: AsyncSession, diff --git a/backend/app/services/native_ingest_common.py b/backend/app/services/native_ingest_common.py index 7731163..29226b0 100644 --- a/backend/app/services/native_ingest_common.py +++ b/backend/app/services/native_ingest_common.py @@ -231,7 +231,7 @@ class Membership: `status` carries the PLATFORM's own word, verbatim and unmapped (`active_patron`, `former_patron`, ...). Deciding what it means is the read - site's job — `membership_roster.has_paid_access` — precisely so an + site's job — `has_paid_access`, below — precisely so an unrecognised word records as evidence rather than as a decision. `is_free_member` is SEPARATE from status and must stay that way. Patreon @@ -396,3 +396,81 @@ class BaseNativeDownloader: sidecar_path = media_path.with_suffix(".json") sidecar_path.write_text(json.dumps(data, indent=2)) return sidecar_path + +# --- membership status vocabulary (#387) ------------------------------------ +# +# Lives here, beside `Membership`, rather than in `membership_roster`. It is +# pure platform knowledge with no database behind it, and the platform clients +# need it too. Patreon's must tell a lapsed membership to a deleted creator +# (skippable) from a paid one it cannot attribute (drift), and a client may not +# import `membership_roster`: test_gated_reason.py forbids any fetch path from +# reaching the roster, so the roster can explain a skip but never cause one. +# +# Platform word -> whether the account currently has paid access. +# +# Every entry here must come from a CHARACTERISED response, never from API docs +# or a plausible guess — project rule 130, and inventing a status before seeing +# it in a real payload is exactly the failure it names. +# +# patreon: from a live capture of the operator's own session, 2026-09-10 +# (Scribe note #3886). Only two values were OBSERVED in `patron_status` and +# only those two are here. +# +# `declined_patron` is deliberately ABSENT even though it looks obviously +# right. It appears in the request's `filter[membership_type]`, and the capture +# proved that filter is NOT the same vocabulary as the attribute — a row +# selected by the filter as `free_member` came back with +# `patron_status: former_patron`, a word the filter does not contain. Reading +# the filter as an enum is the specific mistake the capture caught; adding +# `declined_patron` on the strength of it would be repeating that mistake one +# step later. +# +# Unknown words are NOT an error: an unrecognised status means the roster +# records evidence it cannot yet interpret, which is a better state than +# dropping the row or asserting a meaning for it. +# +# subscribestar: from a live capture of the account's /subscriptions page, +# 2026-09-13 (Scribe note #3989). SubscribeStar gives NO per-row status word — +# a membership's state is which of two tables it sits in — so the "word" stored +# is the table card's own `data-identifier`, verbatim. Those two identifiers are +# the whole vocabulary; there is nothing further to characterise later. +MEMBERSHIP_STATUS: dict[str, dict[str, bool]] = { + "patreon": { + "active_patron": True, + "former_patron": False, + }, + "subscribestar": { + "active_subscriptions": True, + "cancelled_subscriptions": False, + }, +} + + +def has_paid_access( + platform: str, status: str | None, *, is_free_member: bool = False, +) -> bool | None: + """Does this membership mean the account currently PAYS for access? + + Returns None for a status this code has not been taught, which callers must + treat as "unknown" rather than as False. The difference matters: False says + the operator has lost access, and asserting that from an unrecognised word + would tell them to cancel a source they are still paying for. + + `is_free_member` is a second axis, not a status, and that is Patreon's + design rather than ours: the capture shows a free follow expressed as a + boolean alongside `patron_status`, so a "current" membership can still be + one nobody is paying for. Taking status alone would report a free follower + as a paying patron, and C4 would then never offer to clean it up. + + (Honest limit: the capture contains no ACTIVE free member, so it cannot + demonstrate the two axes coming apart. The separation is what the payload's + shape says; the sample only shows it is possible, not that it happens.) + """ + if status is None: + return None + known = MEMBERSHIP_STATUS.get(platform, {}).get(status) + if known is None: + return None + if not known: + return False + return not is_free_member diff --git a/backend/app/services/patreon_client.py b/backend/app/services/patreon_client.py index 6609047..07e2c9c 100644 --- a/backend/app/services/patreon_client.py +++ b/backend/app/services/patreon_client.py @@ -58,6 +58,7 @@ from .native_ingest_common import ( NativeDriftError, NativeIngestError, basename_from_url, + has_paid_access, make_session, retry_after_seconds, ) @@ -650,8 +651,6 @@ class PatreonClient: membership without a creator is something FC cannot vouch for, and dropping it would read downstream as a cancellation. """ - from .membership_roster import has_paid_access - attrs = member.get("attributes") or {} if "patron_status" not in attrs: raise PatreonDriftError( diff --git a/backend/app/services/subscribestar_client.py b/backend/app/services/subscribestar_client.py index e51e026..9ae848a 100644 --- a/backend/app/services/subscribestar_client.py +++ b/backend/app/services/subscribestar_client.py @@ -316,7 +316,7 @@ _ROSTER_URL = f"{_ROSTER_BASE}/subscriptions" # `data-identifier`, the one vocabulary that names a state: the table class # inside the cancelled card says `for-unsubscribed_users`, a different word for # the same list (note #3989, CORRECTION 1). The identifier is stored verbatim as -# Membership.status and mapped in membership_roster.MEMBERSHIP_STATUS. +# Membership.status and mapped in native_ingest_common.MEMBERSHIP_STATUS. _ROSTER_ACTIVE = "active_subscriptions" _ROSTER_CANCELLED = "cancelled_subscriptions" diff --git a/tests/test_membership_roster.py b/tests/test_membership_roster.py index 1e99855..abdf90c 100644 --- a/tests/test_membership_roster.py +++ b/tests/test_membership_roster.py @@ -10,11 +10,8 @@ import pytest from sqlalchemy import select from backend.app.models import PlatformMembership -from backend.app.services.membership_roster import ( - MEMBERSHIP_STATUS, - has_paid_access, - touch_membership, -) +from backend.app.services.membership_roster import touch_membership +from backend.app.services.native_ingest_common import MEMBERSHIP_STATUS, has_paid_access pytestmark = pytest.mark.integration diff --git a/tests/test_subscribestar_memberships.py b/tests/test_subscribestar_memberships.py index 5c68704..6fa1382 100644 --- a/tests/test_subscribestar_memberships.py +++ b/tests/test_subscribestar_memberships.py @@ -22,8 +22,8 @@ from types import SimpleNamespace import pytest -from backend.app.services.membership_roster import has_paid_access, roster_user_id -from backend.app.services.native_ingest_common import Membership +from backend.app.services.membership_roster import roster_user_id +from backend.app.services.native_ingest_common import Membership, has_paid_access from backend.app.services.subscribestar_client import ( SubscribeStarAuthError, SubscribeStarClient,