From d9213e3da8903475d438780466219eb01378deb8 Mon Sep 17 00:00:00 2001 From: Tobias Macey Date: Thu, 20 Aug 2026 18:17:09 -0400 Subject: [PATCH 1/4] feat(anonymization): suppress complements, within a row and across grains The k-anonymity floor only guarded the small side of a count. A count that nearly fills the cohort containing it identifies the members who did NOT do the thing just as precisely: 42 active learners of whom 40 used the chatbot names exactly 2 abstainers. CohortPolicy now declares which secondary counts are strict subsets of which (contained_in), verified against the b2b_analytics dbt SQL, and a subset whose complement is nonzero and below the floor is nulled along with its derived values. Ancestors are walked transitively, so watchers inside engaged inside enrolled is checked at both levels. Counts that are not subsets stay out of it, declared as uncontained with the reason: enrolling does not set active_count, so an org month's enrolling learners are not inside its active learners, and certificates_earned counts events rather than learners, where the subtraction has no meaning at all. Leaving a secondary count out of both lists raises at policy-definition time rather than silently skipping the rule. The second channel opened when the contract-scoped endpoints landed: the same learners are now published at org and contract grain, and the org engagement trend aggregates across an org's contracts. Its event sums add up exactly, so a contract-month the floor withholds is recoverable as org_total minus the visible contract months. 4 of 58 orgs hold more than one contract, so the arithmetic works on production data today. That endpoint now probes the contract-grained view for the months it withholds and blanks its own additive totals for them. The probe compares against the floor in SQL and projects the month alone, so the sub-floor count never enters the process. Learner counts are deliberately left published: a learner active under two contracts is counted in both rows, so subtracting them bounds the withheld cohort rather than revealing it. The content-engagement pair needs no guard at all, since a course run belongs to exactly one contract and its org row and contract row hold identical numbers, leaving no remainder to subtract. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01RVrNyjeL7gSVY61gSUavKf --- src/ol_analytics_api/core/anonymization.py | 216 +++++++++++++++- src/ol_analytics_api/core/db/query.py | 70 ++++- .../tenants/b2b_dashboard/models.py | 97 ++++++- .../b2b_dashboard/routers/organizations.py | 73 ++++++ tests/test_anonymization.py | 239 +++++++++++++++++- tests/test_column_contract.py | 33 +++ tests/test_endpoints.py | 130 +++++++++- tests/test_query_chokepoint.py | 52 +++- 8 files changed, 883 insertions(+), 27 deletions(-) diff --git a/src/ol_analytics_api/core/anonymization.py b/src/ol_analytics_api/core/anonymization.py index bbe7d33..8186fe3 100644 --- a/src/ol_analytics_api/core/anonymization.py +++ b/src/ol_analytics_api/core/anonymization.py @@ -17,15 +17,31 @@ floor is enforced per-column, driven by a `CohortPolicy` the row model declares: - the ``primary`` cohort gates the whole row (below floor -> row withheld), -- each ``secondary`` count is independently nulled when it is sub-floor, and +- each ``secondary`` count is independently nulled when it is sub-floor, +- each ``secondary`` count is *also* nulled when its COMPLEMENT within a cohort + containing it is sub-floor, and - each ``derived`` value is nulled whenever a cohort it is computed over is suppressed (else the hidden count is trivially back-computed from the rate, or read off directly as an average over k engaged_learners -> + total_enrolled_learners`` checks the complement against both. + ``uncontained`` + Secondary counts deliberately subject to no complement rule, each of + which must be justified in the declaring model's docstring. Two things + land here: counts that are not subsets of anything in the row (a + monthly ``enrolling_learners``, where enrolling does not itself make a + learner active), and columns that count *events* rather than entities + (``certificates_earned`` as ``sum(certificate_count)``), where + ``container - count`` can go negative and means nothing either way. + + Every ``secondary`` count must appear in exactly one of ``contained_in`` + and ``uncontained``. Leaving one out is a leak, not an oversight, so it is + rejected at policy-definition time. """ primary: str secondary: tuple[str, ...] = () derived: Mapping[str, tuple[str, ...]] = field(default_factory=dict) + contained_in: Mapping[str, str] = field(default_factory=dict) + uncontained: tuple[str, ...] = () def __post_init__(self) -> None: # A `derived` entry naming a cohort that's neither `primary` nor in @@ -69,12 +107,101 @@ def __post_init__(self) -> None: "secondary cohorts." ) raise ValueError(msg) + self._validate_containment(allowed) # `frozen=True` stops attribute *reassignment*, not mutation of a # mutable object already stored in one — a plain dict handed in (or # reused across CohortPolicy instances by a caller) could still be # mutated in place afterwards. Copy into a read-only view so it # can't be. object.__setattr__(self, "derived", MappingProxyType(dict(self.derived))) + object.__setattr__(self, "contained_in", MappingProxyType(dict(self.contained_in))) + + def _validate_containment(self, allowed: set[str]) -> None: + """Reject a containment declaration that can't be reasoned about. + + Same rationale as the `derived` check above: every failure mode here + ends in a complement going unchecked, so none of them may pass + silently to request time. + """ + for subset, container in self.contained_in.items(): + if subset not in self.secondary: + msg = ( + f"contained_in names {subset!r}, which is not a secondary cohort. " + "Only secondary counts can be suppressed by the complement rule." + ) + raise ValueError(msg) + if container not in allowed: + msg = ( + f"Cohort {subset!r} is declared inside unknown cohort {container!r}. " + f"It must be either the primary cohort {self.primary!r} or in " + "secondary cohorts." + ) + raise ValueError(msg) + for column in self.uncontained: + if column not in self.secondary: + msg = f"uncontained names {column!r}, which is not a secondary cohort." + raise ValueError(msg) + classified = set(self.contained_in) | set(self.uncontained) + if overlap := set(self.contained_in) & set(self.uncontained): + msg = ( + f"Cohorts {sorted(overlap)} are in both contained_in and uncontained. " + "Each secondary count is one or the other." + ) + raise ValueError(msg) + if unclassified := set(self.secondary) - classified: + msg = ( + f"Secondary cohorts {sorted(unclassified)} are classified neither by " + "contained_in nor by uncontained. An undeclared containment silently " + "skips the complement rule, so declare the cohort each is a subset of, " + "or list it in uncontained with the reason in the model's docstring." + ) + raise ValueError(msg) + # A cycle would make the ancestor walk below run forever. It also can't + # describe anything real: strict containment is a partial order. + for subset in self.contained_in: + list(self._ancestors(subset)) + + def _ancestors(self, subset: str) -> Iterator[str]: + """Walk ``subset`` outwards through every cohort that contains it.""" + seen = {subset} + container = self.contained_in.get(subset) + while container is not None: + if container in seen: + msg = f"Containment cycle through cohort {container!r}." + raise ValueError(msg) + seen.add(container) + yield container + container = self.contained_in.get(container) + + def complement_pairs(self) -> tuple[tuple[str, str], ...]: + """Every ``(subset, container)`` pair whose complement must be checked, + including transitive ones. Computed once per response rather than per + row — a policy is fixed ClassVar state on the row model.""" + return tuple( + (subset, container) + for subset in self.contained_in + for container in self._ancestors(subset) + ) + + +@dataclass(frozen=True) +class CrossGrainAdditives: + """Coarse-grained columns that are exact sums over a finer grain's rows. + + ``key_column`` + The column shared by both grains that lines a coarse row up with the + finer rows summing into it (e.g. the activity month). + ``columns`` + The coarse columns that are *exactly* additive across the finer rows. + Only these are recoverable by subtraction, so only these are blanked. + Distinct-entity counts generally are not additive — a learner active + under two contracts is counted in both finer rows — so subtracting + those yields a bound, not a value, and they are left alone rather than + over-suppressed. + """ + + key_column: str + columns: tuple[str, ...] def _is_disclosive(value: int | None, floor: int) -> bool: @@ -86,12 +213,59 @@ def _is_disclosive(value: int | None, floor: int) -> bool: return 0 < value < floor +def _complement_is_disclosive(subset: int, container: int, floor: int) -> bool: + """Does publishing ``subset`` alongside ``container`` name too few of the + entities that are in the container but not the subset? + + Both values are known non-NULL by the time this runs: a NULL secondary is + disclosive on its own and is already suppressed (which skips the pair), and + a NULL primary drops the row. Containers are constrained to the primary or + a secondary at policy-definition time, so there is no third case. + + A complement of exactly 0 is safe for the same reason a count of 0 is: + "everyone did it" singles nobody out. A *negative* complement is not a + small cohort but a false declaration — the subset is provably not inside + the container for this row — and since the whole point of the declaration + is to bound what can be back-computed, an unreasonable one fails closed. + """ + complement = container - subset + return complement != 0 and complement < floor + + +def _suppressed_cohorts(row: Mapping[str, Any], policy: CohortPolicy, floor: int) -> set[str]: + """The secondary cohorts of one row that must not be published.""" + suppressed = {column for column in policy.secondary if _is_disclosive(row.get(column), floor)} + pairs = policy.complement_pairs() + # Suppressing a cohort can hide the container of another pair, which + # removes that pair from play rather than adding one, so this settles in + # at most one pass per level of nesting. Cycles are rejected at + # policy-definition time, so the loop terminates. + changed = True + while changed: + changed = False + for subset, container in pairs: + if subset in suppressed or container in suppressed: + # A complement needs both sides visible to be computed; if the + # container is already withheld, the subset discloses nothing + # beyond itself. + continue + # Indexing, not `.get`: a column missing from the row reads as a + # NULL cohort, which is suppressed above and skipped just before + # this line, so reaching here without both keys is a bug worth a + # KeyError rather than a silently unchecked complement. + if _complement_is_disclosive(row[subset], row[container], floor): + suppressed.add(subset) + changed = True + return suppressed + + def suppress_small_cohorts( rows: list[dict[str, Any]], policy: CohortPolicy, floor: int ) -> list[dict[str, Any]]: """Apply ``policy`` to every row, returning new dicts with sub-floor cohort - counts and their derived values nulled, and rows below the primary floor - dropped. Input rows are not mutated.""" + counts, counts whose complement is sub-floor, and their derived values + nulled, and rows below the primary floor dropped. Input rows are not + mutated.""" kept: list[dict[str, Any]] = [] for row in rows: # `.get(field) or 0` folds both a missing key and a NULL primary to 0, @@ -99,9 +273,7 @@ def suppress_small_cohorts( if (row.get(policy.primary) or 0) < floor: continue redacted = dict(row) - suppressed = { - column for column in policy.secondary if _is_disclosive(redacted.get(column), floor) - } + suppressed = _suppressed_cohorts(redacted, policy, floor) for column in suppressed: redacted[column] = None for column, cohorts in policy.derived.items(): @@ -109,3 +281,33 @@ def suppress_small_cohorts( redacted[column] = None kept.append(redacted) return kept + + +def suppress_cross_grain_additives( + rows: list[dict[str, Any]], + additives: CrossGrainAdditives, + hidden_keys: Collection[Any], +) -> list[dict[str, Any]]: + """Blank the coarse columns that would reconstruct a withheld finer row. + + ``hidden_keys`` is the set of ``additives.key_column`` values for which the + finer grain withheld at least one row. For those keys the caller holds a + coarse total and every finer row but one (or a few), so the difference is + the withheld row's value — a quantity attributable to a cohort the floor + already judged too small to publish. Withholding the coarse total is what + breaks the subtraction; the finer rows themselves stay as they are. + + One withheld finer row is enough to trigger this. Several are not safer: + the difference is then their sum, which can still be a handful of entities. + + Input rows are not mutated. + """ + if not hidden_keys: + return rows + hidden = set(hidden_keys) + return [ + {column: (None if column in additives.columns else value) for column, value in row.items()} + if row.get(additives.key_column) in hidden + else row + for row in rows + ] diff --git a/src/ol_analytics_api/core/db/query.py b/src/ol_analytics_api/core/db/query.py index f6a1576..fdd76dd 100644 --- a/src/ol_analytics_api/core/db/query.py +++ b/src/ol_analytics_api/core/db/query.py @@ -17,11 +17,17 @@ from __future__ import annotations +from collections.abc import Collection from typing import Any, ClassVar, Protocol, cast from sqlmodel import SQLModel -from ol_analytics_api.core.anonymization import CohortPolicy, suppress_small_cohorts +from ol_analytics_api.core.anonymization import ( + CohortPolicy, + CrossGrainAdditives, + suppress_cross_grain_additives, + suppress_small_cohorts, +) from ol_analytics_api.core.db.client import starrocks_pool from ol_analytics_api.core.db.identifiers import validate_sql_identifier @@ -122,6 +128,48 @@ def build_existence_check(schema: str, table: str, filter_columns: tuple[str, .. return f"SELECT 1 FROM {schema_table} WHERE {predicates} LIMIT 1" # noqa: S608 +def build_hidden_grain_probe( + schema: str, + table: str, + *, + key_column: str, + cohort_column: str, + filter_columns: tuple[str, ...], +) -> str: + """Build the probe that asks a finer-grained MV which keys it withholds. + + The service publishes the same learners at organization and contract grain, + and a coarse row's exactly-additive columns are the sum of the contract rows + beneath it. So a contract row dropped by the floor is recoverable as + ``org_total - sum(the visible contract rows)``. Deciding whether to blank + the coarse columns needs one bit per key: does the finer grain withhold a + row here? + + That bit is all this returns. The comparison against the floor happens in + SQL and the projection is the grouping key alone, so the sub-floor cohort + count that motivates the whole exercise is never read into the process — + it cannot be logged, serialized, or leaked by a later change to this file. + + NULL cohorts count as withheld: ``suppress_small_cohorts`` drops a row whose + primary is NULL, and ``cohort < floor`` is NULL (not true) for those, so the + predicate has to name them explicitly or the probe would miss exactly the + rows the floor is most certain about. + + Identifiers are spliced under ``build_select``'s rules — every token is + ``validate_sql_identifier``'d, the floor and filter values are bound. + """ + if not filter_columns: + msg = "build_hidden_grain_probe needs at least one filter column" + raise ValueError(msg) + schema_table = f"{validate_sql_identifier(schema)}.{validate_sql_identifier(table)}" + key = validate_sql_identifier(key_column) + cohort = validate_sql_identifier(cohort_column) + predicates = " AND ".join(f"{validate_sql_identifier(name)} = %s" for name in filter_columns) + # Same justification as build_select: identifiers validated, values bound. + where = f"WHERE {predicates} AND ({cohort} < %s OR {cohort} IS NULL)" + return f"SELECT DISTINCT {key} FROM {schema_table} {where}" # noqa: S608 + + class SuppressibleModel(Protocol): """A row model that declares how the anonymization floor applies to it.""" @@ -146,13 +194,33 @@ async def fetch_and_suppress[ModelT: SQLModel]( params: tuple[Any, ...], model_cls: type[ModelT], floor: int, + *, + cross_grain: tuple[CrossGrainAdditives, Collection[Any]] | None = None, ) -> list[ModelT]: + """Query, suppress, construct — the one path rows take out of the database. + + ``cross_grain`` is for a coarse-grained endpoint whose rows are sums over a + finer grain that this service also publishes: pass the additive columns + together with the keys the finer grain withholds (see + ``build_hidden_grain_probe``) and those columns are blanked before any row + becomes a model. Both suppression passes run here rather than in the router + so that no endpoint can construct a response model from an unsuppressed row. + """ suppressible_cls = _require_cohort_policy(model_cls) rows = await starrocks_pool.fetch_all(query, params) suppressed = suppress_small_cohorts(rows, suppressible_cls.cohort_policy, floor) + if cross_grain is not None: + additives, hidden_keys = cross_grain + suppressed = suppress_cross_grain_additives(suppressed, additives, hidden_keys) return [model_cls(**row) for row in suppressed] +async def fetch_hidden_grain_keys(query: str, params: tuple[Any, ...]) -> frozenset[Any]: + """Run a ``build_hidden_grain_probe`` query, returning the withheld keys.""" + rows = await starrocks_pool.fetch_all(query, params) + return frozenset(next(iter(row.values())) for row in rows) + + async def fetch_visible_count(query: str, params: tuple[Any, ...]) -> int: """Run a ``build_count`` query and return its single number.""" rows = await starrocks_pool.fetch_all(query, params) diff --git a/src/ol_analytics_api/tenants/b2b_dashboard/models.py b/src/ol_analytics_api/tenants/b2b_dashboard/models.py index 1d7eb5d..8bf195c 100644 --- a/src/ol_analytics_api/tenants/b2b_dashboard/models.py +++ b/src/ol_analytics_api/tenants/b2b_dashboard/models.py @@ -6,10 +6,12 @@ schema is owned by dbt, not by this service. Every row model declares a ``cohort_policy`` (see core.anonymization): the -distinct-entity counts subject to the k-anonymity floor and the derived -values computed over them. The response layer nulls sub-floor secondary -counts and their derivatives, so any count/rate/average column that can be -suppressed is typed Optional even though the view never emits a NULL there. +distinct-entity counts subject to the k-anonymity floor, which of them sit +inside which, and the derived values computed over them. The response layer +nulls sub-floor secondary counts, counts whose complement within a containing +cohort is sub-floor, and the derivatives of both — so any count/rate/average +column that can be suppressed is typed Optional even though the view never +emits a NULL there. """ from __future__ import annotations @@ -64,6 +66,15 @@ class ContractUtilization(SQLModel): primary="seats_consumed", secondary=("active_learners", "learners_certified"), derived={"completion_rate_pct": ("learners_certified",)}, + # Both are counted from users the contract's enrollments already + # produced (`active_learners` filters those enrollments; + # `learners_certified` counts certificates on the same contract's + # course runs, which a learner can only hold by enrolling), so each is + # a subset of the seats consumed. + contained_in={ + "active_learners": "seats_consumed", + "learners_certified": "seats_consumed", + }, ) organization_key: str @@ -93,6 +104,18 @@ class EnrollmentCompletionFunnel(SQLModel): "active_rate_pct": ("active_learners",), "completion_rate_pct": ("certified_learners",), }, + # All three are counted off the enrollment row itself — grades and + # certificates join on `(user, course_run)` from the enrollment — so + # each names a subset of the enrolled learners. They are declared flat + # under the primary rather than chained (certified inside passing + # inside active): the view's SQL does not enforce those inner + # containments, and declaring one that does not hold fails closed and + # would suppress good data. + contained_in={ + "active_learners": "enrolled_learners", + "passing_learners": "enrolled_learners", + "certified_learners": "enrolled_learners", + }, ) organization_key: str @@ -167,6 +190,21 @@ class MonthlyEngagementTrend(SQLModel): "total_problems_attempted": ("problem_attempters",), "total_chatbot_interactions": ("chatbot_users",), }, + # Earning a certificate, watching a video, attempting a problem and + # using the chatbot each set `active_count`, so all four cohorts are + # subsets of the month's active learners and their complements are + # real: 42 active of whom 40 used the chatbot names the 2 who did not. + contained_in={ + "certified_learners": "monthly_active_learners", + "video_watchers": "monthly_active_learners", + "problem_attempters": "monthly_active_learners", + "chatbot_users": "monthly_active_learners", + }, + # Enrolling does not set `active_count`, so a learner who only enrolled + # is counted here and not in the primary. `monthly_active_learners - + # enrolling_learners` is therefore not a complement — it can even go + # negative — and reading it as one would suppress on noise. + uncontained=("enrolling_learners",), ) organization_key: str @@ -194,6 +232,13 @@ class ProgramFunnel(SQLModel): cohort_policy: ClassVar[CohortPolicy] = CohortPolicy( primary="enrolled_in_contract_courses", secondary=("enrolled_via_program", "program_course_completers"), + # Both are counted off the same enrollment rows as the primary — one + # filtered to the program pathway, one joined to certificates on + # `(user, course_run)` — so each is a subset of it. + contained_in={ + "enrolled_via_program": "enrolled_in_contract_courses", + "program_course_completers": "enrolled_in_contract_courses", + }, ) organization_key: str @@ -265,6 +310,23 @@ class ContentEngagementDepth(SQLModel): "total_chatbot_interactions": ("chatbot_users",), "chatbot_adoption_pct": ("chatbot_users",), }, + # Nested two deep, and the inner level is the one that bites: watching + # a video sets `active_count`, so the watchers sit inside the engaged + # learners, and 40 watchers of 42 engaged names the 2 engaged learners + # who never watched one. The outer pair is walked transitively, so the + # complement against total enrollment is checked too. + contained_in={ + "engaged_learners": "total_enrolled_learners", + "video_watchers": "engaged_learners", + "problem_attempters": "engaged_learners", + "chatbot_users": "engaged_learners", + }, + # `sum(certificate_count)` counts certificates, not learners: one + # learner can hold several, so it is not a subset of any cohort here + # and can exceed one. It stays floored as a count of itself (see + # above); a complement rule over it would be arithmetic on two + # different units. + uncontained=("certificates_earned",), ) organization_key: str @@ -293,6 +355,13 @@ class MitAdminContractHealth(SQLModel): primary="seats_consumed", secondary=("active_learners", "certified_learners"), derived={"completion_rate_pct": ("certified_learners",)}, + # Same shape as ContractUtilization: both are counted off the + # contract's own enrollment rows, so both are subsets of the seats + # consumed. + contained_in={ + "active_learners": "seats_consumed", + "certified_learners": "seats_consumed", + }, ) organization_key: str @@ -327,7 +396,14 @@ class ContractMonthlyEngagementTrend(MonthlyEngagementTrend): A learner active under two of an org's contracts appears in both rows, so these rows do not partition the org-level view's learner counts; summing ``monthly_active_learners`` across contracts can exceed the org's own - figure. Activity totals, being sums of events, do add up. + figure. Activity totals, being sums of events, do add up — which is what + makes a contract-month the floor withholds recoverable from the org + endpoint as ``org_total - sum(the visible contract months)``. The org + endpoint defends against that itself: it probes this view for the months + it withholds and blanks its own additive totals for them (see + ``routers.organizations._FinerGrain``). The learner counts are left alone, + because not adding up is exactly what stops them from being recovered by + subtraction. """ contract_pk: str @@ -344,10 +420,13 @@ class ContractContentEngagementDepth(ContentEngagementDepth): Unlike the trend view, these rows ARE a strict partition of the org-level view: a course run belongs to exactly one contract, so naming the contract labels a row rather than splitting it, and every count here equals its - org-level counterpart for the same course run. That equality is exactly - what makes a suppressed contract recoverable by subtraction once an org - holds more than one — the k-anonymity floor here is per-row and does not - defend against differencing across the two grains. + org-level counterpart for the same course run. + + That equality is why this pair needs no cross-grain guard, where the trend + pair does. Nothing is aggregated away going from contract grain to org + grain, so there is no remainder to subtract: a course run's org row and its + contract row hold the same numbers, the floor makes the same call on both, + and a caller reading one learns nothing the other withholds. """ contract_pk: str diff --git a/src/ol_analytics_api/tenants/b2b_dashboard/routers/organizations.py b/src/ol_analytics_api/tenants/b2b_dashboard/routers/organizations.py index b3cc645..4c3c1fb 100644 --- a/src/ol_analytics_api/tenants/b2b_dashboard/routers/organizations.py +++ b/src/ol_analytics_api/tenants/b2b_dashboard/routers/organizations.py @@ -23,10 +23,13 @@ from fastapi import APIRouter, Depends from sqlmodel import SQLModel +from ol_analytics_api.core.anonymization import CrossGrainAdditives from ol_analytics_api.core.db.query import ( build_count, + build_hidden_grain_probe, build_select, fetch_and_suppress, + fetch_hidden_grain_keys, fetch_visible_count, ) from ol_analytics_api.core.db.refresh_metadata import latest_refresh_timestamp @@ -59,6 +62,31 @@ _ORG_FILTER_COLUMN = "sso_organization_id" +@dataclass(frozen=True) +class _FinerGrain: + """The contract-grained sibling MV whose rows sum into this endpoint's. + + Only the engagement trend needs one. It is the single org endpoint that + aggregates *across* an org's contracts while the contract router publishes + the same months one contract at a time, so a contract-month the floor + withholds is recoverable as ``org_total - sum(the visible contract + months)``. The other four org endpoints carry a contract per row already, + and the content-engagement pair partitions by course run — a run belongs to + exactly one contract, so its org row and its contract row hold identical + counts and the floor makes the same call on both. Nothing is left over to + subtract in either case. + + ``additive_columns`` are the event sums, which do add up exactly across + contracts. The learner counts do not — a learner active under two contracts + is counted in both rows — so subtracting them bounds the withheld cohort + rather than revealing it, and they stay published. + """ + + mv: str + cohort_column: str + additive_columns: tuple[str, ...] + + @dataclass(frozen=True) class _OrgEndpoint: """One org-scoped MV endpoint, declared instead of hand-written. @@ -72,6 +100,7 @@ class _OrgEndpoint: mv: str model: type[SQLModel] order_by: tuple[str, ...] + finer_grain: _FinerGrain | None = None ENDPOINTS: list[_OrgEndpoint] = [ @@ -92,6 +121,17 @@ class _OrgEndpoint: "mv_b2b_monthly_engagement_trend", MonthlyEngagementTrend, ("activity_year_and_month",), + finer_grain=_FinerGrain( + "mv_b2b_contract_monthly_engagement_trend", + "monthly_active_learners", + ( + "new_enrollments", + "certificates_earned", + "total_videos_watched", + "total_problems_attempted", + "total_chatbot_interactions", + ), + ), ), _OrgEndpoint( "/program-funnel", @@ -124,20 +164,53 @@ def _register(spec: _OrgEndpoint) -> None: what lets a client say "showing 200 of 340" rather than truncating at the page cap with nothing to show for it — worth a second round trip on an endpoint whose data only changes when the MV refreshes, hours apart. + + An endpoint declaring a ``finer_grain`` pays for one more round trip, and + only that endpoint: a probe asking its contract-grained sibling which keys + it withholds, so the additive columns that would reconstruct those rows can + be blanked. The probe projects a grouping key and nothing else — see + ``build_hidden_grain_probe``. """ query = build_select( _SCHEMA, spec.mv, spec.model, filter_columns=(_ORG_FILTER_COLUMN,), order_by=spec.order_by ) count_query = build_count(_SCHEMA, spec.mv, spec.model, filter_columns=(_ORG_FILTER_COLUMN,)) + additives = probe_query = None + if spec.finer_grain is not None: + # The org grain's ordering column is also what lines an org row up with + # the contract rows summing into it, so the same tuple names the probe's + # key. Endpoints with a finer grain are single-keyed by construction. + (key_column,) = spec.order_by + additives = CrossGrainAdditives(key_column, spec.finer_grain.additive_columns) + probe_query = build_hidden_grain_probe( + _SCHEMA, + spec.finer_grain.mv, + key_column=key_column, + cohort_column=spec.finer_grain.cohort_column, + filter_columns=(_ORG_FILTER_COLUMN,), + ) async def endpoint( organization_id: str, page: Annotated[Pagination, Depends(pagination)] ) -> OrgAnalyticsResponse[SQLModel]: + cross_grain = None + if additives is not None and probe_query is not None: + # Probed across the whole org, not just this page: an org row on + # page 1 can be reconstructed from contract rows the caller reads + # in any page of the contract endpoint, so the page boundary is + # not a limit on what they can subtract. + cross_grain = ( + additives, + await fetch_hidden_grain_keys( + probe_query, (organization_id, settings.anonymization_floor) + ), + ) rows = await fetch_and_suppress( query, (organization_id, page.limit, page.offset), spec.model, settings.anonymization_floor, + cross_grain=cross_grain, ) return OrgAnalyticsResponse( organization_id=organization_id, diff --git a/tests/test_anonymization.py b/tests/test_anonymization.py index 68bffc9..2210a78 100644 --- a/tests/test_anonymization.py +++ b/tests/test_anonymization.py @@ -1,6 +1,11 @@ import pytest -from ol_analytics_api.core.anonymization import CohortPolicy, suppress_small_cohorts +from ol_analytics_api.core.anonymization import ( + CohortPolicy, + CrossGrainAdditives, + suppress_cross_grain_additives, + suppress_small_cohorts, +) def test_row_below_primary_floor_is_dropped(): @@ -40,6 +45,11 @@ def test_secondary_count_below_floor_is_nulled_but_row_kept(): policy = CohortPolicy( primary="total_enrolled_learners", secondary=("engaged_learners", "chatbot_users", "certificates_earned"), + contained_in={ + "engaged_learners": "total_enrolled_learners", + "chatbot_users": "engaged_learners", + }, + uncontained=("certificates_earned",), ) (row,) = suppress_small_cohorts(rows, policy, floor=5) assert row["engaged_learners"] == 40 # above floor, retained @@ -50,14 +60,22 @@ def test_secondary_count_below_floor_is_nulled_but_row_kept(): def test_secondary_count_of_zero_is_kept(): # 0 discloses no individual — it stays visible as a real "nobody did this". rows = [{"total_enrolled_learners": 50, "certificates_earned": 0}] - policy = CohortPolicy(primary="total_enrolled_learners", secondary=("certificates_earned",)) + policy = CohortPolicy( + primary="total_enrolled_learners", + secondary=("certificates_earned",), + uncontained=("certificates_earned",), + ) (row,) = suppress_small_cohorts(rows, policy, floor=5) assert row["certificates_earned"] == 0 def test_null_secondary_count_is_treated_as_disclosive(): rows = [{"total_enrolled_learners": 50, "certificates_earned": None}] - policy = CohortPolicy(primary="total_enrolled_learners", secondary=("certificates_earned",)) + policy = CohortPolicy( + primary="total_enrolled_learners", + secondary=("certificates_earned",), + uncontained=("certificates_earned",), + ) (row,) = suppress_small_cohorts(rows, policy, floor=5) assert row["certificates_earned"] is None @@ -82,6 +100,7 @@ def test_derived_values_nulled_when_their_cohort_is_suppressed(): "total_videos_watched": ("engaged_learners",), "avg_videos_per_engaged_learner": ("engaged_learners",), }, + contained_in={"engaged_learners": "total_enrolled_learners"}, ) (row,) = suppress_small_cohorts(rows, policy, floor=5) assert row["engaged_learners"] is None @@ -102,6 +121,7 @@ def test_derived_values_retained_when_cohort_above_floor(): primary="total_enrolled_learners", secondary=("engaged_learners",), derived={"avg_videos_per_engaged_learner": ("engaged_learners",)}, + contained_in={"engaged_learners": "total_enrolled_learners"}, ) (row,) = suppress_small_cohorts(rows, policy, floor=5) assert row["engaged_learners"] == 30 @@ -110,7 +130,11 @@ def test_derived_values_retained_when_cohort_above_floor(): def test_input_rows_are_not_mutated(): rows = [{"total_enrolled_learners": 50, "certificates_earned": 1}] - policy = CohortPolicy(primary="total_enrolled_learners", secondary=("certificates_earned",)) + policy = CohortPolicy( + primary="total_enrolled_learners", + secondary=("certificates_earned",), + uncontained=("certificates_earned",), + ) suppress_small_cohorts(rows, policy, floor=5) assert rows[0]["certificates_earned"] == 1 @@ -124,6 +148,7 @@ def test_derived_referencing_unknown_cohort_raises_at_construction(): primary="total_enrolled_learners", secondary=("engaged_learners",), derived={"avg_videos_per_engaged_learner": ("typo_learners",)}, + contained_in={"engaged_learners": "total_enrolled_learners"}, ) @@ -147,3 +172,209 @@ def test_derived_mapping_is_not_mutable_after_construction(): with pytest.raises(TypeError): policy.derived["sneaky"] = ("total_enrolled_learners",) + + +def _depth_policy(**overrides): + """The nesting the content-engagement view actually declares: watchers + inside the engaged learners, engaged inside everyone enrolled.""" + kwargs = { + "primary": "total_enrolled_learners", + "secondary": ("engaged_learners", "video_watchers"), + "derived": { + "engagement_rate_pct": ("engaged_learners",), + "total_videos_watched": ("video_watchers",), + }, + "contained_in": { + "engaged_learners": "total_enrolled_learners", + "video_watchers": "engaged_learners", + }, + } + return CohortPolicy(**(kwargs | overrides)) + + +def test_near_total_secondary_is_nulled_because_its_complement_is_disclosive(): + # 42 engaged of whom 40 watched a video names the 2 who did not just as + # precisely as a count of 2 watchers would name those 2. + rows = [ + { + "total_enrolled_learners": 50, + "engaged_learners": 42, + "video_watchers": 40, + "total_videos_watched": 900, + } + ] + (row,) = suppress_small_cohorts(rows, _depth_policy(), floor=5) + assert row["video_watchers"] is None + assert row["total_videos_watched"] is None + assert row["engaged_learners"] == 42 # its own complement is 8, above floor + + +def test_complement_of_zero_is_kept(): + # Everybody engaged singles out nobody — the empty complement is not a + # cohort of size < floor, it is no cohort at all. + rows = [{"total_enrolled_learners": 50, "engaged_learners": 50, "video_watchers": 10}] + (row,) = suppress_small_cohorts(rows, _depth_policy(), floor=5) + assert row["engaged_learners"] == 50 + + +def test_complement_at_the_floor_is_kept(): + rows = [{"total_enrolled_learners": 50, "engaged_learners": 45, "video_watchers": 10}] + (row,) = suppress_small_cohorts(rows, _depth_policy(), floor=5) + assert row["engaged_learners"] == 45 + + +def test_negative_complement_fails_closed(): + # A subset larger than the cohort it is declared inside means the + # declaration is wrong for this row. The complement rule's whole job is to + # bound what can be back-computed, so an assumption it cannot trust + # suppresses rather than waves through. + rows = [{"total_enrolled_learners": 50, "engaged_learners": 30, "video_watchers": 31}] + (row,) = suppress_small_cohorts(rows, _depth_policy(), floor=5) + assert row["video_watchers"] is None + + +def test_complement_is_checked_transitively_when_the_inner_cohort_is_suppressed(): + # engaged_learners goes first (complement 2 within the enrolled). That + # hides the container of the (video_watchers, engaged_learners) pair, so + # the inner complement is no longer computable by a caller — but + # video_watchers is still visible next to total_enrolled_learners, whose + # complement is 3, and the transitive pair catches it. + rows = [{"total_enrolled_learners": 42, "engaged_learners": 40, "video_watchers": 39}] + (row,) = suppress_small_cohorts(rows, _depth_policy(), floor=5) + assert row["engaged_learners"] is None + assert row["video_watchers"] is None + + +def test_uncontained_cohort_is_exempt_from_the_complement_rule(): + # Enrolling does not make a learner active, so enrolling_learners is not a + # subset of the primary and the difference is not a complement. Subtracting + # anyway would suppress a perfectly publishable count on noise. + policy = CohortPolicy( + primary="monthly_active_learners", + secondary=("enrolling_learners",), + uncontained=("enrolling_learners",), + ) + rows = [{"monthly_active_learners": 42, "enrolling_learners": 40}] + (row,) = suppress_small_cohorts(rows, policy, floor=5) + assert row["enrolling_learners"] == 40 + + +def test_unclassified_secondary_raises_at_construction(): + # The leak this whole rule exists to close is silent by nature, so a + # cohort nobody thought about must not default to unprotected. + with pytest.raises(ValueError, match="classified neither"): + CohortPolicy(primary="total_enrolled_learners", secondary=("engaged_learners",)) + + +def test_cohort_in_both_contained_in_and_uncontained_raises(): + with pytest.raises(ValueError, match="both contained_in and uncontained"): + CohortPolicy( + primary="total_enrolled_learners", + secondary=("engaged_learners",), + contained_in={"engaged_learners": "total_enrolled_learners"}, + uncontained=("engaged_learners",), + ) + + +def test_containment_naming_an_unknown_container_raises(): + with pytest.raises(ValueError, match="unknown cohort"): + CohortPolicy( + primary="total_enrolled_learners", + secondary=("engaged_learners",), + contained_in={"engaged_learners": "typo_learners"}, + ) + + +def test_containment_of_a_non_secondary_column_raises(): + with pytest.raises(ValueError, match="not a secondary cohort"): + CohortPolicy( + primary="total_enrolled_learners", + secondary=("engaged_learners",), + contained_in={ + "engaged_learners": "total_enrolled_learners", + "some_rate_pct": "total_enrolled_learners", + }, + ) + + +def test_uncontained_naming_a_non_secondary_column_raises(): + with pytest.raises(ValueError, match="not a secondary cohort"): + CohortPolicy( + primary="total_enrolled_learners", + secondary=("engaged_learners",), + contained_in={"engaged_learners": "total_enrolled_learners"}, + uncontained=("some_rate_pct",), + ) + + +def test_containment_cycle_raises(): + # Left to run, a cycle would spin the ancestor walk forever; it also can't + # describe anything real, since strict containment is a partial order. + with pytest.raises(ValueError, match="cycle"): + CohortPolicy( + primary="total_enrolled_learners", + secondary=("a_learners", "b_learners"), + contained_in={"a_learners": "b_learners", "b_learners": "a_learners"}, + ) + + +def test_containment_mapping_is_not_mutable_after_construction(): + source = {"engaged_learners": "total_enrolled_learners"} + policy = CohortPolicy( + primary="total_enrolled_learners", + secondary=("engaged_learners",), + contained_in=source, + ) + + source["sneaky"] = "total_enrolled_learners" + assert "sneaky" not in policy.contained_in + + with pytest.raises(TypeError): + policy.contained_in["sneaky"] = "total_enrolled_learners" + + +_ADDITIVES = CrossGrainAdditives( + key_column="activity_year_and_month", + columns=("new_enrollments", "total_videos_watched"), +) + + +def test_cross_grain_additives_are_blanked_for_a_withheld_key(): + # The caller holds this org total and every contract row but one, so the + # difference is the withheld contract's own activity. + rows = [ + { + "activity_year_and_month": "2026-07", + "monthly_active_learners": 40, + "new_enrollments": 12, + "total_videos_watched": 500, + } + ] + (row,) = suppress_cross_grain_additives(rows, _ADDITIVES, {"2026-07"}) + assert row["new_enrollments"] is None + assert row["total_videos_watched"] is None + # Learner counts are not additive across contracts (one learner active + # under two is counted in both), so subtracting them bounds rather than + # reveals, and they stay published. + assert row["monthly_active_learners"] == 40 + + +def test_cross_grain_leaves_other_keys_alone(): + rows = [ + {"activity_year_and_month": "2026-06", "new_enrollments": 9}, + {"activity_year_and_month": "2026-07", "new_enrollments": 12}, + ] + june, july = suppress_cross_grain_additives(rows, _ADDITIVES, {"2026-07"}) + assert june["new_enrollments"] == 9 + assert july["new_enrollments"] is None + + +def test_cross_grain_with_nothing_withheld_changes_nothing(): + rows = [{"activity_year_and_month": "2026-07", "new_enrollments": 12}] + assert suppress_cross_grain_additives(rows, _ADDITIVES, frozenset()) == rows + + +def test_cross_grain_does_not_mutate_input_rows(): + rows = [{"activity_year_and_month": "2026-07", "new_enrollments": 12}] + suppress_cross_grain_additives(rows, _ADDITIVES, {"2026-07"}) + assert rows[0]["new_enrollments"] == 12 diff --git a/tests/test_column_contract.py b/tests/test_column_contract.py index df21a11..2e64e00 100644 --- a/tests/test_column_contract.py +++ b/tests/test_column_contract.py @@ -115,6 +115,39 @@ def test_cohort_policy_columns_are_real_model_fields(case): for derived, cohorts in policy.derived.items(): assert derived in fields, f"{case.label}: derived column {derived!r} not in model fields" assert set(cohorts) <= fields, f"{case.label}: {derived!r} references a missing cohort" + # Same argument for the containment declarations: a typo'd container is a + # complement that never gets checked. + for subset, container in policy.contained_in.items(): + assert subset in fields, f"{case.label}: contained cohort {subset!r} not a model field" + assert container in fields, f"{case.label}: container {container!r} not a model field" + assert set(policy.uncontained) <= fields, f"{case.label}: uncontained names a missing column" + + +def test_finer_grain_declarations_match_the_contract_endpoint_they_name(): + """The cross-grain guard is only sound if the coarse endpoint and the + contract endpoint it probes really are the same data at two grains.""" + contract_models = {spec.mv: spec.model for spec in contracts.ENDPOINTS} + + for spec in organizations.ENDPOINTS: + if spec.finer_grain is None: + continue + finer_model = contract_models[spec.finer_grain.mv] + coarse_fields = set(spec.model.model_fields) + # The probe filters on the key and the coarse rows are matched by it, + # so both grains have to carry it. + (key_column,) = spec.order_by + assert key_column in coarse_fields + assert key_column in set(finer_model.model_fields) + # The probe compares against the finer grain's own row gate, so it + # withholds exactly the keys suppress_small_cohorts drops there. + assert spec.finer_grain.cohort_column == finer_model.cohort_policy.primary + # Only event sums add up exactly across contracts. Blanking a cohort + # count here would over-suppress; leaving out a sum would leave the + # subtraction open. + policy = spec.model.cohort_policy + additives = set(spec.finer_grain.additive_columns) + assert additives <= set(policy.derived), f"{spec.mv}: additive column is not a derived sum" + assert not additives & {policy.primary, *policy.secondary} @_cases diff --git a/tests/test_endpoints.py b/tests/test_endpoints.py index 2ecb046..5acd822 100644 --- a/tests/test_endpoints.py +++ b/tests/test_endpoints.py @@ -65,16 +65,25 @@ def _is_count_query(query): return "COUNT(*)" in query -def _fake_fetch_all(data_rows, total_count=0): +def _is_hidden_grain_probe(query): + # build_hidden_grain_probe is the only thing in the service that projects + # DISTINCT — it asks the contract-grained MV which keys it withholds. + return "SELECT DISTINCT " in query + + +def _fake_fetch_all(data_rows, total_count=0, hidden_keys=()): """A fetch_all stub that answers the information_schema as_of probe with - _AS_OF, the envelope's total-count query with ``total_count``, and every - other query with the given MV rows.""" + _AS_OF, the envelope's total-count query with ``total_count``, the + hidden-grain probe with ``hidden_keys``, and every other query with the + given MV rows.""" async def fetch_all(query, *_args): if "information_schema" in query: return [{"as_of": _AS_OF}] if _is_count_query(query): return [{"total_count": total_count}] + if _is_hidden_grain_probe(query): + return [{"grain_key": key} for key in hidden_keys] return list(data_rows) return fetch_all @@ -82,14 +91,17 @@ async def fetch_all(query, *_args): def _capture_page_query(captured, rows=(), total_count=0): """A fetch_all stub that records the *paged* SELECT specifically. The as_of - probe and the total-count query go through the same pool, so a stub that - captured every query would report whichever happened to run last.""" + probe, the total-count query and the hidden-grain probe go through the same + pool, so a stub that captured every query would report whichever happened + to run last.""" async def fetch_all(query, params): if "information_schema" in query: return [{"as_of": _AS_OF}] if _is_count_query(query): return [{"total_count": total_count}] + if _is_hidden_grain_probe(query): + return [] captured["query"] = query captured["params"] = params return list(rows) @@ -376,6 +388,114 @@ async def test_monthly_trend_floors_event_counts_through_their_learner_cohorts(a assert data["total_chatbot_interactions"] == 60 +def _trend_row(month="2026-07"): + """An org-grained engagement-trend row that clears every floor on its own, + so anything suppressed in these tests came from the cross-grain guard.""" + return { + "organization_key": "org-a", + "organization_name": "Org A", + "activity_year_and_month": month, + "monthly_active_learners": 40, + "new_enrollments": 12, + "enrolling_learners": 9, + "certificates_earned": 30, + "certified_learners": 22, + "total_videos_watched": 500, + "video_watchers": 18, + "total_problems_attempted": 7, + "problem_attempters": 6, + "total_chatbot_interactions": 60, + "chatbot_users": 15, + } + + +async def _get_trend(app, fetch_all): + with ( + patch("ol_analytics_api.core.db.client.starrocks_pool.fetch_all", new=fetch_all), + patch( + "ol_analytics_api.tenants.b2b_dashboard.auth.mitxonline_client.is_org_manager", + new=AsyncMock(return_value=True), + ), + ): + async with _client(app) as client: + return await client.get( + f"/api/v1/analytics/organizations/{ORG_A_ID}/engagement-trend", + headers={"X-Userinfo": _manager_header(ORG_A_ID)}, + ) + + +async def test_org_trend_blanks_additive_columns_a_withheld_contract_row_would_reveal(app): + # The org row survives every per-row floor. What it cannot survive is the + # caller also reading the contract endpoint for the same month: this org's + # other contracts are published there, so `org_total - sum(them)` is the + # activity of the contract the floor withheld. + response = await _get_trend(app, _fake_fetch_all([_trend_row()], hidden_keys=["2026-07"])) + + assert response.status_code == 200 + (data,) = response.json()["data"] + assert data["new_enrollments"] is None + assert data["certificates_earned"] is None + assert data["total_videos_watched"] is None + assert data["total_problems_attempted"] is None + assert data["total_chatbot_interactions"] is None + # Learner counts don't sum across contracts — a learner active under two is + # counted in both rows — so subtracting them bounds the withheld cohort + # rather than revealing it, and over-suppressing them would cost the + # dashboard its headline number for nothing. + assert data["monthly_active_learners"] == 40 + assert data["certified_learners"] == 22 + assert data["video_watchers"] == 18 + + +async def test_org_trend_untouched_when_the_contract_grain_withholds_nothing(app): + response = await _get_trend(app, _fake_fetch_all([_trend_row()], hidden_keys=[])) + + assert response.status_code == 200 + (data,) = response.json()["data"] + assert data["new_enrollments"] == 12 + assert data["total_videos_watched"] == 500 + assert data["certificates_earned"] == 30 + + +async def test_org_trend_blanks_only_the_months_the_contract_grain_withholds(app): + rows = [_trend_row("2026-06"), _trend_row("2026-07")] + response = await _get_trend(app, _fake_fetch_all(rows, hidden_keys=["2026-07"])) + + june, july = response.json()["data"] + assert june["new_enrollments"] == 12 + assert july["new_enrollments"] is None + + +async def test_org_endpoints_without_a_finer_grain_issue_no_probe(app): + # Only the trend endpoint aggregates across an org's contracts. Probing on + # the other four would be a round trip per request buying nothing. + queries = [] + + async def fetch_all(query, *_args): + queries.append(query) + if "information_schema" in query: + return [{"as_of": _AS_OF}] + if _is_count_query(query): + return [{"total_count": 0}] + return [] + + with ( + patch("ol_analytics_api.core.db.client.starrocks_pool.fetch_all", new=fetch_all), + patch( + "ol_analytics_api.tenants.b2b_dashboard.auth.mitxonline_client.is_org_manager", + new=AsyncMock(return_value=True), + ), + ): + async with _client(app) as client: + response = await client.get( + f"/api/v1/analytics/organizations/{ORG_A_ID}/content-engagement", + headers={"X-Userinfo": _manager_header(ORG_A_ID)}, + ) + + assert response.status_code == 200 + assert not any(_is_hidden_grain_probe(query) for query in queries) + + async def test_org_endpoint_403_for_member_who_is_not_a_manager(app): # A member (org present in the claim) whose MITx Online manager check # comes back False is rejected — membership alone isn't enough. diff --git a/tests/test_query_chokepoint.py b/tests/test_query_chokepoint.py index 703da45..ed9b4ac 100644 --- a/tests/test_query_chokepoint.py +++ b/tests/test_query_chokepoint.py @@ -9,7 +9,13 @@ from sqlmodel import SQLModel from ol_analytics_api.core.anonymization import CohortPolicy -from ol_analytics_api.core.db.query import build_count, fetch_and_suppress, fetch_visible_count +from ol_analytics_api.core.db.query import ( + build_count, + build_hidden_grain_probe, + fetch_and_suppress, + fetch_hidden_grain_keys, + fetch_visible_count, +) class _PolicylessRow(SQLModel): @@ -100,3 +106,47 @@ async def test_count_alias_matches_the_column_fetch_visible_count_reads(): new=AsyncMock(return_value=[{alias: 7}]), ): assert await fetch_visible_count(query, ()) == 7 + + +def test_build_hidden_grain_probe_projects_only_the_key(): + # The sub-floor cohort count that motivates the probe is compared inside + # SQL and never read into the process, so it cannot be logged or returned + # by a later change here. NULL cohorts are named explicitly: `cohort < %s` + # is NULL for them, and those are rows suppression drops outright. + query = build_hidden_grain_probe( + "b2b_analytics", + "mv_b2b_contract_monthly_engagement_trend", + key_column="activity_year_and_month", + cohort_column="monthly_active_learners", + filter_columns=("sso_organization_id",), + ) + + assert query == ( + "SELECT DISTINCT activity_year_and_month " + "FROM b2b_analytics.mv_b2b_contract_monthly_engagement_trend " + "WHERE sso_organization_id = %s " + "AND (monthly_active_learners < %s OR monthly_active_learners IS NULL)" + ) + + +def test_build_hidden_grain_probe_requires_a_filter_column(): + # Unfiltered, the probe would ask which keys *any* org withholds and blank + # this org's totals on another org's data. + with pytest.raises(ValueError, match="at least one filter column"): + build_hidden_grain_probe( + "b2b_analytics", + "mv_thing", + key_column="activity_year_and_month", + cohort_column="monthly_active_learners", + filter_columns=(), + ) + + +async def test_fetch_hidden_grain_keys_returns_the_projected_values(): + with patch( + "ol_analytics_api.core.db.client.starrocks_pool.fetch_all", + new=AsyncMock(return_value=[{"activity_year_and_month": "2026-06"}]), + ): + assert await fetch_hidden_grain_keys("SELECT DISTINCT x FROM y", ()) == frozenset( + {"2026-06"} + ) From 8df184833e77570575c0c7f9149adf9fcb4560ba Mon Sep 17 00:00:00 2001 From: Tobias Macey Date: Fri, 21 Aug 2026 08:32:24 -0400 Subject: [PATCH 2/4] fix(anonymization): guard on what the finer grain hides, not just what it drops The cross-grain guard asked the contract-grained MV a cheaper question than the one that matters: which rows does the floor DROP. Every column it protects is a derived column at that grain, and a contract row can clear its own row gate and still publish NULL for one, because the cohort that column is attributable to is sub-floor. A contract month with 30 active learners of whom 2 used the chatbot publishes its row and withholds its chatbot total; the probe saw nothing hidden, left the org total alone, and org minus the sibling contract handed the withheld number back. Reproduced against the real cohort policy before fixing: 517 - 500 = 17 interactions from a cohort of 2. Re-encoding the rule in SQL was the wrong shape to begin with. The complement rule is a fixpoint over transitive containment pairs, so a second implementation of it drifts, and it drifts toward publishing. The guard now scans the finer grain and runs those rows through suppress_small_cohorts itself, keeping only which additive columns come back NULL per key. One implementation of the governance decision, and it catches all three ways a column goes missing: the row dropped, its cohort sub-floor, or its cohort taken by the complement rule. Blanking is now per column rather than per key, so a month where only the chatbot total is withheld downstream keeps its video and problem totals. The scan is bounded and fails closed: a result that reaches the row cap cannot prove it saw every contributing row, and publishing a total the guard could not check is the failure the guard is for. Also from review: the additive columns were hand-listed with nothing checking the list was complete, so a sixth event sum added to the MV would have left its subtraction open with a green suite. Additive and non-additive must now partition the coarse model's derived columns, enforced at import. And complement_pairs() was documented as computed once per response while being called once per row; it is now hoisted to match. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01RVrNyjeL7gSVY61gSUavKf --- src/ol_analytics_api/core/anonymization.py | 116 ++++++++++++---- src/ol_analytics_api/core/db/query.py | 81 ++++++----- .../b2b_dashboard/routers/organizations.py | 128 +++++++++++++----- tests/test_anonymization.py | 91 ++++++++++++- tests/test_column_contract.py | 40 +++++- tests/test_endpoints.py | 125 ++++++++++++----- tests/test_query_chokepoint.py | 58 ++++---- 7 files changed, 477 insertions(+), 162 deletions(-) diff --git a/src/ol_analytics_api/core/anonymization.py b/src/ol_analytics_api/core/anonymization.py index 8186fe3..6de674d 100644 --- a/src/ol_analytics_api/core/anonymization.py +++ b/src/ol_analytics_api/core/anonymization.py @@ -33,10 +33,17 @@ A second disclosure channel runs *across* rows rather than within one. This service publishes the same learners at two grains — organization and contract — and a coarse row's exactly-additive columns are the sum of the finer rows -beneath it, so a finer row withheld by the floor is recoverable as -``coarse_total - sum(the visible finer rows)``. Rows alone cannot see that; -``suppress_cross_grain_additives`` takes the finer grain's withheld keys as -input and blanks the coarse columns that would reconstruct them. +beneath it, so anything the floor withholds at the finer grain is recoverable +as ``coarse_total - sum(what the finer rows do publish)``. Note "withholds", +not "drops": a finer row can clear its own row gate and still publish NULL for +one additive column, because the cohort that column is attributable to is +sub-floor. Both cases leave the same hole in the sum. + +Rows alone cannot see any of that. ``hidden_additive_columns`` runs the finer +grain through the suppression above — the same function, not a cheaper +approximation of it — and reports which additive columns come back NULL per +key; ``suppress_cross_grain_additives`` blanks exactly those at the coarse +grain. """ from __future__ import annotations @@ -175,8 +182,7 @@ def _ancestors(self, subset: str) -> Iterator[str]: def complement_pairs(self) -> tuple[tuple[str, str], ...]: """Every ``(subset, container)`` pair whose complement must be checked, - including transitive ones. Computed once per response rather than per - row — a policy is fixed ClassVar state on the row model.""" + including transitive ones.""" return tuple( (subset, container) for subset in self.contained_in @@ -204,6 +210,55 @@ class CrossGrainAdditives: columns: tuple[str, ...] +def hidden_additive_columns( + finer_rows: list[dict[str, Any]], + policy: CohortPolicy, + floor: int, + *, + key_column: str, + additive_columns: tuple[str, ...], +) -> dict[Any, frozenset[str]]: + """Per key, which additive columns the finer grain does not publish in full. + + This runs the finer rows through ``suppress_small_cohorts`` — the same + function that suppresses them on their own endpoint — rather than asking + the database a cheaper question about them. Two weaker tests were tried + and are wrong: + + - "is any finer row dropped?" misses the larger case by far. Every additive + column is a ``derived`` column at the finer grain, so a finer row can + clear the row gate and still publish NULL for one of them because the + cohort *that* column is attributable to is sub-floor. A contract month + with 30 active learners of whom 2 used the chatbot publishes its row and + withholds its chatbot total, and the coarse total minus its siblings + hands that withheld number straight back. + - re-deriving the rule in SQL keeps two copies of a governance decision in + step by hand. The complement rule alone is a fixpoint over transitive + containment pairs; a second implementation of it would drift, and the + direction it drifts in is silently publishing. + + A column counts as hidden when it is NULL after suppression, whether the + floor nulled it or the view never had a value. The caller cannot tell those + apart either, so both leave a hole in the sum that the coarse row would + fill in. + """ + hidden: dict[Any, set[str]] = {} + for row in finer_rows: + # One row at a time: suppress_small_cohorts returns only survivors, with + # no back-pointer to the row each came from, and several finer rows share + # a key (that is what makes the coarse row a sum). It is per-row anyway, + # so splitting the call changes nothing but keeps the correspondence. + kept = suppress_small_cohorts([row], policy, floor) + columns = ( + set(additive_columns) # dropped whole: every additive column is gone + if not kept + else {column for column in additive_columns if kept[0].get(column) is None} + ) + if columns: + hidden.setdefault(row.get(key_column), set()).update(columns) + return {key: frozenset(columns) for key, columns in hidden.items()} + + def _is_disclosive(value: int | None, floor: int) -> bool: # NULL (unknown — e.g. a LEFT JOIN miss in the source MV) is treated as # disclosive: we cannot prove the cohort was large enough, so suppress it. @@ -232,10 +287,14 @@ def _complement_is_disclosive(subset: int, container: int, floor: int) -> bool: return complement != 0 and complement < floor -def _suppressed_cohorts(row: Mapping[str, Any], policy: CohortPolicy, floor: int) -> set[str]: +def _suppressed_cohorts( + row: Mapping[str, Any], + policy: CohortPolicy, + floor: int, + pairs: tuple[tuple[str, str], ...], +) -> set[str]: """The secondary cohorts of one row that must not be published.""" suppressed = {column for column in policy.secondary if _is_disclosive(row.get(column), floor)} - pairs = policy.complement_pairs() # Suppressing a cohort can hide the container of another pair, which # removes that pair from play rather than adding one, so this settles in # at most one pass per level of nesting. Cycles are rejected at @@ -267,13 +326,17 @@ def suppress_small_cohorts( nulled, and rows below the primary floor dropped. Input rows are not mutated.""" kept: list[dict[str, Any]] = [] + # Walked once for the whole response, not once per row: a policy is fixed + # ClassVar state on the row model, so the pairs it yields are the same for + # every row in it. + pairs = policy.complement_pairs() for row in rows: # `.get(field) or 0` folds both a missing key and a NULL primary to 0, # so a too-small *or* unknown headline cohort withholds the whole row. if (row.get(policy.primary) or 0) < floor: continue redacted = dict(row) - suppressed = _suppressed_cohorts(redacted, policy, floor) + suppressed = _suppressed_cohorts(redacted, policy, floor, pairs) for column in suppressed: redacted[column] = None for column, cohorts in policy.derived.items(): @@ -286,28 +349,31 @@ def suppress_small_cohorts( def suppress_cross_grain_additives( rows: list[dict[str, Any]], additives: CrossGrainAdditives, - hidden_keys: Collection[Any], + hidden_by_key: Mapping[Any, Collection[str]], ) -> list[dict[str, Any]]: - """Blank the coarse columns that would reconstruct a withheld finer row. - - ``hidden_keys`` is the set of ``additives.key_column`` values for which the - finer grain withheld at least one row. For those keys the caller holds a - coarse total and every finer row but one (or a few), so the difference is - the withheld row's value — a quantity attributable to a cohort the floor - already judged too small to publish. Withholding the coarse total is what - breaks the subtraction; the finer rows themselves stay as they are. - - One withheld finer row is enough to trigger this. Several are not safer: - the difference is then their sum, which can still be a handful of entities. + """Blank the coarse columns that would reconstruct what the finer grain hides. + + ``hidden_by_key`` maps a ``key_column`` value to the additive columns the + finer grain does not publish in full for it — what + ``hidden_additive_columns`` returns. For those the caller holds a coarse + total and every finer contribution but one (or a few), so the difference is + the hidden quantity, attributable to a cohort the floor already judged too + small to publish. Withholding the coarse total is what breaks the + subtraction; the finer rows themselves stay as they are. + + Blanking is per column, not per key: a month where only the chatbot total is + withheld downstream keeps its video and problem totals, which nothing can be + subtracted out of. One hidden finer contribution is enough to blank the + column it belongs to — several are not safer, since the difference is then + their sum, which can still be a handful of entities. Input rows are not mutated. """ - if not hidden_keys: + if not hidden_by_key: return rows - hidden = set(hidden_keys) return [ - {column: (None if column in additives.columns else value) for column, value in row.items()} - if row.get(additives.key_column) in hidden + {column: (None if column in blanked else value) for column, value in row.items()} + if (blanked := hidden_by_key.get(row.get(additives.key_column))) else row for row in rows ] diff --git a/src/ol_analytics_api/core/db/query.py b/src/ol_analytics_api/core/db/query.py index fdd76dd..fc47b4e 100644 --- a/src/ol_analytics_api/core/db/query.py +++ b/src/ol_analytics_api/core/db/query.py @@ -17,7 +17,7 @@ from __future__ import annotations -from collections.abc import Collection +from collections.abc import Collection, Mapping from typing import Any, ClassVar, Protocol, cast from sqlmodel import SQLModel @@ -128,46 +128,38 @@ def build_existence_check(schema: str, table: str, filter_columns: tuple[str, .. return f"SELECT 1 FROM {schema_table} WHERE {predicates} LIMIT 1" # noqa: S608 -def build_hidden_grain_probe( +def build_grain_scan( schema: str, table: str, + model_cls: type[SQLModel], *, - key_column: str, - cohort_column: str, filter_columns: tuple[str, ...], ) -> str: - """Build the probe that asks a finer-grained MV which keys it withholds. - - The service publishes the same learners at organization and contract grain, - and a coarse row's exactly-additive columns are the sum of the contract rows - beneath it. So a contract row dropped by the floor is recoverable as - ``org_total - sum(the visible contract rows)``. Deciding whether to blank - the coarse columns needs one bit per key: does the finer grain withhold a - row here? + """Build the read of a finer-grained MV that a coarse endpoint's cross-grain + guard reasons over. - That bit is all this returns. The comparison against the floor happens in - SQL and the projection is the grouping key alone, so the sub-floor cohort - count that motivates the whole exercise is never read into the process — - it cannot be logged, serialized, or leaked by a later change to this file. + Not ``build_select``: this read never reaches a caller. Its rows are fed to + ``hidden_additive_columns``, which suppresses them exactly as the finer + grain's own endpoint would and reports which additive columns come back + NULL. So it projects the finer model's full column set (the cohort policy + needs every cohort it names) and takes no offset — the guard has to see all + of them at once, because a coarse row can be reconstructed from finer rows + the caller reads on any page of the finer endpoint. - NULL cohorts count as withheld: ``suppress_small_cohorts`` drops a row whose - primary is NULL, and ``cohort < floor`` is NULL (not true) for those, so the - predicate has to name them explicitly or the probe would miss exactly the - rows the floor is most certain about. + The bound ``LIMIT`` is a backstop, not paging. The caller must treat a full + result as a truncated one and fail closed; see ``routers.organizations``. Identifiers are spliced under ``build_select``'s rules — every token is - ``validate_sql_identifier``'d, the floor and filter values are bound. + ``validate_sql_identifier``'d, the filter values and the limit are bound. """ if not filter_columns: - msg = "build_hidden_grain_probe needs at least one filter column" + msg = "build_grain_scan needs at least one filter column" raise ValueError(msg) + columns = ", ".join(validate_sql_identifier(name) for name in model_cls.model_fields) schema_table = f"{validate_sql_identifier(schema)}.{validate_sql_identifier(table)}" - key = validate_sql_identifier(key_column) - cohort = validate_sql_identifier(cohort_column) predicates = " AND ".join(f"{validate_sql_identifier(name)} = %s" for name in filter_columns) # Same justification as build_select: identifiers validated, values bound. - where = f"WHERE {predicates} AND ({cohort} < %s OR {cohort} IS NULL)" - return f"SELECT DISTINCT {key} FROM {schema_table} {where}" # noqa: S608 + return f"SELECT {columns} FROM {schema_table} WHERE {predicates} LIMIT %s" # noqa: S608 class SuppressibleModel(Protocol): @@ -176,6 +168,16 @@ class SuppressibleModel(Protocol): cohort_policy: ClassVar[CohortPolicy] +def cohort_policy_of(model_cls: type[SQLModel]) -> CohortPolicy: + """The policy a model declares, through the same gate the chokepoint uses. + + For callers that need to reason about a model's cohorts without reading its + rows — the cross-grain guard suppresses a finer grain's rows to learn what + it hides, and needs that grain's own policy to do it. + """ + return _require_cohort_policy(model_cls).cohort_policy + + def _require_cohort_policy(model_cls: type[SQLModel]) -> type[SuppressibleModel]: """The chokepoint's gate. A model with no declared policy cannot be read through this module at all — neither its rows nor a count of them, since @@ -195,30 +197,35 @@ async def fetch_and_suppress[ModelT: SQLModel]( model_cls: type[ModelT], floor: int, *, - cross_grain: tuple[CrossGrainAdditives, Collection[Any]] | None = None, + cross_grain: tuple[CrossGrainAdditives, Mapping[Any, Collection[str]]] | None = None, ) -> list[ModelT]: """Query, suppress, construct — the one path rows take out of the database. ``cross_grain`` is for a coarse-grained endpoint whose rows are sums over a finer grain that this service also publishes: pass the additive columns - together with the keys the finer grain withholds (see - ``build_hidden_grain_probe``) and those columns are blanked before any row - becomes a model. Both suppression passes run here rather than in the router - so that no endpoint can construct a response model from an unsuppressed row. + together with what the finer grain hides per key (see + ``anonymization.hidden_additive_columns``) and those columns are blanked + before any row becomes a model. Both suppression passes run here rather than + in the router so that no endpoint can construct a response model from an + unsuppressed row. """ suppressible_cls = _require_cohort_policy(model_cls) rows = await starrocks_pool.fetch_all(query, params) suppressed = suppress_small_cohorts(rows, suppressible_cls.cohort_policy, floor) if cross_grain is not None: - additives, hidden_keys = cross_grain - suppressed = suppress_cross_grain_additives(suppressed, additives, hidden_keys) + additives, hidden_by_key = cross_grain + suppressed = suppress_cross_grain_additives(suppressed, additives, hidden_by_key) return [model_cls(**row) for row in suppressed] -async def fetch_hidden_grain_keys(query: str, params: tuple[Any, ...]) -> frozenset[Any]: - """Run a ``build_hidden_grain_probe`` query, returning the withheld keys.""" - rows = await starrocks_pool.fetch_all(query, params) - return frozenset(next(iter(row.values())) for row in rows) +async def fetch_grain_scan(query: str, params: tuple[Any, ...]) -> list[dict[str, Any]]: + """Run a ``build_grain_scan`` query, returning its raw unsuppressed rows. + + The only caller is a cross-grain guard, which suppresses them itself and + keeps nothing but the set of columns that came back NULL. Nothing from here + may be returned to a caller — that is what ``fetch_and_suppress`` is for. + """ + return await starrocks_pool.fetch_all(query, params) async def fetch_visible_count(query: str, params: tuple[Any, ...]) -> int: diff --git a/src/ol_analytics_api/tenants/b2b_dashboard/routers/organizations.py b/src/ol_analytics_api/tenants/b2b_dashboard/routers/organizations.py index 4c3c1fb..d2db3d6 100644 --- a/src/ol_analytics_api/tenants/b2b_dashboard/routers/organizations.py +++ b/src/ol_analytics_api/tenants/b2b_dashboard/routers/organizations.py @@ -23,13 +23,17 @@ from fastapi import APIRouter, Depends from sqlmodel import SQLModel -from ol_analytics_api.core.anonymization import CrossGrainAdditives +from ol_analytics_api.core.anonymization import ( + CrossGrainAdditives, + hidden_additive_columns, +) from ol_analytics_api.core.db.query import ( build_count, - build_hidden_grain_probe, + build_grain_scan, build_select, + cohort_policy_of, fetch_and_suppress, - fetch_hidden_grain_keys, + fetch_grain_scan, fetch_visible_count, ) from ol_analytics_api.core.db.refresh_metadata import latest_refresh_timestamp @@ -37,6 +41,7 @@ from ol_analytics_api.tenants.b2b_dashboard.config import settings from ol_analytics_api.tenants.b2b_dashboard.models import ( ContentEngagementDepth, + ContractMonthlyEngagementTrend, ContractUtilization, EnrollmentCompletionFunnel, MonthlyEngagementTrend, @@ -62,29 +67,46 @@ _ORG_FILTER_COLUMN = "sso_organization_id" +# A finer-grain scan is a guard, not paging: it has to see every finer row at +# once. This bounds what one request can pull into the pod anyway. It is sized +# far above the real shape of the data (an org's contracts times its months), +# and a scan that reaches it is treated as truncated rather than complete. +_GRAIN_SCAN_LIMIT = 10_000 + + @dataclass(frozen=True) class _FinerGrain: """The contract-grained sibling MV whose rows sum into this endpoint's. - Only the engagement trend needs one. It is the single org endpoint that - aggregates *across* an org's contracts while the contract router publishes - the same months one contract at a time, so a contract-month the floor - withholds is recoverable as ``org_total - sum(the visible contract - months)``. The other four org endpoints carry a contract per row already, - and the content-engagement pair partitions by course run — a run belongs to - exactly one contract, so its org row and its contract row hold identical - counts and the floor makes the same call on both. Nothing is left over to - subtract in either case. + Only the engagement trend needs one, and the other four org endpoints need + none for two different reasons. Three of them (contract-utilization, + enrollment-funnel, program-funnel) carry a contract per row already, so + there is no coarser total to difference against. The fourth, + content-engagement, is org x course_run against a contract-grained sibling + of org x contract x course_run — but a course run belongs to exactly one + contract, so the sibling adds a label rather than splitting a row, the two + hold identical counts, and the floor makes the same call on both. + + That leaves the trend. It is the one org endpoint that aggregates *across* + an org's contracts while the contract router publishes the same months one + contract at a time, so what a contract-month withholds is recoverable as + ``org_total - sum(the visible contract months)``. ``additive_columns`` are the event sums, which do add up exactly across - contracts. The learner counts do not — a learner active under two contracts - is counted in both rows — so subtracting them bounds the withheld cohort - rather than revealing it, and they stay published. + contracts. ``non_additive_columns`` are the derived columns that do not, and + are listed rather than left implicit: between them the two must account for + every derived column on the coarse model, so adding a sixth aggregate to the + MV cannot leave a new subtraction open just because nobody thought about it + here. (The learner counts are ``secondary``, not ``derived``, and are not + additive either — a learner active under two contracts is counted in both + rows — so subtracting them bounds the hidden cohort rather than revealing + it, and they stay published.) """ mv: str - cohort_column: str + model: type[SQLModel] additive_columns: tuple[str, ...] + non_additive_columns: tuple[str, ...] = () @dataclass(frozen=True) @@ -102,6 +124,37 @@ class _OrgEndpoint: order_by: tuple[str, ...] finer_grain: _FinerGrain | None = None + def __post_init__(self) -> None: + """Reject a finer-grain declaration that leaves a derived column + unaccounted for. + + Same argument as ``CohortPolicy``'s own validation, which this mirrors: + the failure being guarded against is a column nobody classified quietly + keeping its subtraction open, and no test would notice. Failing at + import time makes it impossible to deploy. + """ + if self.finer_grain is None: + return + derived = set(cohort_policy_of(self.model).derived) + additive = set(self.finer_grain.additive_columns) + non_additive = set(self.finer_grain.non_additive_columns) + if overlap := additive & non_additive: + msg = f"{self.path}: {sorted(overlap)} are both additive and non-additive." + raise ValueError(msg) + if unknown := (additive | non_additive) - derived: + msg = ( + f"{self.path}: {sorted(unknown)} are not derived columns of " + f"{self.model.__name__}, so nothing sums into them." + ) + raise ValueError(msg) + if unclassified := derived - additive - non_additive: + msg = ( + f"{self.path}: derived columns {sorted(unclassified)} are classified " + "neither additive nor non-additive across the finer grain. An " + "exactly-additive column left out keeps its subtraction open." + ) + raise ValueError(msg) + ENDPOINTS: list[_OrgEndpoint] = [ _OrgEndpoint( @@ -123,8 +176,8 @@ class _OrgEndpoint: ("activity_year_and_month",), finer_grain=_FinerGrain( "mv_b2b_contract_monthly_engagement_trend", - "monthly_active_learners", - ( + ContractMonthlyEngagementTrend, + additive_columns=( "new_enrollments", "certificates_earned", "total_videos_watched", @@ -166,27 +219,25 @@ def _register(spec: _OrgEndpoint) -> None: endpoint whose data only changes when the MV refreshes, hours apart. An endpoint declaring a ``finer_grain`` pays for one more round trip, and - only that endpoint: a probe asking its contract-grained sibling which keys - it withholds, so the additive columns that would reconstruct those rows can - be blanked. The probe projects a grouping key and nothing else — see - ``build_hidden_grain_probe``. + only that endpoint: a scan of its contract-grained sibling, suppressed here + exactly as that sibling's own endpoint would suppress it, so the additive + columns it does not publish in full can be blanked at this grain too. """ query = build_select( _SCHEMA, spec.mv, spec.model, filter_columns=(_ORG_FILTER_COLUMN,), order_by=spec.order_by ) count_query = build_count(_SCHEMA, spec.mv, spec.model, filter_columns=(_ORG_FILTER_COLUMN,)) - additives = probe_query = None + additives = scan_query = None if spec.finer_grain is not None: # The org grain's ordering column is also what lines an org row up with - # the contract rows summing into it, so the same tuple names the probe's + # the contract rows summing into it, so the same tuple names the guard's # key. Endpoints with a finer grain are single-keyed by construction. (key_column,) = spec.order_by additives = CrossGrainAdditives(key_column, spec.finer_grain.additive_columns) - probe_query = build_hidden_grain_probe( + scan_query = build_grain_scan( _SCHEMA, spec.finer_grain.mv, - key_column=key_column, - cohort_column=spec.finer_grain.cohort_column, + spec.finer_grain.model, filter_columns=(_ORG_FILTER_COLUMN,), ) @@ -194,15 +245,30 @@ async def endpoint( organization_id: str, page: Annotated[Pagination, Depends(pagination)] ) -> OrgAnalyticsResponse[SQLModel]: cross_grain = None - if additives is not None and probe_query is not None: - # Probed across the whole org, not just this page: an org row on + if spec.finer_grain is not None and additives is not None and scan_query is not None: + # Scanned across the whole org, not just this page: an org row on # page 1 can be reconstructed from contract rows the caller reads # in any page of the contract endpoint, so the page boundary is # not a limit on what they can subtract. + finer_rows = await fetch_grain_scan(scan_query, (organization_id, _GRAIN_SCAN_LIMIT)) + if len(finer_rows) >= _GRAIN_SCAN_LIMIT: + # Truncated, so the guard cannot prove it saw every contributing + # row. Blanking every additive column for every key is the only + # answer that stays correct; a partial scan silently publishing + # a total it could not check is the failure this whole guard is. + msg = ( + f"{spec.mv}: finer-grain scan hit its {_GRAIN_SCAN_LIMIT}-row limit " + "for one organization, so the cross-grain guard cannot be applied." + ) + raise RuntimeError(msg) cross_grain = ( additives, - await fetch_hidden_grain_keys( - probe_query, (organization_id, settings.anonymization_floor) + hidden_additive_columns( + finer_rows, + cohort_policy_of(spec.finer_grain.model), + settings.anonymization_floor, + key_column=key_column, + additive_columns=spec.finer_grain.additive_columns, ), ) rows = await fetch_and_suppress( diff --git a/tests/test_anonymization.py b/tests/test_anonymization.py index 2210a78..13fde98 100644 --- a/tests/test_anonymization.py +++ b/tests/test_anonymization.py @@ -3,6 +3,7 @@ from ol_analytics_api.core.anonymization import ( CohortPolicy, CrossGrainAdditives, + hidden_additive_columns, suppress_cross_grain_additives, suppress_small_cohorts, ) @@ -338,6 +339,84 @@ def test_containment_mapping_is_not_mutable_after_construction(): columns=("new_enrollments", "total_videos_watched"), ) +# A finer grain shaped like the contract engagement trend: an event sum floored +# through the distinct-learner cohort it is attributable to. +_FINER_POLICY = CohortPolicy( + primary="monthly_active_learners", + secondary=("video_watchers",), + derived={"total_videos_watched": ("video_watchers",)}, + contained_in={"video_watchers": "monthly_active_learners"}, +) + + +def _finer_row(contract, active, watchers, videos, month="2026-07"): + return { + "activity_year_and_month": month, + "contract_id": contract, + "monthly_active_learners": active, + "video_watchers": watchers, + "total_videos_watched": videos, + } + + +def _hidden(rows): + return hidden_additive_columns( + rows, + _FINER_POLICY, + floor=5, + key_column="activity_year_and_month", + additive_columns=("total_videos_watched",), + ) + + +def test_a_finer_row_that_survives_but_nulls_its_total_still_hides_it(): + # The case a row-gate check misses entirely, and the reason this is not one. + # C1 clears the row gate with 30 active learners, so nothing is dropped — + # but only 2 of them watched a video, so its video total is withheld. The + # coarse total minus C2's 500 hands that withheld number straight back. + rows = [ + _finer_row("C1", active=30, watchers=2, videos=17), + _finer_row("C2", active=25, watchers=20, videos=500), + ] + assert _hidden(rows) == {"2026-07": frozenset({"total_videos_watched"})} + + +def test_a_dropped_finer_row_hides_every_additive_column(): + # Below the row gate, so it contributes nothing the caller can see and + # every column it fed into the coarse total is recoverable. + rows = [ + _finer_row("C1", active=2, watchers=2, videos=17), + _finer_row("C2", active=25, watchers=20, videos=500), + ] + assert _hidden(rows) == {"2026-07": frozenset({"total_videos_watched"})} + + +def test_a_finer_row_hidden_by_the_complement_rule_is_caught_too(): + # 30 active of whom 28 watched: the complement rule nulls video_watchers, + # which nulls the total derived from it. Nothing here is sub-floor on its + # own, so only running the real suppression finds this. + rows = [ + _finer_row("C1", active=30, watchers=28, videos=900), + _finer_row("C2", active=25, watchers=10, videos=500), + ] + assert _hidden(rows) == {"2026-07": frozenset({"total_videos_watched"})} + + +def test_fully_published_finer_rows_hide_nothing(): + rows = [ + _finer_row("C1", active=30, watchers=10, videos=900), + _finer_row("C2", active=25, watchers=10, videos=500), + ] + assert _hidden(rows) == {} + + +def test_hidden_columns_are_tracked_per_key(): + rows = [ + _finer_row("C1", active=30, watchers=2, videos=17, month="2026-07"), + _finer_row("C1", active=30, watchers=10, videos=900, month="2026-06"), + ] + assert _hidden(rows) == {"2026-07": frozenset({"total_videos_watched"})} + def test_cross_grain_additives_are_blanked_for_a_withheld_key(): # The caller holds this org total and every contract row but one, so the @@ -350,7 +429,9 @@ def test_cross_grain_additives_are_blanked_for_a_withheld_key(): "total_videos_watched": 500, } ] - (row,) = suppress_cross_grain_additives(rows, _ADDITIVES, {"2026-07"}) + (row,) = suppress_cross_grain_additives( + rows, _ADDITIVES, {"2026-07": frozenset({"new_enrollments", "total_videos_watched"})} + ) assert row["new_enrollments"] is None assert row["total_videos_watched"] is None # Learner counts are not additive across contracts (one learner active @@ -364,17 +445,19 @@ def test_cross_grain_leaves_other_keys_alone(): {"activity_year_and_month": "2026-06", "new_enrollments": 9}, {"activity_year_and_month": "2026-07", "new_enrollments": 12}, ] - june, july = suppress_cross_grain_additives(rows, _ADDITIVES, {"2026-07"}) + june, july = suppress_cross_grain_additives( + rows, _ADDITIVES, {"2026-07": frozenset({"new_enrollments"})} + ) assert june["new_enrollments"] == 9 assert july["new_enrollments"] is None def test_cross_grain_with_nothing_withheld_changes_nothing(): rows = [{"activity_year_and_month": "2026-07", "new_enrollments": 12}] - assert suppress_cross_grain_additives(rows, _ADDITIVES, frozenset()) == rows + assert suppress_cross_grain_additives(rows, _ADDITIVES, {}) == rows def test_cross_grain_does_not_mutate_input_rows(): rows = [{"activity_year_and_month": "2026-07", "new_enrollments": 12}] - suppress_cross_grain_additives(rows, _ADDITIVES, {"2026-07"}) + suppress_cross_grain_additives(rows, _ADDITIVES, {"2026-07": frozenset({"new_enrollments"})}) assert rows[0]["new_enrollments"] == 12 diff --git a/tests/test_column_contract.py b/tests/test_column_contract.py index 2e64e00..d4c1553 100644 --- a/tests/test_column_contract.py +++ b/tests/test_column_contract.py @@ -14,7 +14,7 @@ from __future__ import annotations -from dataclasses import dataclass +from dataclasses import dataclass, replace import pytest from sqlmodel import SQLModel @@ -138,16 +138,39 @@ def test_finer_grain_declarations_match_the_contract_endpoint_they_name(): (key_column,) = spec.order_by assert key_column in coarse_fields assert key_column in set(finer_model.model_fields) - # The probe compares against the finer grain's own row gate, so it - # withholds exactly the keys suppress_small_cohorts drops there. - assert spec.finer_grain.cohort_column == finer_model.cohort_policy.primary + # The guard suppresses the finer rows with the finer model's own policy, + # so the spec must name the model whose MV it scans. + assert spec.finer_grain.model is finer_model # Only event sums add up exactly across contracts. Blanking a cohort # count here would over-suppress; leaving out a sum would leave the - # subtraction open. + # subtraction open. _OrgEndpoint.__post_init__ enforces that the two + # lists partition the coarse model's derived columns; this pins the + # halves it does not know how to check. policy = spec.model.cohort_policy additives = set(spec.finer_grain.additive_columns) assert additives <= set(policy.derived), f"{spec.mv}: additive column is not a derived sum" assert not additives & {policy.primary, *policy.secondary} + assert additives, f"{spec.mv}: a finer grain with no additive column guards nothing" + + +def test_finer_grain_must_classify_every_derived_column(): + # The leak this guards against is an exactly-additive column nobody + # classified quietly keeping its subtraction open, which no other test + # would notice. It has to fail at import time. + trend = next(spec for spec in organizations.ENDPOINTS if spec.finer_grain is not None) + partial = replace(trend.finer_grain, additive_columns=trend.finer_grain.additive_columns[:-1]) + with pytest.raises(ValueError, match="classified neither additive nor non-additive"): + replace(trend, finer_grain=partial) + + +def test_finer_grain_rejects_a_column_that_is_not_derived(): + trend = next(spec for spec in organizations.ENDPOINTS if spec.finer_grain is not None) + bogus = replace( + trend.finer_grain, + additive_columns=(*trend.finer_grain.additive_columns, "monthly_active_learners"), + ) + with pytest.raises(ValueError, match="not derived columns"): + replace(trend, finer_grain=bogus) @_cases @@ -168,3 +191,10 @@ def test_build_select_rejects_empty_order_by(): MitAdminContractHealth, order_by=(), ) + + +def test_finer_grain_rejects_a_column_listed_as_both(): + trend = next(spec for spec in organizations.ENDPOINTS if spec.finer_grain is not None) + both = replace(trend.finer_grain, non_additive_columns=trend.finer_grain.additive_columns) + with pytest.raises(ValueError, match="both additive and non-additive"): + replace(trend, finer_grain=both) diff --git a/tests/test_endpoints.py b/tests/test_endpoints.py index 5acd822..966c013 100644 --- a/tests/test_endpoints.py +++ b/tests/test_endpoints.py @@ -18,6 +18,7 @@ from ol_analytics_api.core.db.client import PoolAcquireTimeoutError from ol_analytics_api.core.db.refresh_metadata import _clear_cache from ol_analytics_api.main import create_app +from ol_analytics_api.tenants.b2b_dashboard.routers import organizations _AS_OF = datetime.datetime(2026, 7, 2, 4, 0, 0) # noqa: DTZ001 @@ -65,25 +66,28 @@ def _is_count_query(query): return "COUNT(*)" in query -def _is_hidden_grain_probe(query): - # build_hidden_grain_probe is the only thing in the service that projects - # DISTINCT — it asks the contract-grained MV which keys it withholds. - return "SELECT DISTINCT " in query +_CONTRACT_TREND_MV = "mv_b2b_contract_monthly_engagement_trend" -def _fake_fetch_all(data_rows, total_count=0, hidden_keys=()): +def _is_grain_scan(query): + # The cross-grain guard's read of the contract-grained sibling. It is the + # only query naming that MV on an org-scoped request. + return _CONTRACT_TREND_MV in query + + +def _fake_fetch_all(data_rows, total_count=0, finer_rows=None): """A fetch_all stub that answers the information_schema as_of probe with _AS_OF, the envelope's total-count query with ``total_count``, the - hidden-grain probe with ``hidden_keys``, and every other query with the - given MV rows.""" + cross-grain scan of the contract-grained MV with ``finer_rows``, and every + other query with the given MV rows.""" async def fetch_all(query, *_args): if "information_schema" in query: return [{"as_of": _AS_OF}] if _is_count_query(query): return [{"total_count": total_count}] - if _is_hidden_grain_probe(query): - return [{"grain_key": key} for key in hidden_keys] + if _is_grain_scan(query): + return list(finer_rows or []) return list(data_rows) return fetch_all @@ -100,7 +104,7 @@ async def fetch_all(query, params): return [{"as_of": _AS_OF}] if _is_count_query(query): return [{"total_count": total_count}] - if _is_hidden_grain_probe(query): + if _is_grain_scan(query): return [] captured["query"] = query captured["params"] = params @@ -409,6 +413,18 @@ def _trend_row(month="2026-07"): } +def _contract_trend_row(contract, *, active, chatbot_users, chatbot_total, month="2026-07"): + """A contract-grained trend row, the grain the org row sums over.""" + return _trend_row(month) | { + "contract_pk": f"pk-{contract}", + "contract_id": contract, + "b2b_contract_name": contract, + "monthly_active_learners": active, + "chatbot_users": chatbot_users, + "total_chatbot_interactions": chatbot_total, + } + + async def _get_trend(app, fetch_all): with ( patch("ol_analytics_api.core.db.client.starrocks_pool.fetch_all", new=fetch_all), @@ -424,50 +440,82 @@ async def _get_trend(app, fetch_all): ) -async def test_org_trend_blanks_additive_columns_a_withheld_contract_row_would_reveal(app): - # The org row survives every per-row floor. What it cannot survive is the - # caller also reading the contract endpoint for the same month: this org's - # other contracts are published there, so `org_total - sum(them)` is the - # activity of the contract the floor withheld. - response = await _get_trend(app, _fake_fetch_all([_trend_row()], hidden_keys=["2026-07"])) +async def test_org_trend_blanks_the_total_a_surviving_contract_row_withholds(app): + # The contract row is NOT dropped: 30 active learners clears the row gate. + # But only 2 of them used the chatbot, so the contract endpoint publishes + # the row with its chatbot total withheld. Leave the org total alone and + # `517 - 500` hands that withheld 17 back, attributable to those 2 learners. + finer = [ + _contract_trend_row("C1", active=30, chatbot_users=2, chatbot_total=17), + _contract_trend_row("C2", active=25, chatbot_users=20, chatbot_total=500), + ] + response = await _get_trend(app, _fake_fetch_all([_trend_row()], finer_rows=finer)) assert response.status_code == 200 (data,) = response.json()["data"] - assert data["new_enrollments"] is None - assert data["certificates_earned"] is None - assert data["total_videos_watched"] is None - assert data["total_problems_attempted"] is None assert data["total_chatbot_interactions"] is None + # Only the column the contract grain actually withholds. The other totals + # are published in full downstream, so nothing can be subtracted out of them + # and blanking them would cost the dashboard data for no gain. + assert data["total_videos_watched"] == 500 + assert data["total_problems_attempted"] == 7 + assert data["new_enrollments"] == 12 # Learner counts don't sum across contracts — a learner active under two is # counted in both rows — so subtracting them bounds the withheld cohort - # rather than revealing it, and over-suppressing them would cost the - # dashboard its headline number for nothing. + # rather than revealing it. assert data["monthly_active_learners"] == 40 - assert data["certified_learners"] == 22 - assert data["video_watchers"] == 18 + assert data["chatbot_users"] == 15 + + +async def test_org_trend_blanks_every_total_when_a_contract_row_is_dropped(app): + # Below the row gate the contract contributes nothing visible at all, so + # every column it fed into the org total is recoverable by subtraction. + finer = [ + _contract_trend_row("C1", active=2, chatbot_users=2, chatbot_total=17), + _contract_trend_row("C2", active=25, chatbot_users=20, chatbot_total=500), + ] + response = await _get_trend(app, _fake_fetch_all([_trend_row()], finer_rows=finer)) + (data,) = response.json()["data"] + for column in ( + "new_enrollments", + "certificates_earned", + "total_videos_watched", + "total_problems_attempted", + "total_chatbot_interactions", + ): + assert data[column] is None, column + assert data["monthly_active_learners"] == 40 -async def test_org_trend_untouched_when_the_contract_grain_withholds_nothing(app): - response = await _get_trend(app, _fake_fetch_all([_trend_row()], hidden_keys=[])) + +async def test_org_trend_untouched_when_the_contract_grain_publishes_in_full(app): + finer = [_contract_trend_row("C1", active=40, chatbot_users=15, chatbot_total=60)] + response = await _get_trend(app, _fake_fetch_all([_trend_row()], finer_rows=finer)) assert response.status_code == 200 (data,) = response.json()["data"] assert data["new_enrollments"] == 12 assert data["total_videos_watched"] == 500 assert data["certificates_earned"] == 30 + assert data["total_chatbot_interactions"] == 60 async def test_org_trend_blanks_only_the_months_the_contract_grain_withholds(app): rows = [_trend_row("2026-06"), _trend_row("2026-07")] - response = await _get_trend(app, _fake_fetch_all(rows, hidden_keys=["2026-07"])) + finer = [ + _contract_trend_row("C1", active=40, chatbot_users=15, chatbot_total=60, month="2026-06"), + _contract_trend_row("C1", active=30, chatbot_users=2, chatbot_total=17, month="2026-07"), + _contract_trend_row("C2", active=25, chatbot_users=20, chatbot_total=500, month="2026-07"), + ] + response = await _get_trend(app, _fake_fetch_all(rows, finer_rows=finer)) june, july = response.json()["data"] - assert june["new_enrollments"] == 12 - assert july["new_enrollments"] is None + assert june["total_chatbot_interactions"] == 60 + assert july["total_chatbot_interactions"] is None -async def test_org_endpoints_without_a_finer_grain_issue_no_probe(app): - # Only the trend endpoint aggregates across an org's contracts. Probing on +async def test_org_endpoints_without_a_finer_grain_issue_no_scan(app): + # Only the trend endpoint aggregates across an org's contracts. Scanning on # the other four would be a round trip per request buying nothing. queries = [] @@ -493,7 +541,7 @@ async def fetch_all(query, *_args): ) assert response.status_code == 200 - assert not any(_is_hidden_grain_probe(query) for query in queries) + assert not any(_is_grain_scan(query) for query in queries) async def test_org_endpoint_403_for_member_who_is_not_a_manager(app): @@ -925,3 +973,16 @@ async def test_contract_endpoint_suppresses_below_the_floor(app): # Contract identity is never suppressed — it is not a cohort. assert data["contract_id"] == "101" assert data["monthly_active_learners"] == 40 + + +async def test_org_trend_fails_closed_when_the_finer_grain_scan_is_truncated(app): + # A truncated scan cannot prove it saw every contributing contract row, so + # the guard cannot say which totals are safe. Publishing an unchecked total + # is the exact failure the guard exists to prevent, so the request fails. + limit = organizations._GRAIN_SCAN_LIMIT # noqa: SLF001 + finer = [ + _contract_trend_row(f"C{index}", active=30, chatbot_users=15, chatbot_total=60) + for index in range(limit) + ] + with pytest.raises(RuntimeError, match="cross-grain guard cannot be applied"): + await _get_trend(app, _fake_fetch_all([_trend_row()], finer_rows=finer)) diff --git a/tests/test_query_chokepoint.py b/tests/test_query_chokepoint.py index ed9b4ac..23b7f05 100644 --- a/tests/test_query_chokepoint.py +++ b/tests/test_query_chokepoint.py @@ -11,9 +11,10 @@ from ol_analytics_api.core.anonymization import CohortPolicy from ol_analytics_api.core.db.query import ( build_count, - build_hidden_grain_probe, + build_grain_scan, + cohort_policy_of, fetch_and_suppress, - fetch_hidden_grain_keys, + fetch_grain_scan, fetch_visible_count, ) @@ -108,45 +109,46 @@ async def test_count_alias_matches_the_column_fetch_visible_count_reads(): assert await fetch_visible_count(query, ()) == 7 -def test_build_hidden_grain_probe_projects_only_the_key(): - # The sub-floor cohort count that motivates the probe is compared inside - # SQL and never read into the process, so it cannot be logged or returned - # by a later change here. NULL cohorts are named explicitly: `cohort < %s` - # is NULL for them, and those are rows suppression drops outright. - query = build_hidden_grain_probe( +def test_build_grain_scan_projects_the_finer_model_and_takes_no_offset(): + # Not build_select: these rows never reach a caller. The guard suppresses + # them itself, so it needs every column the finer cohort policy names, and + # it needs them all at once — a coarse row can be reconstructed from finer + # rows the caller reads on any page of the finer endpoint. + query = build_grain_scan( "b2b_analytics", "mv_b2b_contract_monthly_engagement_trend", - key_column="activity_year_and_month", - cohort_column="monthly_active_learners", + _PolicyRow, filter_columns=("sso_organization_id",), ) assert query == ( - "SELECT DISTINCT activity_year_and_month " + "SELECT enrolled_learners " "FROM b2b_analytics.mv_b2b_contract_monthly_engagement_trend " - "WHERE sso_organization_id = %s " - "AND (monthly_active_learners < %s OR monthly_active_learners IS NULL)" + "WHERE sso_organization_id = %s LIMIT %s" ) + assert "OFFSET" not in query -def test_build_hidden_grain_probe_requires_a_filter_column(): - # Unfiltered, the probe would ask which keys *any* org withholds and blank - # this org's totals on another org's data. +def test_build_grain_scan_requires_a_filter_column(): + # Unfiltered, the scan would read every org's contract rows and blank this + # org's totals on another org's suppression. with pytest.raises(ValueError, match="at least one filter column"): - build_hidden_grain_probe( - "b2b_analytics", - "mv_thing", - key_column="activity_year_and_month", - cohort_column="monthly_active_learners", - filter_columns=(), - ) + build_grain_scan("b2b_analytics", "mv_thing", _PolicyRow, filter_columns=()) -async def test_fetch_hidden_grain_keys_returns_the_projected_values(): +async def test_fetch_grain_scan_returns_rows_unsuppressed(): + # Deliberately raw: the caller suppresses them with the finer grain's own + # policy and keeps only which columns came back NULL. Nothing from here is + # allowed to reach a response. + row = {"enrolled_learners": 2} with patch( "ol_analytics_api.core.db.client.starrocks_pool.fetch_all", - new=AsyncMock(return_value=[{"activity_year_and_month": "2026-06"}]), + new=AsyncMock(return_value=[row]), ): - assert await fetch_hidden_grain_keys("SELECT DISTINCT x FROM y", ()) == frozenset( - {"2026-06"} - ) + assert await fetch_grain_scan("SELECT ...", ()) == [row] + + +def test_cohort_policy_of_gates_on_the_declaration(): + assert cohort_policy_of(_PolicyRow).primary == "enrolled_learners" + with pytest.raises(TypeError, match="cohort_policy"): + cohort_policy_of(_PolicylessRow) From 0628d1205f5dba49e80bfb879a90a3c3fbf680c6 Mon Sep 17 00:00:00 2001 From: Tobias Macey Date: Mon, 24 Aug 2026 16:31:51 -0400 Subject: [PATCH 3/4] feat(openapi): publish a per-tenant spec and generate clients from it (#37) * feat(openapi): publish a per-tenant spec and generate clients from it MIT Learn consumes this service through hand-written types and a hand-written axios client that mirror models.py column for column. That was the right call for the first cut - nothing here published a client, and blocking the dashboard on a cross-repo publish pipeline was not worth it - but it means the frontend drifts silently every time a materialized view gains or renames a column. It already has: ol-analytics-api#33 made three engagement totals nullable and added seven columns, and mit-learn's types still say otherwise. Each tenant is a mounted sub-app, so it owns its own /openapi.json and the root app's schema contains none of it. `openapi.py` builds the apps through the same create_app() the server runs and takes each tenant's document from there, with two fixups that exist because the output is for a client generator rather than for the tenant's own /docs: paths are re-prefixed with the mount path, since Starlette strips it before the sub-app sees a request and a client pointed at the service host would otherwise call URLs that do not exist; and the document version is pinned rather than read from the package, so a release that changes no route produces no diff. Every route now names its own operation_id. That string becomes the generated client's method name, and FastAPI's default derives one from the function name and the whole path - `contractUtilizationOrganizationsOrganizationIdContract UtilizationGet`, renamed whenever the path moves. The tag prefix is also what keeps the org and contract routers' identically-named panels apart. Verified rather than assumed, since the org and contract endpoints are registered in a loop over a table of specs with a runtime-parametrized generic: openapi-generator v7.2.0 emits a distinct TypeScript interface per row model and per envelope (no collapse to one untyped OrgAnalyticsResponse), resolves the 3.1 `anyOf: [integer, null]` columns to `number | null`, and the result typechecks clean under `tsc --strict`. A test asserts the non-collapse so a future registration change cannot quietly undo it. The spec is committed because it is a cross-repo interface: ol-infrastructure's api_clients_pipeline watches openapi/specs/*.yaml on release and publishes the TypeScript package from it, the same arrangement behind @mitodl/mitxonline-api-axios. Drift fails CI twice over - as a test, and as a --check run of the generator, which is the only thing that exercises the generator at all. openapi-diff.yml comments the changelog on any PR touching a spec and fails on a breaking change, because breaking one here means breaking a client someone already shipped. Still to land before mit-learn can drop its hand-written client: the ol-analytics-api-clients repo and the PIPELINE_CONFIGS entry pointing at it. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01AVSXwars1LvgtqV1YWhrB1 * fix(openapi): diff the union of base/head specs, pin oasdiff, describe the pipeline as future The base-only spec loop skipped added specs in the changelog and let the -f guard silently skip deleted specs in the breaking-change check -- deleting a whole published API would have passed CI. Iterate the union of base and head filenames instead, and fail explicitly on a removed spec. Pin oasdiff by digest in both invocations so an upstream image change can't silently alter breaking-change classification. The README, generator script, and workflow comment described the ol-infrastructure client-publishing pipeline as already wired up; none of it exists yet (no PIPELINE_CONFIGS entry, no release branch, MIT Learn still on its hand-written client). Reworded to the intended future state. Added a test asserting every route's operation_id is explicit: the existing uniqueness check still passes for a route that fell back to FastAPI's path-derived default, which is unique but not stable. --------- Co-authored-by: Claude Opus 5 --- .github/workflows/ci.yml | 5 + .github/workflows/openapi-diff.yml | 133 ++ README.md | 36 + bin/generate-openapi-spec | 68 + openapi/specs/b2b_dashboard.yaml | 1587 +++++++++++++++++ pyproject.toml | 6 + src/ol_analytics_api/main.py | 13 +- src/ol_analytics_api/openapi.py | 78 + .../tenants/b2b_dashboard/routers/admin.py | 2 +- .../b2b_dashboard/routers/contracts.py | 5 + .../b2b_dashboard/routers/organizations.py | 7 + tests/test_lifespan.py | 4 +- tests/test_openapi_spec.py | 101 ++ uv.lock | 141 ++ 14 files changed, 2182 insertions(+), 4 deletions(-) create mode 100644 .github/workflows/openapi-diff.yml create mode 100755 bin/generate-openapi-spec create mode 100644 openapi/specs/b2b_dashboard.yaml create mode 100644 src/ol_analytics_api/openapi.py create mode 100644 tests/test_openapi_spec.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c7fbd8a..9fa68ff 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -31,4 +31,9 @@ jobs: - run: uv run ruff check . - run: uv run ruff format --check . - run: uv run mypy src + # tests/test_openapi_spec.py already asserts the spec is current. This + # step is here for the other half: nothing else runs the generator + # itself, and a build-time script that only ever runs by hand is one + # that breaks unnoticed and is discovered when someone needs it. + - run: uv run bin/generate-openapi-spec --check - run: uv run pytest --cov=ol_analytics_api --cov-report=term-missing diff --git a/.github/workflows/openapi-diff.yml b/.github/workflows/openapi-diff.yml new file mode 100644 index 0000000..0081647 --- /dev/null +++ b/.github/workflows/openapi-diff.yml @@ -0,0 +1,133 @@ +name: OpenAPI Diff + +# The committed spec is meant to be what a future Concourse client pipeline +# generates a published TypeScript package from (see README.md), so a diff +# here is a preview of a change to somebody else's build. This surfaces that +# change as a comment and fails the PR on a breaking one, rather than leaving +# it to whoever reads 1500 lines of YAML. + +on: + pull_request: + paths: + - "openapi/specs/**" + +permissions: {} + +jobs: + openapi-diff: + runs-on: ubuntu-24.04 + permissions: + contents: read + pull-requests: write + steps: + - name: Checkout HEAD + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + # The exact commit under review, not the branch name: a push while + # this runs would otherwise diff a commit nobody reviewed. + ref: ${{ github.event.pull_request.head.sha }} + path: head + persist-credentials: false + - name: Checkout BASE + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + ref: ${{ github.event.pull_request.base.sha }} + path: base + persist-credentials: false + - name: Generate oasdiff changelog + run: | # Write the comment body to a file rather than a step output. + # A large changelog interpolated into a JS action's `body:` input becomes a + # huge INPUT_BODY env var, which can blow past the OS argv+envp size limit + # and crash the action with "Argument list too long". Writing straight to a + # file and using `body-path` avoids that entirely. + # + # The spec list is the union of base and head filenames, not just base's: + # a base-only loop silently drops both a spec added in this PR (never in + # base, so never iterated) and a spec removed in this PR (caught by the + # -f guard below, so skipped instead of reported as a removal). + specs=$( + { + [ -d base/openapi/specs ] && (cd base/openapi/specs && ls -1 ./*.yaml) + [ -d head/openapi/specs ] && (cd head/openapi/specs && ls -1 ./*.yaml) + } 2>/dev/null | xargs -n1 basename | sort -u + ) + { + echo "## OpenAPI Changes" + echo "" + echo "
" + echo "Show/hide changes" + echo "" + echo '```' + for name in $specs; do + base_spec="base/openapi/specs/$name" + head_spec="head/openapi/specs/$name" + if [ -f "$base_spec" ] && [ -f "$head_spec" ]; then + echo "## Changes for $name:" + docker run --rm \ + --workdir "$GITHUB_WORKSPACE" \ + --volume "$GITHUB_WORKSPACE:$GITHUB_WORKSPACE:ro" \ + tufin/oasdiff@sha256:6065c16a4c9ce12504752f444d4981091e58c2a35436fac90b649be47d833db3 \ + changelog "$base_spec" "$head_spec" + echo "" + elif [ -f "$head_spec" ]; then + echo "## $name: added" + echo "" + elif [ -f "$base_spec" ]; then + echo "## $name: removed" + echo "" + fi + done + echo '```' + echo "" + echo "Unexpected changes? Ensure your branch is up-to-date with \`main\` (consider rebasing)." + echo "
" + } > comment_body.md + - name: Find existing comment + id: find_comment + uses: peter-evans/find-comment@b30e6a3c0ed37e7c023ccd3f1db5c6c0b0c23aad # v4 + with: + token: ${{ secrets.GITHUB_TOKEN }} + repository: ${{ github.repository }} + issue-number: ${{ github.event.pull_request.number }} + body-includes: "## OpenAPI Changes" + - name: Post changes as comment + uses: peter-evans/create-or-update-comment@e8674b075228eee787fea43ef493e45ece1004c9 # v5 + # Even with no changes, update the old comment if one was found. + with: + token: ${{ secrets.GITHUB_TOKEN }} + edit-mode: "replace" + repository: ${{ github.repository }} + issue-number: ${{ github.event.pull_request.number }} + comment-id: ${{ steps.find_comment.outputs.comment-id }} + body-path: comment_body.md + - name: Check for breaking changes + run: | + # Breaking here means breaking a client someone else already + # generated and shipped, so this fails the PR rather than warning. + # A spec removed outright is the most breaking change there is — + # deleting the whole published API for a tenant — so it's checked + # explicitly rather than relying on the -f guard to skip it. + specs=$( + { + [ -d base/openapi/specs ] && (cd base/openapi/specs && ls -1 ./*.yaml) + [ -d head/openapi/specs ] && (cd head/openapi/specs && ls -1 ./*.yaml) + } 2>/dev/null | xargs -n1 basename | sort -u + ) + for name in $specs; do + base_spec="base/openapi/specs/$name" + head_spec="head/openapi/specs/$name" + if [ -f "$base_spec" ] && [ -f "$head_spec" ]; then + echo "Checking $name for breaking changes..." + docker run --rm \ + --workdir "$GITHUB_WORKSPACE" \ + --volume "$GITHUB_WORKSPACE:$GITHUB_WORKSPACE:ro" \ + tufin/oasdiff@sha256:6065c16a4c9ce12504752f444d4981091e58c2a35436fac90b649be47d833db3 \ + breaking \ + --fail-on ERR \ + --format githubactions \ + "$base_spec" "$head_spec" + elif [ -f "$base_spec" ]; then + echo "::error::$name was removed — deleting a published spec is a breaking change." + exit 1 + fi + done diff --git a/README.md b/README.md index bf9e315..ae68d27 100644 --- a/README.md +++ b/README.md @@ -177,3 +177,39 @@ uv run pytest uv run ruff check . uv run mypy src ``` + +## The published API contract + +Each tenant's OpenAPI document is committed under `openapi/specs/.yaml` +and regenerated with: + +```bash +uv run bin/generate-openapi-spec +``` + +Run it whenever a response model, route or query parameter changes. CI fails +otherwise — both as a test (`tests/test_openapi_spec.py`) and as a +`--check` run of the generator itself. + +The spec is committed rather than served-and-forgotten because it is meant to +become a cross-repo interface. The intended pipeline mirrors the one already +running for `mitxonline` and `mit-learn`: a Concourse pipeline in +`ol-infrastructure` (`ol_concourse/pipelines/libraries/api_clients_pipeline.py`) +watching these files on a release branch, running `openapi-generator` over +them, and publishing a TypeScript client the same way +`@mitodl/mitxonline-api-axios` and `@mitodl/mit-learn-api-axios` are today. +None of that is wired up yet — this repo has no entry in `PIPELINE_CONFIGS` +and no `release` branch, and MIT Learn's dashboard still uses its hand-written +client. Until it is, committing the spec still buys the same thing locally: a +column that appears here without appearing in the diff is a column a +consumer would find out about at runtime once the pipeline exists. + +Two details are worth knowing before editing a route: + +- **`operation_id` is named explicitly on every route.** It becomes the + generated client's method name, so FastAPI's path-derived default would both + produce an unreadable name and rename the method whenever the path moves. +- **Published paths carry the tenant's mount prefix.** A mounted sub-app + describes its routes relative to its own root; `openapi.py` re-prefixes them + so a generated client configured with the service host requests the URLs the + service actually serves. diff --git a/bin/generate-openapi-spec b/bin/generate-openapi-spec new file mode 100755 index 0000000..41c3782 --- /dev/null +++ b/bin/generate-openapi-spec @@ -0,0 +1,68 @@ +#!/usr/bin/env python3 +"""Write each mounted tenant's OpenAPI document to openapi/specs/.yaml. + +Run as `uv run bin/generate-openapi-spec`. + +The output is committed, and that is the point: a materialized view gaining or +renaming a column changes a response model, which changes this file, which +shows up in review as an interface diff instead of silently drifting away from +the clients generated off it. `tests/test_openapi_spec.py` fails when the +committed file no longer matches what the code produces. + +The intended consumer is a Concourse pipeline in ol-infrastructure +(`ol_concourse/pipelines/libraries/api_clients_pipeline.py`), mirroring the one +mitxonline and mit-learn already use: watch `openapi/specs/*.yaml` on a +release branch and regenerate the published TypeScript client from it. That +pipeline isn't wired up for this repo yet (no `PIPELINE_CONFIGS` entry, no +`release` branch) — this file exists so the spec is ready to publish once it +is. +""" + +from __future__ import annotations + +import sys +from pathlib import Path + +import cyclopts + +from ol_analytics_api.openapi import render, tenant_specs + +DEFAULT_DIRECTORY = Path("openapi/specs") + +app = cyclopts.App(name="generate-openapi-spec", help=__doc__) + + +@app.default +def generate(*, directory: Path = DEFAULT_DIRECTORY, check: bool = False) -> None: + """Write (or, with --check, verify) the per-tenant OpenAPI documents. + + Parameters + ---------- + directory + Where the .yaml files are written. + check + Compare against what is already on disk and exit non-zero on any + difference, without writing anything. + """ + stale = [] + for tenant_name, spec in tenant_specs().items(): + path = directory / f"{tenant_name}.yaml" + rendered = render(spec) + if check: + if not path.exists() or path.read_text() != rendered: + stale.append(path) + continue + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(rendered) + sys.stdout.write(f"wrote {path}\n") + if stale: + names = ", ".join(str(path) for path in stale) + sys.stderr.write( + f"OpenAPI spec is out of date: {names}. " + "Regenerate with `uv run bin/generate-openapi-spec`.\n" + ) + raise SystemExit(1) + + +if __name__ == "__main__": + app() diff --git a/openapi/specs/b2b_dashboard.yaml b/openapi/specs/b2b_dashboard.yaml new file mode 100644 index 0000000..a97a280 --- /dev/null +++ b/openapi/specs/b2b_dashboard.yaml @@ -0,0 +1,1587 @@ +openapi: 3.1.0 +info: + title: B2B Analytics Dashboard + description: Aggregated-only B2B site-license analytics for org managers and MIT + contract admins. No individual learner PII. + version: 0.0.1 +paths: + /api/v1/analytics/organizations/{organization_id}/contract-utilization: + get: + tags: + - organizations + summary: Contract Utilization + operationId: organizations_contract_utilization_retrieve + parameters: + - name: organization_id + in: path + required: true + schema: + type: string + title: Organization Id + - name: limit + in: query + required: false + schema: + type: integer + maximum: 1000 + minimum: 1 + default: 100 + title: Limit + - name: offset + in: query + required: false + schema: + type: integer + minimum: 0 + default: 0 + title: Offset + responses: + '200': + description: Successful Response + content: + application/json: + schema: + $ref: '#/components/schemas/OrgAnalyticsResponse_ContractUtilization_' + '422': + description: Validation Error + content: + application/json: + schema: + $ref: '#/components/schemas/HTTPValidationError' + /api/v1/analytics/organizations/{organization_id}/enrollment-funnel: + get: + tags: + - organizations + summary: Enrollment Funnel + operationId: organizations_enrollment_funnel_retrieve + parameters: + - name: organization_id + in: path + required: true + schema: + type: string + title: Organization Id + - name: limit + in: query + required: false + schema: + type: integer + maximum: 1000 + minimum: 1 + default: 100 + title: Limit + - name: offset + in: query + required: false + schema: + type: integer + minimum: 0 + default: 0 + title: Offset + responses: + '200': + description: Successful Response + content: + application/json: + schema: + $ref: '#/components/schemas/OrgAnalyticsResponse_EnrollmentCompletionFunnel_' + '422': + description: Validation Error + content: + application/json: + schema: + $ref: '#/components/schemas/HTTPValidationError' + /api/v1/analytics/organizations/{organization_id}/engagement-trend: + get: + tags: + - organizations + summary: Engagement Trend + operationId: organizations_engagement_trend_retrieve + parameters: + - name: organization_id + in: path + required: true + schema: + type: string + title: Organization Id + - name: limit + in: query + required: false + schema: + type: integer + maximum: 1000 + minimum: 1 + default: 100 + title: Limit + - name: offset + in: query + required: false + schema: + type: integer + minimum: 0 + default: 0 + title: Offset + responses: + '200': + description: Successful Response + content: + application/json: + schema: + $ref: '#/components/schemas/OrgAnalyticsResponse_MonthlyEngagementTrend_' + '422': + description: Validation Error + content: + application/json: + schema: + $ref: '#/components/schemas/HTTPValidationError' + /api/v1/analytics/organizations/{organization_id}/program-funnel: + get: + tags: + - organizations + summary: Program Funnel + operationId: organizations_program_funnel_retrieve + parameters: + - name: organization_id + in: path + required: true + schema: + type: string + title: Organization Id + - name: limit + in: query + required: false + schema: + type: integer + maximum: 1000 + minimum: 1 + default: 100 + title: Limit + - name: offset + in: query + required: false + schema: + type: integer + minimum: 0 + default: 0 + title: Offset + responses: + '200': + description: Successful Response + content: + application/json: + schema: + $ref: '#/components/schemas/OrgAnalyticsResponse_ProgramFunnel_' + '422': + description: Validation Error + content: + application/json: + schema: + $ref: '#/components/schemas/HTTPValidationError' + /api/v1/analytics/organizations/{organization_id}/content-engagement: + get: + tags: + - organizations + summary: Content Engagement + operationId: organizations_content_engagement_retrieve + parameters: + - name: organization_id + in: path + required: true + schema: + type: string + title: Organization Id + - name: limit + in: query + required: false + schema: + type: integer + maximum: 1000 + minimum: 1 + default: 100 + title: Limit + - name: offset + in: query + required: false + schema: + type: integer + minimum: 0 + default: 0 + title: Offset + responses: + '200': + description: Successful Response + content: + application/json: + schema: + $ref: '#/components/schemas/OrgAnalyticsResponse_ContentEngagementDepth_' + '422': + description: Validation Error + content: + application/json: + schema: + $ref: '#/components/schemas/HTTPValidationError' + /api/v1/analytics/organizations/{organization_id}/contracts/{contract_id}/contract-utilization: + get: + tags: + - contracts + summary: Contract Utilization + operationId: contracts_contract_utilization_retrieve + parameters: + - name: organization_id + in: path + required: true + schema: + type: string + title: Organization Id + - name: contract_id + in: path + required: true + schema: + type: string + title: Contract Id + - name: limit + in: query + required: false + schema: + type: integer + maximum: 1000 + minimum: 1 + default: 100 + title: Limit + - name: offset + in: query + required: false + schema: + type: integer + minimum: 0 + default: 0 + title: Offset + responses: + '200': + description: Successful Response + content: + application/json: + schema: + $ref: '#/components/schemas/OrgAnalyticsResponse_ContractUtilization_' + '422': + description: Validation Error + content: + application/json: + schema: + $ref: '#/components/schemas/HTTPValidationError' + /api/v1/analytics/organizations/{organization_id}/contracts/{contract_id}/enrollment-funnel: + get: + tags: + - contracts + summary: Enrollment Funnel + operationId: contracts_enrollment_funnel_retrieve + parameters: + - name: organization_id + in: path + required: true + schema: + type: string + title: Organization Id + - name: contract_id + in: path + required: true + schema: + type: string + title: Contract Id + - name: limit + in: query + required: false + schema: + type: integer + maximum: 1000 + minimum: 1 + default: 100 + title: Limit + - name: offset + in: query + required: false + schema: + type: integer + minimum: 0 + default: 0 + title: Offset + responses: + '200': + description: Successful Response + content: + application/json: + schema: + $ref: '#/components/schemas/OrgAnalyticsResponse_EnrollmentCompletionFunnel_' + '422': + description: Validation Error + content: + application/json: + schema: + $ref: '#/components/schemas/HTTPValidationError' + /api/v1/analytics/organizations/{organization_id}/contracts/{contract_id}/engagement-trend: + get: + tags: + - contracts + summary: Engagement Trend + operationId: contracts_engagement_trend_retrieve + parameters: + - name: organization_id + in: path + required: true + schema: + type: string + title: Organization Id + - name: contract_id + in: path + required: true + schema: + type: string + title: Contract Id + - name: limit + in: query + required: false + schema: + type: integer + maximum: 1000 + minimum: 1 + default: 100 + title: Limit + - name: offset + in: query + required: false + schema: + type: integer + minimum: 0 + default: 0 + title: Offset + responses: + '200': + description: Successful Response + content: + application/json: + schema: + $ref: '#/components/schemas/OrgAnalyticsResponse_ContractMonthlyEngagementTrend_' + '422': + description: Validation Error + content: + application/json: + schema: + $ref: '#/components/schemas/HTTPValidationError' + /api/v1/analytics/organizations/{organization_id}/contracts/{contract_id}/program-funnel: + get: + tags: + - contracts + summary: Program Funnel + operationId: contracts_program_funnel_retrieve + parameters: + - name: organization_id + in: path + required: true + schema: + type: string + title: Organization Id + - name: contract_id + in: path + required: true + schema: + type: string + title: Contract Id + - name: limit + in: query + required: false + schema: + type: integer + maximum: 1000 + minimum: 1 + default: 100 + title: Limit + - name: offset + in: query + required: false + schema: + type: integer + minimum: 0 + default: 0 + title: Offset + responses: + '200': + description: Successful Response + content: + application/json: + schema: + $ref: '#/components/schemas/OrgAnalyticsResponse_ProgramFunnel_' + '422': + description: Validation Error + content: + application/json: + schema: + $ref: '#/components/schemas/HTTPValidationError' + /api/v1/analytics/organizations/{organization_id}/contracts/{contract_id}/content-engagement: + get: + tags: + - contracts + summary: Content Engagement + operationId: contracts_content_engagement_retrieve + parameters: + - name: organization_id + in: path + required: true + schema: + type: string + title: Organization Id + - name: contract_id + in: path + required: true + schema: + type: string + title: Contract Id + - name: limit + in: query + required: false + schema: + type: integer + maximum: 1000 + minimum: 1 + default: 100 + title: Limit + - name: offset + in: query + required: false + schema: + type: integer + minimum: 0 + default: 0 + title: Offset + responses: + '200': + description: Successful Response + content: + application/json: + schema: + $ref: '#/components/schemas/OrgAnalyticsResponse_ContractContentEngagementDepth_' + '422': + description: Validation Error + content: + application/json: + schema: + $ref: '#/components/schemas/HTTPValidationError' + /api/v1/analytics/admin/contract-health: + get: + tags: + - admin + summary: Contract Health + operationId: admin_contract_health_retrieve + parameters: + - name: limit + in: query + required: false + schema: + type: integer + maximum: 1000 + minimum: 1 + default: 100 + title: Limit + - name: offset + in: query + required: false + schema: + type: integer + minimum: 0 + default: 0 + title: Offset + responses: + '200': + description: Successful Response + content: + application/json: + schema: + $ref: '#/components/schemas/AdminAnalyticsResponse_MitAdminContractHealth_' + '422': + description: Validation Error + content: + application/json: + schema: + $ref: '#/components/schemas/HTTPValidationError' +components: + schemas: + AdminAnalyticsResponse_MitAdminContractHealth_: + properties: + as_of: + anyOf: + - type: string + format: date-time + - type: 'null' + title: As Of + total_count: + type: integer + title: Total Count + data: + items: + $ref: '#/components/schemas/MitAdminContractHealth' + type: array + title: Data + type: object + required: + - as_of + - total_count + - data + title: AdminAnalyticsResponse[MitAdminContractHealth] + ContentEngagementDepth: + properties: + organization_key: + type: string + title: Organization Key + organization_name: + type: string + title: Organization Name + courserun_readable_id: + type: string + title: Courserun Readable Id + courserun_title: + type: string + title: Courserun Title + total_enrolled_learners: + type: integer + title: Total Enrolled Learners + engaged_learners: + anyOf: + - type: integer + - type: 'null' + title: Engaged Learners + engagement_rate_pct: + anyOf: + - type: number + - type: 'null' + title: Engagement Rate Pct + total_videos_watched: + anyOf: + - type: integer + - type: 'null' + title: Total Videos Watched + video_watchers: + anyOf: + - type: integer + - type: 'null' + title: Video Watchers + avg_videos_per_engaged_learner: + anyOf: + - type: number + - type: 'null' + title: Avg Videos Per Engaged Learner + total_problems_attempted: + anyOf: + - type: integer + - type: 'null' + title: Total Problems Attempted + problem_attempters: + anyOf: + - type: integer + - type: 'null' + title: Problem Attempters + avg_problems_per_engaged_learner: + anyOf: + - type: number + - type: 'null' + title: Avg Problems Per Engaged Learner + total_chatbot_interactions: + anyOf: + - type: integer + - type: 'null' + title: Total Chatbot Interactions + chatbot_users: + anyOf: + - type: integer + - type: 'null' + title: Chatbot Users + chatbot_adoption_pct: + anyOf: + - type: number + - type: 'null' + title: Chatbot Adoption Pct + certificates_earned: + anyOf: + - type: integer + - type: 'null' + title: Certificates Earned + type: object + required: + - organization_key + - organization_name + - courserun_readable_id + - courserun_title + - total_enrolled_learners + - engaged_learners + - engagement_rate_pct + - total_videos_watched + - video_watchers + - avg_videos_per_engaged_learner + - total_problems_attempted + - problem_attempters + - avg_problems_per_engaged_learner + - total_chatbot_interactions + - chatbot_users + - chatbot_adoption_pct + - certificates_earned + title: ContentEngagementDepth + description: 'mv_b2b_content_engagement_depth — grain: org x course_run (all-time). + + + The chatbot columns are exact: ``total_chatbot_interactions`` sums over, + + and ``chatbot_adoption_pct`` divides by, ``chatbot_users`` — which this + + view does emit, so both are correctly floored. ``engagement_rate_pct`` is + + ``engaged_learners / total_enrolled_learners``, also correct. + + + The video and problem columns are floored through the cohorts the view now + + publishes (ol-data-platform PR #2520): ``total_videos_watched`` is summed + + over ``video_watchers`` and ``total_problems_attempted`` over + + ``problem_attempters``, each a strict subset of ``engaged_learners`` + + because watching a video or attempting a problem is one of the activities + + that sets ``active_count``. (Every cohort this view emits is such a + + subset. That is a property of these particular cohorts, not a general + + rule — see ``MonthlyEngagementTrend``, where ``enrolling_learners`` is + + not a subset of its primary because enrolling does not set + + ``active_count``.) + + + The ``avg_*_per_engaged_learner`` columns are derived from *two* cohorts, + + which is why each names both. The denominator is ``engaged_learners`` — + + that is what the dbt SQL divides by, so the naming is now accurate — but + + the numerator is the activity SUM, contributed by only the narrower + + cohort. Mapping the average to its denominator alone would leave the + + numerator recoverable: an unsuppressed average multiplied by a published + + ``engaged_learners`` yields the suppressed total exactly, and when the + + contributing cohort is a single learner that total *is* that learner''s + + value. Naming both cohorts nulls the average whenever either is sub-floor. + + + ``certificates_earned`` is the one column still floored as a count of + + itself: it is ``sum(certificate_count)``, an event count, and this view + + emits no certified-learner cohort to attribute it to (unlike + + ``MonthlyEngagementTrend``, which has ``certified_learners``). Flooring an + + event count is weaker than flooring a cohort — several certificates can + + come from one learner — but strictly better than not flooring it. Emitting + + the cohort from dbt would close this the same way #2520 closed the others.' + ContractContentEngagementDepth: + properties: + organization_key: + type: string + title: Organization Key + organization_name: + type: string + title: Organization Name + courserun_readable_id: + type: string + title: Courserun Readable Id + courserun_title: + type: string + title: Courserun Title + total_enrolled_learners: + type: integer + title: Total Enrolled Learners + engaged_learners: + anyOf: + - type: integer + - type: 'null' + title: Engaged Learners + engagement_rate_pct: + anyOf: + - type: number + - type: 'null' + title: Engagement Rate Pct + total_videos_watched: + anyOf: + - type: integer + - type: 'null' + title: Total Videos Watched + video_watchers: + anyOf: + - type: integer + - type: 'null' + title: Video Watchers + avg_videos_per_engaged_learner: + anyOf: + - type: number + - type: 'null' + title: Avg Videos Per Engaged Learner + total_problems_attempted: + anyOf: + - type: integer + - type: 'null' + title: Total Problems Attempted + problem_attempters: + anyOf: + - type: integer + - type: 'null' + title: Problem Attempters + avg_problems_per_engaged_learner: + anyOf: + - type: number + - type: 'null' + title: Avg Problems Per Engaged Learner + total_chatbot_interactions: + anyOf: + - type: integer + - type: 'null' + title: Total Chatbot Interactions + chatbot_users: + anyOf: + - type: integer + - type: 'null' + title: Chatbot Users + chatbot_adoption_pct: + anyOf: + - type: number + - type: 'null' + title: Chatbot Adoption Pct + certificates_earned: + anyOf: + - type: integer + - type: 'null' + title: Certificates Earned + contract_pk: + type: string + title: Contract Pk + contract_id: + type: string + title: Contract Id + b2b_contract_name: + type: string + title: B2B Contract Name + type: object + required: + - organization_key + - organization_name + - courserun_readable_id + - courserun_title + - total_enrolled_learners + - engaged_learners + - engagement_rate_pct + - total_videos_watched + - video_watchers + - avg_videos_per_engaged_learner + - total_problems_attempted + - problem_attempters + - avg_problems_per_engaged_learner + - total_chatbot_interactions + - chatbot_users + - chatbot_adoption_pct + - certificates_earned + - contract_pk + - contract_id + - b2b_contract_name + title: ContractContentEngagementDepth + description: 'mv_b2b_contract_content_engagement_depth — grain: org x contract + x run. + + + The contract-scoped sibling of ``ContentEngagementDepth``, inherited for + + the same reason as ``ContractMonthlyEngagementTrend``. + + + Unlike the trend view, these rows ARE a strict partition of the org-level + + view: a course run belongs to exactly one contract, so naming the contract + + labels a row rather than splitting it, and every count here equals its + + org-level counterpart for the same course run. + + + That equality is why this pair needs no cross-grain guard, where the trend + + pair does. Nothing is aggregated away going from contract grain to org + + grain, so there is no remainder to subtract: a course run''s org row and its + + contract row hold the same numbers, the floor makes the same call on both, + + and a caller reading one learns nothing the other withholds.' + ContractMonthlyEngagementTrend: + properties: + organization_key: + type: string + title: Organization Key + organization_name: + type: string + title: Organization Name + activity_year_and_month: + type: string + title: Activity Year And Month + monthly_active_learners: + type: integer + title: Monthly Active Learners + new_enrollments: + anyOf: + - type: integer + - type: 'null' + title: New Enrollments + enrolling_learners: + anyOf: + - type: integer + - type: 'null' + title: Enrolling Learners + certificates_earned: + anyOf: + - type: integer + - type: 'null' + title: Certificates Earned + certified_learners: + anyOf: + - type: integer + - type: 'null' + title: Certified Learners + total_videos_watched: + anyOf: + - type: integer + - type: 'null' + title: Total Videos Watched + video_watchers: + anyOf: + - type: integer + - type: 'null' + title: Video Watchers + total_problems_attempted: + anyOf: + - type: integer + - type: 'null' + title: Total Problems Attempted + problem_attempters: + anyOf: + - type: integer + - type: 'null' + title: Problem Attempters + total_chatbot_interactions: + anyOf: + - type: integer + - type: 'null' + title: Total Chatbot Interactions + chatbot_users: + anyOf: + - type: integer + - type: 'null' + title: Chatbot Users + contract_pk: + type: string + title: Contract Pk + contract_id: + type: string + title: Contract Id + b2b_contract_name: + type: string + title: B2B Contract Name + type: object + required: + - organization_key + - organization_name + - activity_year_and_month + - monthly_active_learners + - new_enrollments + - enrolling_learners + - certificates_earned + - certified_learners + - total_videos_watched + - video_watchers + - total_problems_attempted + - problem_attempters + - total_chatbot_interactions + - chatbot_users + - contract_pk + - contract_id + - b2b_contract_name + title: ContractMonthlyEngagementTrend + description: 'mv_b2b_contract_monthly_engagement_trend — grain: org x contract + x month. + + + The contract-scoped sibling of ``MonthlyEngagementTrend``, backing the + + endpoints nested under a contract. Subclassed rather than redeclared so the + + two can''t drift: the column set and the ``cohort_policy`` — which is what + + the anonymization floor reads — are inherited verbatim, and only contract + + identity is added. The dbt models are siblings in the same way. + + + The contract columns are not cohorts and take no part in the policy. + + + A learner active under two of an org''s contracts appears in both rows, so + + these rows do not partition the org-level view''s learner counts; summing + + ``monthly_active_learners`` across contracts can exceed the org''s own + + figure. Activity totals, being sums of events, do add up — which is what + + makes a contract-month the floor withholds recoverable from the org + + endpoint as ``org_total - sum(the visible contract months)``. The org + + endpoint defends against that itself: it probes this view for the months + + it withholds and blanks its own additive totals for them (see + + ``routers.organizations._FinerGrain``). The learner counts are left alone, + + because not adding up is exactly what stops them from being recovered by + + subtraction.' + ContractUtilization: + properties: + organization_key: + type: string + title: Organization Key + organization_name: + type: string + title: Organization Name + contract_pk: + type: string + title: Contract Pk + contract_id: + type: string + title: Contract Id + b2b_contract_name: + type: string + title: B2B Contract Name + b2b_contract_is_active: + type: boolean + title: B2B Contract Is Active + b2b_contract_start_date: + anyOf: + - type: string + format: date + - type: 'null' + title: B2B Contract Start Date + b2b_contract_end_date: + anyOf: + - type: string + format: date + - type: 'null' + title: B2B Contract End Date + seat_limit: + anyOf: + - type: integer + - type: 'null' + title: Seat Limit + b2b_contract_membership_type: + anyOf: + - type: string + - type: 'null' + title: B2B Contract Membership Type + seats_consumed: + type: integer + title: Seats Consumed + active_learners: + anyOf: + - type: integer + - type: 'null' + title: Active Learners + learners_certified: + anyOf: + - type: integer + - type: 'null' + title: Learners Certified + seat_utilization_pct: + anyOf: + - type: number + - type: 'null' + title: Seat Utilization Pct + completion_rate_pct: + anyOf: + - type: number + - type: 'null' + title: Completion Rate Pct + type: object + required: + - organization_key + - organization_name + - contract_pk + - contract_id + - b2b_contract_name + - b2b_contract_is_active + - b2b_contract_start_date + - b2b_contract_end_date + - seat_limit + - b2b_contract_membership_type + - seats_consumed + - active_learners + - learners_certified + - seat_utilization_pct + - completion_rate_pct + title: ContractUtilization + description: 'mv_b2b_contract_utilization — grain: org x contract.' + EnrollmentCompletionFunnel: + properties: + organization_key: + type: string + title: Organization Key + organization_name: + type: string + title: Organization Name + contract_pk: + type: string + title: Contract Pk + contract_id: + type: string + title: Contract Id + b2b_contract_name: + type: string + title: B2B Contract Name + courserun_pk: + type: string + title: Courserun Pk + courserun_readable_id: + type: string + title: Courserun Readable Id + courserun_title: + type: string + title: Courserun Title + enrolled_learners: + type: integer + title: Enrolled Learners + active_learners: + anyOf: + - type: integer + - type: 'null' + title: Active Learners + passing_learners: + anyOf: + - type: integer + - type: 'null' + title: Passing Learners + certified_learners: + anyOf: + - type: integer + - type: 'null' + title: Certified Learners + active_rate_pct: + anyOf: + - type: number + - type: 'null' + title: Active Rate Pct + completion_rate_pct: + anyOf: + - type: number + - type: 'null' + title: Completion Rate Pct + type: object + required: + - organization_key + - organization_name + - contract_pk + - contract_id + - b2b_contract_name + - courserun_pk + - courserun_readable_id + - courserun_title + - enrolled_learners + - active_learners + - passing_learners + - certified_learners + - active_rate_pct + - completion_rate_pct + title: EnrollmentCompletionFunnel + description: 'mv_b2b_enrollment_completion_funnel — grain: org x contract x + course_run.' + HTTPValidationError: + properties: + detail: + items: + $ref: '#/components/schemas/ValidationError' + type: array + title: Detail + type: object + title: HTTPValidationError + MitAdminContractHealth: + properties: + organization_key: + type: string + title: Organization Key + organization_name: + type: string + title: Organization Name + contract_pk: + type: string + title: Contract Pk + contract_id: + type: string + title: Contract Id + b2b_contract_name: + type: string + title: B2B Contract Name + b2b_contract_is_active: + type: boolean + title: B2B Contract Is Active + b2b_contract_start_date: + anyOf: + - type: string + format: date + - type: 'null' + title: B2B Contract Start Date + b2b_contract_end_date: + anyOf: + - type: string + format: date + - type: 'null' + title: B2B Contract End Date + seat_limit: + anyOf: + - type: integer + - type: 'null' + title: Seat Limit + b2b_contract_membership_type: + anyOf: + - type: string + - type: 'null' + title: B2B Contract Membership Type + seats_consumed: + type: integer + title: Seats Consumed + active_learners: + anyOf: + - type: integer + - type: 'null' + title: Active Learners + certified_learners: + anyOf: + - type: integer + - type: 'null' + title: Certified Learners + seat_utilization_pct: + anyOf: + - type: number + - type: 'null' + title: Seat Utilization Pct + completion_rate_pct: + anyOf: + - type: number + - type: 'null' + title: Completion Rate Pct + health_status: + type: string + title: Health Status + type: object + required: + - organization_key + - organization_name + - contract_pk + - contract_id + - b2b_contract_name + - b2b_contract_is_active + - b2b_contract_start_date + - b2b_contract_end_date + - seat_limit + - b2b_contract_membership_type + - seats_consumed + - active_learners + - certified_learners + - seat_utilization_pct + - completion_rate_pct + - health_status + title: MitAdminContractHealth + description: 'mv_b2b_mit_admin_contract_health — grain: org x contract (MIT + admin only).' + MonthlyEngagementTrend: + properties: + organization_key: + type: string + title: Organization Key + organization_name: + type: string + title: Organization Name + activity_year_and_month: + type: string + title: Activity Year And Month + monthly_active_learners: + type: integer + title: Monthly Active Learners + new_enrollments: + anyOf: + - type: integer + - type: 'null' + title: New Enrollments + enrolling_learners: + anyOf: + - type: integer + - type: 'null' + title: Enrolling Learners + certificates_earned: + anyOf: + - type: integer + - type: 'null' + title: Certificates Earned + certified_learners: + anyOf: + - type: integer + - type: 'null' + title: Certified Learners + total_videos_watched: + anyOf: + - type: integer + - type: 'null' + title: Total Videos Watched + video_watchers: + anyOf: + - type: integer + - type: 'null' + title: Video Watchers + total_problems_attempted: + anyOf: + - type: integer + - type: 'null' + title: Total Problems Attempted + problem_attempters: + anyOf: + - type: integer + - type: 'null' + title: Problem Attempters + total_chatbot_interactions: + anyOf: + - type: integer + - type: 'null' + title: Total Chatbot Interactions + chatbot_users: + anyOf: + - type: integer + - type: 'null' + title: Chatbot Users + type: object + required: + - organization_key + - organization_name + - activity_year_and_month + - monthly_active_learners + - new_enrollments + - enrolling_learners + - certificates_earned + - certified_learners + - total_videos_watched + - video_watchers + - total_problems_attempted + - problem_attempters + - total_chatbot_interactions + - chatbot_users + title: MonthlyEngagementTrend + description: "mv_b2b_monthly_engagement_trend — grain: org x year_month.\n\n\ + Every aggregate here is floored through the cohort that contributes to it,\n\ + which the view publishes alongside it (ol-data-platform PR #2520).\n\nNone\ + \ of them is attributable to ``monthly_active_learners``. Each is a\nplain\ + \ SUM over the source report, so only the learners who did that\nspecific\ + \ thing contribute — and clearing the primary floor says nothing\nabout whether\ + \ that narrower cohort cleared it. A month with 40 active\nlearners can carry\ + \ a chatbot total contributed by exactly one of them,\nwhich is why each total\ + \ is ``derived`` from its own cohort rather than\nfrom the primary.\n\nHow\ + \ each cohort relates to the primary differs, and neither case makes\nmapping\ + \ to the primary safe:\n\n- ``certified_learners``, ``video_watchers``, ``problem_attempters``\ + \ and\n ``chatbot_users`` are strict *subsets*. ``active_count`` is 1 when\ + \ any\n of navigation, discussion, videos, problems, chatbot or certificate\n\ + \ activity is nonzero (organization_administration_report.sql), so each\n\ + \ of those actions sets it.\n- ``enrolling_learners`` is *not* a subset.\ + \ ``enrolled_count`` is absent\n from that expression, so enrolling alone\ + \ never sets ``active_count``\n and a learner who only enrolled is counted\ + \ here but not in the primary.\n The row gate is unaffected — a month whose\ + \ primary is sub-floor is\n dropped whole, which over-suppresses a large\ + \ enrollment cohort rather\n than disclosing one — but the subset reasoning\ + \ does not apply, and\n ``new_enrollments`` is floored through ``enrolling_learners``\ + \ on its\n own terms.\n\n``new_enrollments`` and ``certificates_earned``\ + \ are SUMs of\nper-learner-per-course-run markers, so they count *events*,\ + \ not learners:\none learner enrolling in six runs reads as ``new_enrollments\ + \ == 6`` and\nwould clear a floor of 5 on its own. Flooring them directly\ + \ is therefore\nthe wrong instrument — they are ``derived`` from ``enrolling_learners``\n\ + and ``certified_learners``, the distinct-learner counts they are actually\n\ + attributable to, which do carry the floor." + OrgAnalyticsResponse_ContentEngagementDepth_: + properties: + organization_id: + type: string + title: Organization Id + as_of: + anyOf: + - type: string + format: date-time + - type: 'null' + title: As Of + total_count: + type: integer + title: Total Count + data: + items: + $ref: '#/components/schemas/ContentEngagementDepth' + type: array + title: Data + type: object + required: + - organization_id + - as_of + - total_count + - data + title: OrgAnalyticsResponse[ContentEngagementDepth] + OrgAnalyticsResponse_ContractContentEngagementDepth_: + properties: + organization_id: + type: string + title: Organization Id + as_of: + anyOf: + - type: string + format: date-time + - type: 'null' + title: As Of + total_count: + type: integer + title: Total Count + data: + items: + $ref: '#/components/schemas/ContractContentEngagementDepth' + type: array + title: Data + type: object + required: + - organization_id + - as_of + - total_count + - data + title: OrgAnalyticsResponse[ContractContentEngagementDepth] + OrgAnalyticsResponse_ContractMonthlyEngagementTrend_: + properties: + organization_id: + type: string + title: Organization Id + as_of: + anyOf: + - type: string + format: date-time + - type: 'null' + title: As Of + total_count: + type: integer + title: Total Count + data: + items: + $ref: '#/components/schemas/ContractMonthlyEngagementTrend' + type: array + title: Data + type: object + required: + - organization_id + - as_of + - total_count + - data + title: OrgAnalyticsResponse[ContractMonthlyEngagementTrend] + OrgAnalyticsResponse_ContractUtilization_: + properties: + organization_id: + type: string + title: Organization Id + as_of: + anyOf: + - type: string + format: date-time + - type: 'null' + title: As Of + total_count: + type: integer + title: Total Count + data: + items: + $ref: '#/components/schemas/ContractUtilization' + type: array + title: Data + type: object + required: + - organization_id + - as_of + - total_count + - data + title: OrgAnalyticsResponse[ContractUtilization] + OrgAnalyticsResponse_EnrollmentCompletionFunnel_: + properties: + organization_id: + type: string + title: Organization Id + as_of: + anyOf: + - type: string + format: date-time + - type: 'null' + title: As Of + total_count: + type: integer + title: Total Count + data: + items: + $ref: '#/components/schemas/EnrollmentCompletionFunnel' + type: array + title: Data + type: object + required: + - organization_id + - as_of + - total_count + - data + title: OrgAnalyticsResponse[EnrollmentCompletionFunnel] + OrgAnalyticsResponse_MonthlyEngagementTrend_: + properties: + organization_id: + type: string + title: Organization Id + as_of: + anyOf: + - type: string + format: date-time + - type: 'null' + title: As Of + total_count: + type: integer + title: Total Count + data: + items: + $ref: '#/components/schemas/MonthlyEngagementTrend' + type: array + title: Data + type: object + required: + - organization_id + - as_of + - total_count + - data + title: OrgAnalyticsResponse[MonthlyEngagementTrend] + OrgAnalyticsResponse_ProgramFunnel_: + properties: + organization_id: + type: string + title: Organization Id + as_of: + anyOf: + - type: string + format: date-time + - type: 'null' + title: As Of + total_count: + type: integer + title: Total Count + data: + items: + $ref: '#/components/schemas/ProgramFunnel' + type: array + title: Data + type: object + required: + - organization_id + - as_of + - total_count + - data + title: OrgAnalyticsResponse[ProgramFunnel] + ProgramFunnel: + properties: + organization_key: + type: string + title: Organization Key + organization_name: + type: string + title: Organization Name + contract_pk: + type: string + title: Contract Pk + contract_id: + type: string + title: Contract Id + b2b_contract_name: + type: string + title: B2B Contract Name + program_pk: + type: string + title: Program Pk + program_title: + type: string + title: Program Title + total_courses: + type: integer + title: Total Courses + enrolled_in_contract_courses: + type: integer + title: Enrolled In Contract Courses + enrolled_via_program: + anyOf: + - type: integer + - type: 'null' + title: Enrolled Via Program + program_course_completers: + anyOf: + - type: integer + - type: 'null' + title: Program Course Completers + type: object + required: + - organization_key + - organization_name + - contract_pk + - contract_id + - b2b_contract_name + - program_pk + - program_title + - total_courses + - enrolled_in_contract_courses + - enrolled_via_program + - program_course_completers + title: ProgramFunnel + description: 'mv_b2b_program_funnel — grain: org x contract x program. + + + ``total_courses`` counts courses, not learners, so it is not a cohort.' + ValidationError: + properties: + loc: + items: + anyOf: + - type: string + - type: integer + type: array + title: Location + msg: + type: string + title: Message + type: + type: string + title: Error Type + input: + title: Input + ctx: + type: object + title: Context + type: object + required: + - loc + - msg + - type + title: ValidationError diff --git a/pyproject.toml b/pyproject.toml index f1e00b0..c28c509 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -83,6 +83,9 @@ dev = [ "types-hvac>=2.3", "asgi-lifespan>=2.1.0", "pytest-cov>=7.1.0", + "cyclopts>=4.22.5", + "pyyaml>=6.0.3", + "types-pyyaml>=6.0.12.20260724", ] [build-system] @@ -106,6 +109,9 @@ skip_covered = false [tool.ruff] line-length = 100 target-version = "py312" +# bin/ scripts are extensionless with a shebang, matching the bin/starrocks-auth +# convention. Ruff discovers *.py only, so `ruff check .` would skip them. +extend-include = ["bin/*"] [tool.ruff.lint] select = ["ALL"] diff --git a/src/ol_analytics_api/main.py b/src/ol_analytics_api/main.py index 5a163b9..c2d7a3d 100644 --- a/src/ol_analytics_api/main.py +++ b/src/ol_analytics_api/main.py @@ -88,8 +88,14 @@ class Tenant: idiom a tenant author already writes) makes the lifecycle contract structural: a tenant declares its lifespan in one place and hands it over, instead of remembering to wire a bespoke hook pair into a registry. + + ``name`` is the tenant's stable slug — the tenant's own ``TENANT_NAME``, + which already names its readiness sub-path. It also names the tenant's + published OpenAPI document (openapi/specs/.yaml), so it is part of a + consumer-visible filename and should not be renamed casually. """ + name: str mount_path: str create_app: Callable[[], FastAPI] lifespan: Callable[[FastAPI], AbstractAsyncContextManager[None]] | None = None @@ -97,7 +103,12 @@ class Tenant: # Add a new tenant by appending a Tenant() entry here. TENANTS: list[Tenant] = [ - Tenant("/api/v1/analytics", b2b_dashboard.create_app, b2b_dashboard.lifespan), + Tenant( + b2b_dashboard.TENANT_NAME, + "/api/v1/analytics", + b2b_dashboard.create_app, + b2b_dashboard.lifespan, + ), ] diff --git a/src/ol_analytics_api/openapi.py b/src/ol_analytics_api/openapi.py new file mode 100644 index 0000000..ca6192e --- /dev/null +++ b/src/ol_analytics_api/openapi.py @@ -0,0 +1,78 @@ +"""Compose one OpenAPI document per mounted tenant. + +Each tenant is an independent ``FastAPI()`` mounted under the root app (see +main.py), so it owns its own ``/openapi.json`` and the root app's schema does +not contain a single tenant path. Dumping ``app.openapi()`` therefore yields +the health endpoints and nothing a client would generate against — the schema +a consumer needs has to come from the sub-app. + +Two things are fixed up on the way out, and both exist because the document is +written for a *client generator* rather than for the sub-app's own ``/docs``: + +- **Paths are re-prefixed with the mount path.** A sub-app describes its routes + relative to its own root ("/organizations/{id}/..."), because Starlette's + Mount strips the prefix before the sub-app ever sees the request. A generated + client configured with the service host as its base URL would then request + the wrong URL. Prefixing here keeps the generated client's paths identical to + the absolute paths the service actually serves, which is also what + mit-learn's hand-written client hardcodes today. + +- **The document version is pinned, not read from the package.** Sourcing it + from the package's CalVer would rewrite every spec on every release, and the + committed spec exists to make *interface* changes visible in review. A + release that changes no route should produce no diff here. + +``tenant_specs()`` builds the documents and ``render()`` serializes one exactly +as the committed file holds it. Both live here rather than in +``bin/generate-openapi-spec`` so the drift test compares against the same +serializer that wrote the file, instead of a second one that can disagree. + +This module is a build-time tool. Nothing the server imports reaches it, which +is why PyYAML and cyclopts are dev dependencies: the running service never +needs either. +""" + +from __future__ import annotations + +from typing import Any + +import yaml + +from ol_analytics_api.main import TENANTS, create_app + +# Pinned rather than derived from the package version — see the module +# docstring. Bump deliberately when a tenant's interface breaks. +SPEC_VERSION = "0.0.1" + + +def _prefix_paths(paths: dict[str, Any], mount_path: str) -> dict[str, Any]: + return {f"{mount_path}{path}": item for path, item in paths.items()} + + +def tenant_specs() -> dict[str, dict[str, Any]]: + """Returns ``{tenant name: OpenAPI document}`` for every mounted tenant. + + Builds the apps through the same ``create_app()`` the server runs, so a + route the registry does not actually mount cannot reach a published spec. + """ + root = create_app() + tenant_apps = root.state.tenant_apps + specs: dict[str, dict[str, Any]] = {} + for tenant in TENANTS: + spec = tenant_apps[tenant.mount_path].openapi() + spec["info"]["version"] = SPEC_VERSION + spec["paths"] = _prefix_paths(spec["paths"], tenant.mount_path) + specs[tenant.name] = spec + return specs + + +def render(spec: dict[str, Any]) -> str: + """Serialize one OpenAPI document exactly as the committed file holds it. + + ``sort_keys=False`` keeps FastAPI's own ordering (paths in registration + order, then components) rather than alphabetising the whole document. That + order is already deterministic across runs, and alphabetising it would put + every path's method, parameters and responses in an order nobody wrote, + making real changes harder to find in a diff. + """ + return yaml.safe_dump(spec, sort_keys=False, default_flow_style=False, allow_unicode=True) diff --git a/src/ol_analytics_api/tenants/b2b_dashboard/routers/admin.py b/src/ol_analytics_api/tenants/b2b_dashboard/routers/admin.py index 9292cc9..5beff3e 100644 --- a/src/ol_analytics_api/tenants/b2b_dashboard/routers/admin.py +++ b/src/ol_analytics_api/tenants/b2b_dashboard/routers/admin.py @@ -38,7 +38,7 @@ _ORDER_BY = ("organization_key", "contract_pk") -@router.get("/contract-health") +@router.get("/contract-health", operation_id="admin_contract_health_retrieve") async def contract_health( page: Annotated[Pagination, Depends(pagination)], ) -> AdminAnalyticsResponse[MitAdminContractHealth]: diff --git a/src/ol_analytics_api/tenants/b2b_dashboard/routers/contracts.py b/src/ol_analytics_api/tenants/b2b_dashboard/routers/contracts.py index 6ab76f8..6b3c261 100644 --- a/src/ol_analytics_api/tenants/b2b_dashboard/routers/contracts.py +++ b/src/ol_analytics_api/tenants/b2b_dashboard/routers/contracts.py @@ -156,6 +156,11 @@ async def endpoint( # its concrete row model; mypy can't type a value used as a type param. response_model=OrgAnalyticsResponse[spec.model], # type: ignore[name-defined] name=endpoint.__name__, + # See the same call in organizations.py: named explicitly so a + # generated client's method name is ours rather than a derivative of + # the path. The `contracts_` prefix is what separates these from the + # org router's identically-named panels. + operation_id=f"contracts_{endpoint.__name__}_retrieve", ) diff --git a/src/ol_analytics_api/tenants/b2b_dashboard/routers/organizations.py b/src/ol_analytics_api/tenants/b2b_dashboard/routers/organizations.py index d2db3d6..e696bba 100644 --- a/src/ol_analytics_api/tenants/b2b_dashboard/routers/organizations.py +++ b/src/ol_analytics_api/tenants/b2b_dashboard/routers/organizations.py @@ -296,6 +296,13 @@ async def endpoint( # its concrete row model; mypy can't type a value used as a type param. response_model=OrgAnalyticsResponse[spec.model], # type: ignore[name-defined] name=endpoint.__name__, + # Named explicitly because this is what a generated client's method is + # called. FastAPI's default derives one from the function name *and* + # the whole path, which would make the TS method + # `contractUtilizationOrganizationsOrganizationIdContractUtilizationGet` + # and — worse — churn it whenever the path changes. The tag prefix is + # what keeps this distinct from the contract router's same-named panel. + operation_id=f"organizations_{endpoint.__name__}_retrieve", ) diff --git a/tests/test_lifespan.py b/tests/test_lifespan.py index 59a4009..9309e74 100644 --- a/tests/test_lifespan.py +++ b/tests/test_lifespan.py @@ -67,8 +67,8 @@ async def failing_lifespan(_app: object): yield # unreachable, but keeps this an async generator fake_tenants = [ - Tenant("/ok", create_app=object, lifespan=ok_lifespan), - Tenant("/broken", create_app=object, lifespan=failing_lifespan), + Tenant("ok", "/ok", create_app=object, lifespan=ok_lifespan), + Tenant("broken", "/broken", create_app=object, lifespan=failing_lifespan), ] root_app = SimpleNamespace( state=SimpleNamespace(tenant_apps={"/ok": object(), "/broken": object()}) diff --git a/tests/test_openapi_spec.py b/tests/test_openapi_spec.py new file mode 100644 index 0000000..f0b43b7 --- /dev/null +++ b/tests/test_openapi_spec.py @@ -0,0 +1,101 @@ +"""The published OpenAPI contract. + +`openapi/specs/.yaml` is what a future Concourse client pipeline is +meant to generate the TypeScript package from (see README.md), so these +assertions are about what consumers would receive, not about FastAPI's +internals. +""" + +from pathlib import Path + +import pytest +from fastapi.routing import APIRoute + +from ol_analytics_api.main import TENANTS, create_app +from ol_analytics_api.openapi import render, tenant_specs + +SPECS_DIR = Path(__file__).resolve().parent.parent / "openapi" / "specs" + + +@pytest.fixture(scope="module") +def specs(): + return tenant_specs() + + +def test_committed_spec_matches_the_code(specs): + """The whole reason the spec is committed: drift is a failing test, not a + consumer discovering a renamed column at runtime.""" + for tenant_name, spec in specs.items(): + path = SPECS_DIR / f"{tenant_name}.yaml" + assert path.exists(), f"{path} is missing. Run `uv run bin/generate-openapi-spec`." + assert path.read_text() == render(spec), ( + f"{path} is out of date. Run `uv run bin/generate-openapi-spec`." + ) + + +def test_every_mounted_tenant_publishes_a_spec(specs): + assert set(specs) == {tenant.name for tenant in TENANTS} + + +def test_paths_carry_the_mount_prefix(specs): + """A sub-app describes its routes relative to its own root, but a generated + client is configured with the service host as its base URL. Publishing the + unprefixed paths would produce a client that requests URLs the service does + not serve.""" + for tenant in TENANTS: + paths = specs[tenant.name]["paths"] + assert paths, f"{tenant.name} published no paths at all" + assert all(path.startswith(f"{tenant.mount_path}/") for path in paths) + + +def test_each_row_model_gets_its_own_response_schema(specs): + """The org and contract endpoints are registered in a loop over a table of + specs, parametrizing one generic envelope at runtime. If that collapsed to + a single `OrgAnalyticsResponse` component, every panel would generate the + same untyped row and the whole point of generating a client would be lost. + """ + schemas = specs["b2b_dashboard"]["components"]["schemas"] + envelopes = {name for name in schemas if name.startswith("OrgAnalyticsResponse")} + row_models = { + "ContractUtilization", + "EnrollmentCompletionFunnel", + "MonthlyEngagementTrend", + "ProgramFunnel", + "ContentEngagementDepth", + "ContractMonthlyEngagementTrend", + "ContractContentEngagementDepth", + } + assert envelopes == {f"OrgAnalyticsResponse_{model}_" for model in row_models} + assert row_models <= set(schemas) + + +def test_operation_ids_are_unique_and_stable(specs): + """openapi-generator names a client method after its operationId, so a + collision silently drops a method and a path-derived default renames every + method whenever a route moves. Both are named explicitly in the routers.""" + operation_ids = [ + operation["operationId"] + for spec in specs.values() + for path_item in spec["paths"].values() + for operation in path_item.values() + ] + assert len(operation_ids) == len(set(operation_ids)) + # The org and contract routers expose identically-named panels; the tag + # prefix is what keeps them apart. + assert "organizations_contract_utilization_retrieve" in operation_ids + assert "contracts_contract_utilization_retrieve" in operation_ids + + +def test_operation_ids_are_explicit(): + """Uniqueness alone doesn't catch a route that never set operation_id: + FastAPI falls back to a path-derived default, which is unique but not + stable, so a route relying on it would pass the test above and still + rename its generated client method whenever the path moves.""" + root = create_app() + for tenant in TENANTS: + tenant_app = root.state.tenant_apps[tenant.mount_path] + for route in tenant_app.routes: + if isinstance(route, APIRoute): + assert route.operation_id is not None, ( + f"{tenant.name} route {route.path} has no explicit operation_id" + ) diff --git a/uv.lock b/uv.lock index a9a4b1b..8fdd992 100644 --- a/uv.lock +++ b/uv.lock @@ -162,6 +162,15 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/66/40/c53deb2cd0c9b0fb636d24d9f40924cf2e65028e6b20b10cd5c1eeb2c730/ast_serialize-0.6.0-cp39-abi3-win_arm64.whl", hash = "sha256:ccd132fe8db56f61fe743b1f644d01b8d65b83248a8da506f3132bda86d6ed5e", size = 1072965, upload-time = "2026-06-30T20:02:54.097Z" }, ] +[[package]] +name = "attrs" +version = "26.1.0" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/9a/8e/82a0fe20a541c03148528be8cac2408564a6c9a0cc7e9171802bc1d26985/attrs-26.1.0.tar.gz", hash = "sha256:d03ceb89cb322a8fd706d4fb91940737b6642aa36998fe130a9bc96c985eff32", size = 952055, upload-time = "2026-03-19T14:22:25.026Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/64/b4/17d4b0b2a2dc85a6df63d1157e028ed19f90d4cd97c36717afef2bc2f395/attrs-26.1.0-py3-none-any.whl", hash = "sha256:c647aa4a12dfbad9333ca4e71fe62ddc36f4e63b2d260a37a8b83d2f043ac309", size = 67548, upload-time = "2026-03-19T14:22:23.645Z" }, +] + [[package]] name = "cachetools" version = "7.1.4" @@ -343,6 +352,30 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/52/30/21b2ad45959cd50e909e02ebac1e30b4ceb7162e91c11d4c570223a458b7/coverage-7.15.0-py3-none-any.whl", hash = "sha256:56da6a4cbe8f7e9e80bd072ca9cefe67d7106a440a7ec06519ec6507ac94ad19", size = 212632, upload-time = "2026-07-02T13:10:48.641Z" }, ] +[[package]] +name = "cyclopts" +version = "4.22.5" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "attrs" }, + { name = "docstring-parser" }, + { name = "rich" }, + { name = "rich-rst" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/be/05/689617b7e86503417c172f577d791524cb13b9697303d5d44409a971ba10/cyclopts-4.22.5.tar.gz", hash = "sha256:94044506317462cad90fb01a917dadce1f48a0915ba3605dc8d178dea1229e24", size = 195144, upload-time = "2026-08-04T13:53:00.303Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/83/58/bcab9c33fb7a25a1f5970f357c5b19729bc81d50615d2f737b20c4255909/cyclopts-4.22.5-py3-none-any.whl", hash = "sha256:cf9ce285836053d156730ea4ea0ad0c75cf63beb3f3d8edf222a795bc57666ab", size = 234557, upload-time = "2026-08-04T13:52:58.509Z" }, +] + +[[package]] +name = "docstring-parser" +version = "0.18.0" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/e0/4d/f332313098c1de1b2d2ff91cf2674415cc7cddab2ca1b01ae29774bd5fdf/docstring_parser-0.18.0.tar.gz", hash = "sha256:292510982205c12b1248696f44959db3cdd1740237a968ea1e2e7a900eeb2015", size = 29341, upload-time = "2026-04-14T04:09:19.867Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/a7/5f/ed01f9a3cdffbd5a008556fc7b2a08ddb1cc6ace7effa7340604b1d16699/docstring_parser-0.18.0-py3-none-any.whl", hash = "sha256:b3fcbed555c47d8479be0796ef7e19c2670d428d72e96da63f3a40122860374b", size = 22484, upload-time = "2026-04-14T04:09:18.638Z" }, +] + [[package]] name = "fastapi" version = "0.139.0" @@ -624,6 +657,27 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/06/e6/42a475bfca683b0cd5366f6dd06580062b7e567bb8534d225c877c2f14f3/librt-0.12.0-cp314-cp314t-win_arm64.whl", hash = "sha256:bca1472acbd473eff61059b4409f802c5a1bcb4cd0344d06f939df9c4c125d40", size = 104282, upload-time = "2026-06-30T16:14:09.29Z" }, ] +[[package]] +name = "markdown-it-py" +version = "4.2.0" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "mdurl" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/06/ff/7841249c247aa650a76b9ee4bbaeae59370dc8bfd2f6c01f3630c35eb134/markdown_it_py-4.2.0.tar.gz", hash = "sha256:04a21681d6fbb623de53f6f364d352309d4094dd4194040a10fd51833e418d49", size = 82454, upload-time = "2026-05-07T12:08:28.36Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/b3/81/4da04ced5a082363ecfa159c010d200ecbd959ae410c10c0264a38cac0f5/markdown_it_py-4.2.0-py3-none-any.whl", hash = "sha256:9f7ebbcd14fe59494226453aed97c1070d83f8d24b6fc3a3bcf9a38092641c4a", size = 91687, upload-time = "2026-05-07T12:08:27.182Z" }, +] + +[[package]] +name = "mdurl" +version = "0.1.2" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/d6/54/cfe61301667036ec958cb99bd3efefba235e65cdeb9c84d24a8293ba1d90/mdurl-0.1.2.tar.gz", hash = "sha256:bb413d29f5eea38f31dd4754dd7377d4465116fb207585f97bf925588687c1ba", size = 8729, upload-time = "2022-08-14T12:40:10.846Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/b3/38/89ba8ad64ae25be8de66a6d463314cf1eb366222074cfda9ee839c56a4b4/mdurl-0.1.2-py3-none-any.whl", hash = "sha256:84008a41e51615a49fc9966191ff91509e3c40b939176e643fd50a5c2196b8f8", size = 9979, upload-time = "2022-08-14T12:40:09.779Z" }, +] + [[package]] name = "mypy" version = "2.1.0" @@ -702,14 +756,17 @@ dependencies = [ [package.dev-dependencies] dev = [ { name = "asgi-lifespan" }, + { name = "cyclopts" }, { name = "mypy" }, { name = "pytest" }, { name = "pytest-asyncio" }, { name = "pytest-cov" }, { name = "pytest-httpx" }, + { name = "pyyaml" }, { name = "ruff" }, { name = "types-cachetools" }, { name = "types-hvac" }, + { name = "types-pyyaml" }, ] [package.metadata] @@ -734,14 +791,17 @@ requires-dist = [ [package.metadata.requires-dev] dev = [ { name = "asgi-lifespan", specifier = ">=2.1.0" }, + { name = "cyclopts", specifier = ">=4.22.5" }, { name = "mypy", specifier = ">=1.13" }, { name = "pytest", specifier = ">=8.3" }, { name = "pytest-asyncio", specifier = ">=0.24" }, { name = "pytest-cov", specifier = ">=7.1.0" }, { name = "pytest-httpx", specifier = ">=0.35" }, + { name = "pyyaml", specifier = ">=6.0.3" }, { name = "ruff", specifier = ">=0.8" }, { name = "types-cachetools", specifier = ">=5.5" }, { name = "types-hvac", specifier = ">=2.3" }, + { name = "types-pyyaml", specifier = ">=6.0.12.20260724" }, ] [[package]] @@ -1126,6 +1186,52 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/0b/d7/1959b9648791274998a9c3526f6d0ec8fd2233e4d4acce81bbae76b44b2a/python_dotenv-1.2.2-py3-none-any.whl", hash = "sha256:1d8214789a24de455a8b8bd8ae6fe3c6b69a5e3d64aa8a8e5d68e694bbcb285a", size = 22101, upload-time = "2026-03-01T16:00:25.09Z" }, ] +[[package]] +name = "pyyaml" +version = "6.0.3" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/05/8e/961c0007c59b8dd7729d542c61a4d537767a59645b82a0b521206e1e25c2/pyyaml-6.0.3.tar.gz", hash = "sha256:d76623373421df22fb4cf8817020cbb7ef15c725b9d5e45f17e189bfc384190f", size = 130960, upload-time = "2025-09-25T21:33:16.546Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/d1/33/422b98d2195232ca1826284a76852ad5a86fe23e31b009c9886b2d0fb8b2/pyyaml-6.0.3-cp312-cp312-macosx_10_13_x86_64.whl", hash = "sha256:7f047e29dcae44602496db43be01ad42fc6f1cc0d8cd6c83d342306c32270196", size = 182063, upload-time = "2025-09-25T21:32:11.445Z" }, + { url = "https://files.pythonhosted.org/packages/89/a0/6cf41a19a1f2f3feab0e9c0b74134aa2ce6849093d5517a0c550fe37a648/pyyaml-6.0.3-cp312-cp312-macosx_11_0_arm64.whl", hash = "sha256:fc09d0aa354569bc501d4e787133afc08552722d3ab34836a80547331bb5d4a0", size = 173973, upload-time = "2025-09-25T21:32:12.492Z" }, + { url = "https://files.pythonhosted.org/packages/ed/23/7a778b6bd0b9a8039df8b1b1d80e2e2ad78aa04171592c8a5c43a56a6af4/pyyaml-6.0.3-cp312-cp312-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:9149cad251584d5fb4981be1ecde53a1ca46c891a79788c0df828d2f166bda28", size = 775116, upload-time = "2025-09-25T21:32:13.652Z" }, + { url = "https://files.pythonhosted.org/packages/65/30/d7353c338e12baef4ecc1b09e877c1970bd3382789c159b4f89d6a70dc09/pyyaml-6.0.3-cp312-cp312-manylinux2014_s390x.manylinux_2_17_s390x.manylinux_2_28_s390x.whl", hash = "sha256:5fdec68f91a0c6739b380c83b951e2c72ac0197ace422360e6d5a959d8d97b2c", size = 844011, upload-time = "2025-09-25T21:32:15.21Z" }, + { url = "https://files.pythonhosted.org/packages/8b/9d/b3589d3877982d4f2329302ef98a8026e7f4443c765c46cfecc8858c6b4b/pyyaml-6.0.3-cp312-cp312-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:ba1cc08a7ccde2d2ec775841541641e4548226580ab850948cbfda66a1befcdc", size = 807870, upload-time = "2025-09-25T21:32:16.431Z" }, + { url = "https://files.pythonhosted.org/packages/05/c0/b3be26a015601b822b97d9149ff8cb5ead58c66f981e04fedf4e762f4bd4/pyyaml-6.0.3-cp312-cp312-musllinux_1_2_aarch64.whl", hash = "sha256:8dc52c23056b9ddd46818a57b78404882310fb473d63f17b07d5c40421e47f8e", size = 761089, upload-time = "2025-09-25T21:32:17.56Z" }, + { url = "https://files.pythonhosted.org/packages/be/8e/98435a21d1d4b46590d5459a22d88128103f8da4c2d4cb8f14f2a96504e1/pyyaml-6.0.3-cp312-cp312-musllinux_1_2_x86_64.whl", hash = "sha256:41715c910c881bc081f1e8872880d3c650acf13dfa8214bad49ed4cede7c34ea", size = 790181, upload-time = "2025-09-25T21:32:18.834Z" }, + { url = "https://files.pythonhosted.org/packages/74/93/7baea19427dcfbe1e5a372d81473250b379f04b1bd3c4c5ff825e2327202/pyyaml-6.0.3-cp312-cp312-win32.whl", hash = "sha256:96b533f0e99f6579b3d4d4995707cf36df9100d67e0c8303a0c55b27b5f99bc5", size = 137658, upload-time = "2025-09-25T21:32:20.209Z" }, + { url = "https://files.pythonhosted.org/packages/86/bf/899e81e4cce32febab4fb42bb97dcdf66bc135272882d1987881a4b519e9/pyyaml-6.0.3-cp312-cp312-win_amd64.whl", hash = "sha256:5fcd34e47f6e0b794d17de1b4ff496c00986e1c83f7ab2fb8fcfe9616ff7477b", size = 154003, upload-time = "2025-09-25T21:32:21.167Z" }, + { url = "https://files.pythonhosted.org/packages/1a/08/67bd04656199bbb51dbed1439b7f27601dfb576fb864099c7ef0c3e55531/pyyaml-6.0.3-cp312-cp312-win_arm64.whl", hash = "sha256:64386e5e707d03a7e172c0701abfb7e10f0fb753ee1d773128192742712a98fd", size = 140344, upload-time = "2025-09-25T21:32:22.617Z" }, + { url = "https://files.pythonhosted.org/packages/d1/11/0fd08f8192109f7169db964b5707a2f1e8b745d4e239b784a5a1dd80d1db/pyyaml-6.0.3-cp313-cp313-macosx_10_13_x86_64.whl", hash = "sha256:8da9669d359f02c0b91ccc01cac4a67f16afec0dac22c2ad09f46bee0697eba8", size = 181669, upload-time = "2025-09-25T21:32:23.673Z" }, + { url = "https://files.pythonhosted.org/packages/b1/16/95309993f1d3748cd644e02e38b75d50cbc0d9561d21f390a76242ce073f/pyyaml-6.0.3-cp313-cp313-macosx_11_0_arm64.whl", hash = "sha256:2283a07e2c21a2aa78d9c4442724ec1eb15f5e42a723b99cb3d822d48f5f7ad1", size = 173252, upload-time = "2025-09-25T21:32:25.149Z" }, + { url = "https://files.pythonhosted.org/packages/50/31/b20f376d3f810b9b2371e72ef5adb33879b25edb7a6d072cb7ca0c486398/pyyaml-6.0.3-cp313-cp313-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:ee2922902c45ae8ccada2c5b501ab86c36525b883eff4255313a253a3160861c", size = 767081, upload-time = "2025-09-25T21:32:26.575Z" }, + { url = "https://files.pythonhosted.org/packages/49/1e/a55ca81e949270d5d4432fbbd19dfea5321eda7c41a849d443dc92fd1ff7/pyyaml-6.0.3-cp313-cp313-manylinux2014_s390x.manylinux_2_17_s390x.manylinux_2_28_s390x.whl", hash = "sha256:a33284e20b78bd4a18c8c2282d549d10bc8408a2a7ff57653c0cf0b9be0afce5", size = 841159, upload-time = "2025-09-25T21:32:27.727Z" }, + { url = "https://files.pythonhosted.org/packages/74/27/e5b8f34d02d9995b80abcef563ea1f8b56d20134d8f4e5e81733b1feceb2/pyyaml-6.0.3-cp313-cp313-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:0f29edc409a6392443abf94b9cf89ce99889a1dd5376d94316ae5145dfedd5d6", size = 801626, upload-time = "2025-09-25T21:32:28.878Z" }, + { url = "https://files.pythonhosted.org/packages/f9/11/ba845c23988798f40e52ba45f34849aa8a1f2d4af4b798588010792ebad6/pyyaml-6.0.3-cp313-cp313-musllinux_1_2_aarch64.whl", hash = "sha256:f7057c9a337546edc7973c0d3ba84ddcdf0daa14533c2065749c9075001090e6", size = 753613, upload-time = "2025-09-25T21:32:30.178Z" }, + { url = "https://files.pythonhosted.org/packages/3d/e0/7966e1a7bfc0a45bf0a7fb6b98ea03fc9b8d84fa7f2229e9659680b69ee3/pyyaml-6.0.3-cp313-cp313-musllinux_1_2_x86_64.whl", hash = "sha256:eda16858a3cab07b80edaf74336ece1f986ba330fdb8ee0d6c0d68fe82bc96be", size = 794115, upload-time = "2025-09-25T21:32:31.353Z" }, + { url = "https://files.pythonhosted.org/packages/de/94/980b50a6531b3019e45ddeada0626d45fa85cbe22300844a7983285bed3b/pyyaml-6.0.3-cp313-cp313-win32.whl", hash = "sha256:d0eae10f8159e8fdad514efdc92d74fd8d682c933a6dd088030f3834bc8e6b26", size = 137427, upload-time = "2025-09-25T21:32:32.58Z" }, + { url = "https://files.pythonhosted.org/packages/97/c9/39d5b874e8b28845e4ec2202b5da735d0199dbe5b8fb85f91398814a9a46/pyyaml-6.0.3-cp313-cp313-win_amd64.whl", hash = "sha256:79005a0d97d5ddabfeeea4cf676af11e647e41d81c9a7722a193022accdb6b7c", size = 154090, upload-time = "2025-09-25T21:32:33.659Z" }, + { url = "https://files.pythonhosted.org/packages/73/e8/2bdf3ca2090f68bb3d75b44da7bbc71843b19c9f2b9cb9b0f4ab7a5a4329/pyyaml-6.0.3-cp313-cp313-win_arm64.whl", hash = "sha256:5498cd1645aa724a7c71c8f378eb29ebe23da2fc0d7a08071d89469bf1d2defb", size = 140246, upload-time = "2025-09-25T21:32:34.663Z" }, + { url = "https://files.pythonhosted.org/packages/9d/8c/f4bd7f6465179953d3ac9bc44ac1a8a3e6122cf8ada906b4f96c60172d43/pyyaml-6.0.3-cp314-cp314-macosx_10_13_x86_64.whl", hash = "sha256:8d1fab6bb153a416f9aeb4b8763bc0f22a5586065f86f7664fc23339fc1c1fac", size = 181814, upload-time = "2025-09-25T21:32:35.712Z" }, + { url = "https://files.pythonhosted.org/packages/bd/9c/4d95bb87eb2063d20db7b60faa3840c1b18025517ae857371c4dd55a6b3a/pyyaml-6.0.3-cp314-cp314-macosx_11_0_arm64.whl", hash = "sha256:34d5fcd24b8445fadc33f9cf348c1047101756fd760b4dacb5c3e99755703310", size = 173809, upload-time = "2025-09-25T21:32:36.789Z" }, + { url = "https://files.pythonhosted.org/packages/92/b5/47e807c2623074914e29dabd16cbbdd4bf5e9b2db9f8090fa64411fc5382/pyyaml-6.0.3-cp314-cp314-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:501a031947e3a9025ed4405a168e6ef5ae3126c59f90ce0cd6f2bfc477be31b7", size = 766454, upload-time = "2025-09-25T21:32:37.966Z" }, + { url = "https://files.pythonhosted.org/packages/02/9e/e5e9b168be58564121efb3de6859c452fccde0ab093d8438905899a3a483/pyyaml-6.0.3-cp314-cp314-manylinux2014_s390x.manylinux_2_17_s390x.manylinux_2_28_s390x.whl", hash = "sha256:b3bc83488de33889877a0f2543ade9f70c67d66d9ebb4ac959502e12de895788", size = 836355, upload-time = "2025-09-25T21:32:39.178Z" }, + { url = "https://files.pythonhosted.org/packages/88/f9/16491d7ed2a919954993e48aa941b200f38040928474c9e85ea9e64222c3/pyyaml-6.0.3-cp314-cp314-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:c458b6d084f9b935061bc36216e8a69a7e293a2f1e68bf956dcd9e6cbcd143f5", size = 794175, upload-time = "2025-09-25T21:32:40.865Z" }, + { url = "https://files.pythonhosted.org/packages/dd/3f/5989debef34dc6397317802b527dbbafb2b4760878a53d4166579111411e/pyyaml-6.0.3-cp314-cp314-musllinux_1_2_aarch64.whl", hash = "sha256:7c6610def4f163542a622a73fb39f534f8c101d690126992300bf3207eab9764", size = 755228, upload-time = "2025-09-25T21:32:42.084Z" }, + { url = "https://files.pythonhosted.org/packages/d7/ce/af88a49043cd2e265be63d083fc75b27b6ed062f5f9fd6cdc223ad62f03e/pyyaml-6.0.3-cp314-cp314-musllinux_1_2_x86_64.whl", hash = "sha256:5190d403f121660ce8d1d2c1bb2ef1bd05b5f68533fc5c2ea899bd15f4399b35", size = 789194, upload-time = "2025-09-25T21:32:43.362Z" }, + { url = "https://files.pythonhosted.org/packages/23/20/bb6982b26a40bb43951265ba29d4c246ef0ff59c9fdcdf0ed04e0687de4d/pyyaml-6.0.3-cp314-cp314-win_amd64.whl", hash = "sha256:4a2e8cebe2ff6ab7d1050ecd59c25d4c8bd7e6f400f5f82b96557ac0abafd0ac", size = 156429, upload-time = "2025-09-25T21:32:57.844Z" }, + { url = "https://files.pythonhosted.org/packages/f4/f4/a4541072bb9422c8a883ab55255f918fa378ecf083f5b85e87fc2b4eda1b/pyyaml-6.0.3-cp314-cp314-win_arm64.whl", hash = "sha256:93dda82c9c22deb0a405ea4dc5f2d0cda384168e466364dec6255b293923b2f3", size = 143912, upload-time = "2025-09-25T21:32:59.247Z" }, + { url = "https://files.pythonhosted.org/packages/7c/f9/07dd09ae774e4616edf6cda684ee78f97777bdd15847253637a6f052a62f/pyyaml-6.0.3-cp314-cp314t-macosx_10_13_x86_64.whl", hash = "sha256:02893d100e99e03eda1c8fd5c441d8c60103fd175728e23e431db1b589cf5ab3", size = 189108, upload-time = "2025-09-25T21:32:44.377Z" }, + { url = "https://files.pythonhosted.org/packages/4e/78/8d08c9fb7ce09ad8c38ad533c1191cf27f7ae1effe5bb9400a46d9437fcf/pyyaml-6.0.3-cp314-cp314t-macosx_11_0_arm64.whl", hash = "sha256:c1ff362665ae507275af2853520967820d9124984e0f7466736aea23d8611fba", size = 183641, upload-time = "2025-09-25T21:32:45.407Z" }, + { url = "https://files.pythonhosted.org/packages/7b/5b/3babb19104a46945cf816d047db2788bcaf8c94527a805610b0289a01c6b/pyyaml-6.0.3-cp314-cp314t-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:6adc77889b628398debc7b65c073bcb99c4a0237b248cacaf3fe8a557563ef6c", size = 831901, upload-time = "2025-09-25T21:32:48.83Z" }, + { url = "https://files.pythonhosted.org/packages/8b/cc/dff0684d8dc44da4d22a13f35f073d558c268780ce3c6ba1b87055bb0b87/pyyaml-6.0.3-cp314-cp314t-manylinux2014_s390x.manylinux_2_17_s390x.manylinux_2_28_s390x.whl", hash = "sha256:a80cb027f6b349846a3bf6d73b5e95e782175e52f22108cfa17876aaeff93702", size = 861132, upload-time = "2025-09-25T21:32:50.149Z" }, + { url = "https://files.pythonhosted.org/packages/b1/5e/f77dc6b9036943e285ba76b49e118d9ea929885becb0a29ba8a7c75e29fe/pyyaml-6.0.3-cp314-cp314t-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:00c4bdeba853cc34e7dd471f16b4114f4162dc03e6b7afcc2128711f0eca823c", size = 839261, upload-time = "2025-09-25T21:32:51.808Z" }, + { url = "https://files.pythonhosted.org/packages/ce/88/a9db1376aa2a228197c58b37302f284b5617f56a5d959fd1763fb1675ce6/pyyaml-6.0.3-cp314-cp314t-musllinux_1_2_aarch64.whl", hash = "sha256:66e1674c3ef6f541c35191caae2d429b967b99e02040f5ba928632d9a7f0f065", size = 805272, upload-time = "2025-09-25T21:32:52.941Z" }, + { url = "https://files.pythonhosted.org/packages/da/92/1446574745d74df0c92e6aa4a7b0b3130706a4142b2d1a5869f2eaa423c6/pyyaml-6.0.3-cp314-cp314t-musllinux_1_2_x86_64.whl", hash = "sha256:16249ee61e95f858e83976573de0f5b2893b3677ba71c9dd36b9cf8be9ac6d65", size = 829923, upload-time = "2025-09-25T21:32:54.537Z" }, + { url = "https://files.pythonhosted.org/packages/f0/7a/1c7270340330e575b92f397352af856a8c06f230aa3e76f86b39d01b416a/pyyaml-6.0.3-cp314-cp314t-win_amd64.whl", hash = "sha256:4ad1906908f2f5ae4e5a8ddfce73c320c2a1429ec52eafd27138b7f1cbe341c9", size = 174062, upload-time = "2025-09-25T21:32:55.767Z" }, + { url = "https://files.pythonhosted.org/packages/f1/12/de94a39c2ef588c7e6455cfbe7343d3b2dc9d6b6b2f40c4c6565744c873d/pyyaml-6.0.3-cp314-cp314t-win_arm64.whl", hash = "sha256:ebc55a14a21cb14062aa4162f906cd962b28e2e9ea38f9b4391244cd8de4ae0b", size = 149341, upload-time = "2025-09-25T21:32:56.828Z" }, +] + [[package]] name = "requests" version = "2.34.2" @@ -1141,6 +1247,32 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/a0/f4/c67b0b3f1b9245e8d266f0f112c500d50e5b4e83cb6f3b71b6528104182a/requests-2.34.2-py3-none-any.whl", hash = "sha256:2a0d60c172f83ac6ab31e4554906c0f3b3588d37b5cb939b1c061f4907e278e0", size = 73075, upload-time = "2026-05-14T19:25:26.443Z" }, ] +[[package]] +name = "rich" +version = "15.0.0" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "markdown-it-py" }, + { name = "pygments" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/c0/8f/0722ca900cc807c13a6a0c696dacf35430f72e0ec571c4275d2371fca3e9/rich-15.0.0.tar.gz", hash = "sha256:edd07a4824c6b40189fb7ac9bc4c52536e9780fbbfbddf6f1e2502c31b068c36", size = 230680, upload-time = "2026-04-12T08:24:00.75Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/82/3b/64d4899d73f91ba49a8c18a8ff3f0ea8f1c1d75481760df8c68ef5235bf5/rich-15.0.0-py3-none-any.whl", hash = "sha256:33bd4ef74232fb73fe9279a257718407f169c09b78a87ad3d296f548e27de0bb", size = 310654, upload-time = "2026-04-12T08:24:02.83Z" }, +] + +[[package]] +name = "rich-rst" +version = "2.1.0" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "pygments" }, + { name = "rich" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/e2/d6/d0b9fafc73b65767200da027acab1db1bdb1048f4fea5ebf659df01c700e/rich_rst-2.1.0.tar.gz", hash = "sha256:f4d117b49697f338769759fa5cacf5197da4888b347b9fda2e50aef5cd8d93bd", size = 302732, upload-time = "2026-07-05T02:59:44.308Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/2a/68/1fc93dd759605b5d00fc98b50200739e41ed32bd22d6ba35ca6c3932371b/rich_rst-2.1.0-py3-none-any.whl", hash = "sha256:7ecd1343ee12c879d0e7ae74c3eb6d263b023d2929c6d114212eb1fd91057255", size = 272987, upload-time = "2026-07-05T02:59:42.792Z" }, +] + [[package]] name = "ruff" version = "0.15.20" @@ -1291,6 +1423,15 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/91/34/7a493b4a37cf73bbbc9108a23f9c9e0dd3eceb5e8c9d56eb030d558bc2cb/types_hvac-2.4.0.20260610-py3-none-any.whl", hash = "sha256:534137803ae181274834bd92c45b9f763c6df61cf4ce0e6f78b425cbd32d5134", size = 42400, upload-time = "2026-06-10T06:11:20.286Z" }, ] +[[package]] +name = "types-pyyaml" +version = "6.0.12.20260724" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/3f/6f/a28f44bcd56bebed42b028a2894c79853e2f5e6b5279e633cb3f287a05e7/types_pyyaml-6.0.12.20260724.tar.gz", hash = "sha256:3c1ce1bb73cd5ec02e90390c2b1f00e810d241d8825fd73ff359696839271b6b", size = 17893, upload-time = "2026-07-24T04:58:43.453Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/8b/42/0337fefc615e20ee55d1c8f71b774a9b2b734a04669139c20753b27a2a3a/types_pyyaml-6.0.12.20260724-py3-none-any.whl", hash = "sha256:d57db930a4b2efbc57cf430ec8882765d246929432fa253092f383902329a453", size = 20312, upload-time = "2026-07-24T04:58:42.486Z" }, +] + [[package]] name = "typing-extensions" version = "4.16.0" From b953e6e3cc87a545f67d8fe22acc4f60f45b6751 Mon Sep 17 00:00:00 2001 From: Tobias Macey Date: Fri, 28 Aug 2026 16:50:31 -0400 Subject: [PATCH 4/4] fix(anonymization): guard cohort counts across grains, not just event sums Copilot review on #36: the cross-grain guard only blanked additive event sums, on the reasoning that distinct-learner cohort counts (monthly_active_ learners and friends) aren't exactly additive across a org's contracts, so subtracting them only bounds a withheld cohort rather than revealing it. That reasoning holds when contracts overlap, but not when they don't -- two disjoint contracts sum exactly, so a hidden one's headline count comes back whole from `org_total - visible_contract_total`, the same as a hidden event sum would. Nothing available here can tell overlapping contracts from disjoint ones. CrossGrainAdditives gains guarded_cohorts: the coarse grain's primary and secondary cohort columns, blanked wholesale for any key the finer grain hides anything for, regardless of which specific column triggered it. organizations._register derives it from the coarse model's own cohort_policy, so nothing has to be hand-declared per endpoint the way additive_columns is. monthly_active_learners becomes Optional to carry the blank. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_015gRjAEb8aQ5KXSrqUVT8m7 --- openapi/specs/b2b_dashboard.yaml | 50 +++++++++++----- src/ol_analytics_api/core/anonymization.py | 58 +++++++++++++++---- .../tenants/b2b_dashboard/models.py | 38 ++++++++---- .../b2b_dashboard/routers/organizations.py | 27 +++++++-- tests/test_anonymization.py | 16 ++++- tests/test_column_contract.py | 12 ++-- tests/test_endpoints.py | 47 ++++++++++++--- 7 files changed, 189 insertions(+), 59 deletions(-) diff --git a/openapi/specs/b2b_dashboard.yaml b/openapi/specs/b2b_dashboard.yaml index a97a280..1a48bfb 100644 --- a/openapi/specs/b2b_dashboard.yaml +++ b/openapi/specs/b2b_dashboard.yaml @@ -836,7 +836,9 @@ components: type: string title: Activity Year And Month monthly_active_learners: - type: integer + anyOf: + - type: integer + - type: 'null' title: Monthly Active Learners new_enrollments: anyOf: @@ -937,25 +939,38 @@ components: A learner active under two of an org''s contracts appears in both rows, so - these rows do not partition the org-level view''s learner counts; summing + these rows do not partition the org-level view''s learner counts in + + general; summing ``monthly_active_learners`` across contracts can exceed + + the org''s own figure. Activity totals, being sums of events, always add up + + — which is what makes a contract-month the floor withholds recoverable + + from the org endpoint as ``org_total - sum(the visible contract months)``. - ``monthly_active_learners`` across contracts can exceed the org''s own + The org endpoint defends against that itself: it probes this view for the - figure. Activity totals, being sums of events, do add up — which is what + months it withholds and blanks its own additive totals for them (see - makes a contract-month the floor withholds recoverable from the org + ``routers.organizations._FinerGrain``). - endpoint as ``org_total - sum(the visible contract months)``. The org - endpoint defends against that itself: it probes this view for the months + The learner counts don''t get to skip that defense on the strength of "not - it withholds and blanks its own additive totals for them (see + adding up in general": two contracts that happen to share no learners *do* - ``routers.organizations._FinerGrain``). The learner counts are left alone, + add up exactly, and a hidden one comes back from the visible sibling''s - because not adding up is exactly what stops them from being recovered by + total the same as a hidden event sum would. Nothing here can tell that - subtraction.' + case from an overlapping one, so the org endpoint guards every cohort + + column — not just the additive totals — for any month it hides anything + + for (``CrossGrainAdditives.guarded_cohorts``), accepting the cost of + + blanking counts that overlap would have made safe to publish.' ContractUtilization: properties: organization_key: @@ -1223,7 +1238,9 @@ components: type: string title: Activity Year And Month monthly_active_learners: - type: integer + anyOf: + - type: integer + - type: 'null' title: Monthly Active Learners new_enrollments: anyOf: @@ -1319,7 +1336,14 @@ components: \ == 6`` and\nwould clear a floor of 5 on its own. Flooring them directly\ \ is therefore\nthe wrong instrument — they are ``derived`` from ``enrolling_learners``\n\ and ``certified_learners``, the distinct-learner counts they are actually\n\ - attributable to, which do carry the floor." + attributable to, which do carry the floor.\n\n``monthly_active_learners``\ + \ is Optional even though it is the primary —\neverywhere else the primary\ + \ gates the row (below floor, the row is dropped\nwhole, never nulled) rather\ + \ than being nulled itself. The org grain is the\nexception: it is also this\ + \ endpoint's ``_FinerGrain.guarded_cohorts``\ntarget, so a month whose contract-level\ + \ breakdown hides anything gets its\norg-level ``monthly_active_learners``\ + \ blanked post hoc, after its own row\ngate already passed. See ``routers.organizations``\ + \ and\n``ContractMonthlyEngagementTrend``." OrgAnalyticsResponse_ContentEngagementDepth_: properties: organization_id: diff --git a/src/ol_analytics_api/core/anonymization.py b/src/ol_analytics_api/core/anonymization.py index 6de674d..986fa98 100644 --- a/src/ol_analytics_api/core/anonymization.py +++ b/src/ol_analytics_api/core/anonymization.py @@ -39,11 +39,20 @@ one additive column, because the cohort that column is attributable to is sub-floor. Both cases leave the same hole in the sum. +The coarse grain's distinct-entity cohort counts are not exactly additive the +same way — a learner active under two contracts is one coarse learner but two +finer rows — so subtracting a hidden contract's siblings from the coarse total +is usually only a bound. It stops being only a bound when the finer rows +happen to share no learners: two disjoint contracts sum exactly, and a hidden +one comes back the same way a hidden event sum would. Nothing available here +can tell overlapping contracts from disjoint ones, so cohort columns are +guarded as if every finer grain were disjoint. + Rows alone cannot see any of that. ``hidden_additive_columns`` runs the finer grain through the suppression above — the same function, not a cheaper approximation of it — and reports which additive columns come back NULL per key; ``suppress_cross_grain_additives`` blanks exactly those at the coarse -grain. +grain, plus every cohort column for any key with something hidden. """ from __future__ import annotations @@ -199,15 +208,28 @@ class CrossGrainAdditives: finer rows summing into it (e.g. the activity month). ``columns`` The coarse columns that are *exactly* additive across the finer rows. - Only these are recoverable by subtraction, so only these are blanked. - Distinct-entity counts generally are not additive — a learner active - under two contracts is counted in both finer rows — so subtracting - those yields a bound, not a value, and they are left alone rather than - over-suppressed. + Blanked one at a time: only the specific column the finer grain hides + for a key is recoverable by subtraction, so only that one is blanked. + ``guarded_cohorts`` + The coarse grain's distinct-entity cohort columns (its ``primary`` and + ``secondary`` policy fields) — blanked wholesale, all of them, for any + key the finer grain hides *anything* for. + + These are not exactly additive across the finer rows in general — a + learner active under two contracts is counted once at the coarse grain + but appears in two finer rows — so ``coarse - sum(visible finer)`` is + usually a bound on a hidden cohort, not its value. But nothing here can + tell overlapping contracts from disjoint ones, and when the finer rows + happen to partition the coarse cohort with no overlap, that bound is + exact: a hidden contract-month's learner counts come back the same way + a hidden event sum would. So every cohort column is blanked whenever + anything is hidden for the key, at the cost of also blanking counts + that overlap would have made safe to publish. """ key_column: str columns: tuple[str, ...] + guarded_cohorts: tuple[str, ...] = () def hidden_additive_columns( @@ -361,18 +383,30 @@ def suppress_cross_grain_additives( small to publish. Withholding the coarse total is what breaks the subtraction; the finer rows themselves stay as they are. - Blanking is per column, not per key: a month where only the chatbot total is - withheld downstream keeps its video and problem totals, which nothing can be - subtracted out of. One hidden finer contribution is enough to blank the - column it belongs to — several are not safer, since the difference is then - their sum, which can still be a handful of entities. + ``additives.columns`` is blanked per column, not per key: a month where + only the chatbot total is withheld downstream keeps its video and problem + totals, which nothing can be subtracted out of. One hidden finer + contribution is enough to blank the column it belongs to — several are not + safer, since the difference is then their sum, which can still be a + handful of entities. + + ``additives.guarded_cohorts`` is blanked per key instead: any hidden entry + for a key blanks every cohort column for that key, regardless of which + specific additive column triggered it. These columns aren't attributable + to one hidden contribution the way an additive column is, so there is no + narrower blanking that stays safe under the disjoint-contract case + ``CrossGrainAdditives`` documents. Input rows are not mutated. """ if not hidden_by_key: return rows + guarded = set(additives.guarded_cohorts) return [ - {column: (None if column in blanked else value) for column, value in row.items()} + { + column: (None if column in blanked or column in guarded else value) + for column, value in row.items() + } if (blanked := hidden_by_key.get(row.get(additives.key_column))) else row for row in rows diff --git a/src/ol_analytics_api/tenants/b2b_dashboard/models.py b/src/ol_analytics_api/tenants/b2b_dashboard/models.py index 8bf195c..680fdd2 100644 --- a/src/ol_analytics_api/tenants/b2b_dashboard/models.py +++ b/src/ol_analytics_api/tenants/b2b_dashboard/models.py @@ -172,6 +172,15 @@ class MonthlyEngagementTrend(SQLModel): the wrong instrument — they are ``derived`` from ``enrolling_learners`` and ``certified_learners``, the distinct-learner counts they are actually attributable to, which do carry the floor. + + ``monthly_active_learners`` is Optional even though it is the primary — + everywhere else the primary gates the row (below floor, the row is dropped + whole, never nulled) rather than being nulled itself. The org grain is the + exception: it is also this endpoint's ``_FinerGrain.guarded_cohorts`` + target, so a month whose contract-level breakdown hides anything gets its + org-level ``monthly_active_learners`` blanked post hoc, after its own row + gate already passed. See ``routers.organizations`` and + ``ContractMonthlyEngagementTrend``. """ cohort_policy: ClassVar[CohortPolicy] = CohortPolicy( @@ -210,7 +219,7 @@ class MonthlyEngagementTrend(SQLModel): organization_key: str organization_name: str activity_year_and_month: str - monthly_active_learners: int + monthly_active_learners: int | None new_enrollments: int | None enrolling_learners: int | None certificates_earned: int | None @@ -394,16 +403,23 @@ class ContractMonthlyEngagementTrend(MonthlyEngagementTrend): The contract columns are not cohorts and take no part in the policy. A learner active under two of an org's contracts appears in both rows, so - these rows do not partition the org-level view's learner counts; summing - ``monthly_active_learners`` across contracts can exceed the org's own - figure. Activity totals, being sums of events, do add up — which is what - makes a contract-month the floor withholds recoverable from the org - endpoint as ``org_total - sum(the visible contract months)``. The org - endpoint defends against that itself: it probes this view for the months - it withholds and blanks its own additive totals for them (see - ``routers.organizations._FinerGrain``). The learner counts are left alone, - because not adding up is exactly what stops them from being recovered by - subtraction. + these rows do not partition the org-level view's learner counts in + general; summing ``monthly_active_learners`` across contracts can exceed + the org's own figure. Activity totals, being sums of events, always add up + — which is what makes a contract-month the floor withholds recoverable + from the org endpoint as ``org_total - sum(the visible contract months)``. + The org endpoint defends against that itself: it probes this view for the + months it withholds and blanks its own additive totals for them (see + ``routers.organizations._FinerGrain``). + + The learner counts don't get to skip that defense on the strength of "not + adding up in general": two contracts that happen to share no learners *do* + add up exactly, and a hidden one comes back from the visible sibling's + total the same as a hidden event sum would. Nothing here can tell that + case from an overlapping one, so the org endpoint guards every cohort + column — not just the additive totals — for any month it hides anything + for (``CrossGrainAdditives.guarded_cohorts``), accepting the cost of + blanking counts that overlap would have made safe to publish. """ contract_pk: str diff --git a/src/ol_analytics_api/tenants/b2b_dashboard/routers/organizations.py b/src/ol_analytics_api/tenants/b2b_dashboard/routers/organizations.py index e696bba..4ff978b 100644 --- a/src/ol_analytics_api/tenants/b2b_dashboard/routers/organizations.py +++ b/src/ol_analytics_api/tenants/b2b_dashboard/routers/organizations.py @@ -97,10 +97,18 @@ class _FinerGrain: are listed rather than left implicit: between them the two must account for every derived column on the coarse model, so adding a sixth aggregate to the MV cannot leave a new subtraction open just because nobody thought about it - here. (The learner counts are ``secondary``, not ``derived``, and are not - additive either — a learner active under two contracts is counted in both - rows — so subtracting them bounds the hidden cohort rather than revealing - it, and they stay published.) + here. + + The coarse model's ``primary`` and ``secondary`` learner counts get no + equivalent declaration here — ``_register`` guards all of them together, + unconditionally, via ``CrossGrainAdditives.guarded_cohorts``. They are not + exactly additive across contracts in general (a learner active under two + is counted once at the org grain but in both contract rows), but two + contracts sharing no learners sum exactly, and this service has no way to + tell that case from an overlapping one before deciding what to publish. So + every cohort column at the coarse grain is blanked whenever the finer scan + hides anything at all for that key, not only the columns known to be safe + from it. """ mv: str @@ -221,7 +229,9 @@ def _register(spec: _OrgEndpoint) -> None: An endpoint declaring a ``finer_grain`` pays for one more round trip, and only that endpoint: a scan of its contract-grained sibling, suppressed here exactly as that sibling's own endpoint would suppress it, so the additive - columns it does not publish in full can be blanked at this grain too. + columns it does not publish in full — and, more bluntly, every cohort + column at this grain — can be blanked for whatever key it hid something + for. """ query = build_select( _SCHEMA, spec.mv, spec.model, filter_columns=(_ORG_FILTER_COLUMN,), order_by=spec.order_by @@ -233,7 +243,12 @@ def _register(spec: _OrgEndpoint) -> None: # the contract rows summing into it, so the same tuple names the guard's # key. Endpoints with a finer grain are single-keyed by construction. (key_column,) = spec.order_by - additives = CrossGrainAdditives(key_column, spec.finer_grain.additive_columns) + coarse_policy = cohort_policy_of(spec.model) + additives = CrossGrainAdditives( + key_column, + spec.finer_grain.additive_columns, + guarded_cohorts=(coarse_policy.primary, *coarse_policy.secondary), + ) scan_query = build_grain_scan( _SCHEMA, spec.finer_grain.mv, diff --git a/tests/test_anonymization.py b/tests/test_anonymization.py index 13fde98..9df5fd8 100644 --- a/tests/test_anonymization.py +++ b/tests/test_anonymization.py @@ -337,6 +337,7 @@ def test_containment_mapping_is_not_mutable_after_construction(): _ADDITIVES = CrossGrainAdditives( key_column="activity_year_and_month", columns=("new_enrollments", "total_videos_watched"), + guarded_cohorts=("monthly_active_learners",), ) # A finer grain shaped like the contract engagement trend: an event sum floored @@ -434,9 +435,18 @@ def test_cross_grain_additives_are_blanked_for_a_withheld_key(): ) assert row["new_enrollments"] is None assert row["total_videos_watched"] is None - # Learner counts are not additive across contracts (one learner active - # under two is counted in both), so subtracting them bounds rather than - # reveals, and they stay published. + # guarded_cohorts is blanked wholesale for any key with something hidden, + # not matched against the specific columns in `blanked` — two contracts + # sharing no learners would sum exactly, and nothing here can tell that + # case from an overlapping one. + assert row["monthly_active_learners"] is None + + +def test_cross_grain_guarded_cohorts_survive_a_key_with_nothing_hidden(): + rows = [{"activity_year_and_month": "2026-06", "monthly_active_learners": 40}] + (row,) = suppress_cross_grain_additives( + rows, _ADDITIVES, {"2026-07": frozenset({"new_enrollments"})} + ) assert row["monthly_active_learners"] == 40 diff --git a/tests/test_column_contract.py b/tests/test_column_contract.py index d4c1553..b42157c 100644 --- a/tests/test_column_contract.py +++ b/tests/test_column_contract.py @@ -141,11 +141,13 @@ def test_finer_grain_declarations_match_the_contract_endpoint_they_name(): # The guard suppresses the finer rows with the finer model's own policy, # so the spec must name the model whose MV it scans. assert spec.finer_grain.model is finer_model - # Only event sums add up exactly across contracts. Blanking a cohort - # count here would over-suppress; leaving out a sum would leave the - # subtraction open. _OrgEndpoint.__post_init__ enforces that the two - # lists partition the coarse model's derived columns; this pins the - # halves it does not know how to check. + # additive_columns names only the event sums that add up exactly + # across contracts regardless of overlap; the cohort counts get their + # own guard in `_register` (CrossGrainAdditives.guarded_cohorts), + # unconditionally, so they have no business showing up here too. + # _OrgEndpoint.__post_init__ enforces that additive/non-additive + # partition the coarse model's derived columns; this pins the halves + # it does not know how to check. policy = spec.model.cohort_policy additives = set(spec.finer_grain.additive_columns) assert additives <= set(policy.derived), f"{spec.mv}: additive column is not a derived sum" diff --git a/tests/test_endpoints.py b/tests/test_endpoints.py index 966c013..2e1f946 100644 --- a/tests/test_endpoints.py +++ b/tests/test_endpoints.py @@ -454,17 +454,23 @@ async def test_org_trend_blanks_the_total_a_surviving_contract_row_withholds(app assert response.status_code == 200 (data,) = response.json()["data"] assert data["total_chatbot_interactions"] is None - # Only the column the contract grain actually withholds. The other totals - # are published in full downstream, so nothing can be subtracted out of them - # and blanking them would cost the dashboard data for no gain. + # Only the additive totals the contract grain actually withholds are + # blanked per-column. The other totals are published in full downstream, + # so nothing can be subtracted out of them and blanking them would cost + # the dashboard data for no gain. assert data["total_videos_watched"] == 500 assert data["total_problems_attempted"] == 7 assert data["new_enrollments"] == 12 - # Learner counts don't sum across contracts — a learner active under two is - # counted in both rows — so subtracting them bounds the withheld cohort - # rather than revealing it. - assert data["monthly_active_learners"] == 40 - assert data["chatbot_users"] == 15 + # But every cohort column goes wholesale for this month, chatbot_users + # included, even though that specific column isn't what triggered the + # guard: two contracts sharing no learners would sum exactly, and nothing + # here can distinguish that case from this one. + assert data["monthly_active_learners"] is None + assert data["chatbot_users"] is None + assert data["certified_learners"] is None + assert data["video_watchers"] is None + assert data["problem_attempters"] is None + assert data["enrolling_learners"] is None async def test_org_trend_blanks_every_total_when_a_contract_row_is_dropped(app): @@ -483,9 +489,32 @@ async def test_org_trend_blanks_every_total_when_a_contract_row_is_dropped(app): "total_videos_watched", "total_problems_attempted", "total_chatbot_interactions", + "monthly_active_learners", + "enrolling_learners", + "certified_learners", + "video_watchers", + "problem_attempters", + "chatbot_users", ): assert data[column] is None, column - assert data["monthly_active_learners"] == 40 + + +async def test_org_trend_blanks_the_headline_count_disjoint_contracts_would_reveal(app): + # The counterexample the guard closes: C1 (2 active, dropped) and C2 (25 + # active, published) share no learners, so the org total is their exact + # sum. Left alone, `27 - 25` would hand back C1's suppressed headcount + # exactly -- the failure mode `monthly_active_learners` staying published + # was supposed to be safe from, on the assumption contracts overlap. + org_row = _trend_row() | {"monthly_active_learners": 27} + finer = [ + _contract_trend_row("C1", active=2, chatbot_users=1, chatbot_total=3), + _contract_trend_row("C2", active=25, chatbot_users=20, chatbot_total=500), + ] + response = await _get_trend(app, _fake_fetch_all([org_row], finer_rows=finer)) + + assert response.status_code == 200 + (data,) = response.json()["data"] + assert data["monthly_active_learners"] is None async def test_org_trend_untouched_when_the_contract_grain_publishes_in_full(app):