From 7100f139b84a93864954a1e450c4a3c5de9a2a90 Mon Sep 17 00:00:00 2001 From: Tobias Macey Date: Tue, 22 Sep 2026 10:48:52 -0400 Subject: [PATCH 1/6] feat(b2b_dashboard): return per-status counts on the learner-progress envelope MIT Learn's learner directory renders four summary tiles (Enrollments, Not started, In progress, Completed) above the learner table. The other three came from three extra /learner-progress requests with limit=1 and a completion_status filter, each paying for an org-manager check, a page query and a full COUNT(*) scan regardless of limit. This adds completion_status_counts to LearnerProgressResponse, computed as conditional SUMs alongside the outcomes_withheld_count SUM already in the count query, so the breakdown costs no extra scan. not_started, in_progress, passed and certified come from mutually exclusive branches of _COMPLETION_STATUS, so the buckets never overlap, and consent-withheld rows land in none of them: the buckets plus outcomes_withheld_count sum to total_count. The counts share the response's own WHERE clause, so they narrow along with search/status/include_inactive filters rather than staying contract-wide. That's the cheaper option (the count query already has the clause) and keeps the envelope internally consistent; a contract-wide variant would need a second, unfiltered aggregate query. Fixes mitodl/ol-analytics-api#67. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01N1inM1db2mZzptMmC9VJfs --- .../tenants/b2b_dashboard/learner_models.py | 20 ++++++- .../tenants/b2b_dashboard/learner_queries.py | 11 +++- .../tenants/b2b_dashboard/routers/learners.py | 7 +++ tests/test_dashboard_learner_progress.py | 57 ++++++++++++++++++- 4 files changed, 91 insertions(+), 4 deletions(-) diff --git a/src/ol_analytics_api/tenants/b2b_dashboard/learner_models.py b/src/ol_analytics_api/tenants/b2b_dashboard/learner_models.py index b4ed010..21f44a1 100644 --- a/src/ol_analytics_api/tenants/b2b_dashboard/learner_models.py +++ b/src/ol_analytics_api/tenants/b2b_dashboard/learner_models.py @@ -25,7 +25,9 @@ - ``last_active_on`` is NULL for every row until activity data lands, whatever ``outcomes_shared`` says. - ``outcomes_withheld_count`` counts the rows in ``total_count`` whose - ``outcomes_shared`` is false. + ``outcomes_shared`` is false. ``completion_status_counts`` buckets the rest + by status; the two together add up to ``total_count``, since + ``CompletionStatus`` is exhaustive and its branches don't overlap. """ from __future__ import annotations @@ -149,6 +151,16 @@ def _gate_outcomes(self) -> Self: return self +class CompletionStatusCounts(BaseModel): + """Matches ``LearnerProgressResponse.total_count``'s own filters, not the + contract as a whole, so it narrows along with the table it summarizes.""" + + not_started: int = Field(description="Matching enrollments that haven't been started yet.") + in_progress: int = Field(description="Matching enrollments with a nonzero grade so far.") + passed: int = Field(description="Matching enrollments with a currently passing grade.") + certified: int = Field(description="Matching enrollments with an unrevoked certificate.") + + class LearnerProgressResponse(BaseModel): """The org envelope (``organization_id``, ``as_of``, ``total_count``, ``data``) plus ``outcomes_withheld_count``, so a client can show how many @@ -167,4 +179,10 @@ class LearnerProgressResponse(BaseModel): "agreed to share it." ) ) + completion_status_counts: CompletionStatusCounts = Field( + description=( + "How many of those enrollments are in each stage of completion. Enrollments with " + "hidden progress aren't counted in any stage." + ) + ) data: list[LearnerProgress] = Field(description="This page of enrollments.") diff --git a/src/ol_analytics_api/tenants/b2b_dashboard/learner_queries.py b/src/ol_analytics_api/tenants/b2b_dashboard/learner_queries.py index 2d5f083..cfd9592 100644 --- a/src/ol_analytics_api/tenants/b2b_dashboard/learner_queries.py +++ b/src/ol_analytics_api/tenants/b2b_dashboard/learner_queries.py @@ -45,6 +45,10 @@ " END" ) +# The four branches of _COMPLETION_STATUS. Mutually exclusive, so these buckets +# never overlap; a row with withheld outcomes falls into none of them. +_STATUSES = ("not_started", "in_progress", "passed", "certified") + _COLUMNS = ( "learner_id", "email", @@ -162,9 +166,14 @@ def learner_progress(filters: ProgressFilters) -> ProgressQuery: f"SELECT {projection} FROM ({records}) records{where}" # noqa: S608 f" ORDER BY {order_by} LIMIT %s OFFSET %s" ) + status_sums = ", ".join( + f"SUM(CASE WHEN {shared} AND completion_status = '{value}' THEN 1 ELSE 0 END) AS {value}" + for value in _STATUSES + ) count = ( "SELECT COUNT(*) AS total_count," # noqa: S608 - f" SUM(CASE WHEN {shared} THEN 0 ELSE 1 END) AS outcomes_withheld_count" + f" SUM(CASE WHEN {shared} THEN 0 ELSE 1 END) AS outcomes_withheld_count," + f" {status_sums}" f" FROM ({records}) records{where}" ) return ProgressQuery(page, count, tuple(params)) diff --git a/src/ol_analytics_api/tenants/b2b_dashboard/routers/learners.py b/src/ol_analytics_api/tenants/b2b_dashboard/routers/learners.py index 90ece14..9d0b197 100644 --- a/src/ol_analytics_api/tenants/b2b_dashboard/routers/learners.py +++ b/src/ol_analytics_api/tenants/b2b_dashboard/routers/learners.py @@ -28,6 +28,7 @@ ) from ol_analytics_api.tenants.b2b_dashboard.config import settings from ol_analytics_api.tenants.b2b_dashboard.learner_models import ( + CompletionStatusCounts, LearnerProgress, LearnerProgressResponse, ) @@ -103,5 +104,11 @@ async def learner_progress( # noqa: PLR0913 total_count=int(counts["total_count"]), # SUM over zero rows is NULL. outcomes_withheld_count=int(counts["outcomes_withheld_count"] or 0), + completion_status_counts=CompletionStatusCounts( + not_started=int(counts["not_started"] or 0), + in_progress=int(counts["in_progress"] or 0), + passed=int(counts["passed"] or 0), + certified=int(counts["certified"] or 0), + ), data=[LearnerProgress(**row) for row in rows], ) diff --git a/tests/test_dashboard_learner_progress.py b/tests/test_dashboard_learner_progress.py index 133eb78..e015e8a 100644 --- a/tests/test_dashboard_learner_progress.py +++ b/tests/test_dashboard_learner_progress.py @@ -63,9 +63,19 @@ class _FakePool: """Answers the as_of probe, the contract gate, the count query and the page query, recording every call.""" - def __init__(self, rows=(), total_count=0, withheld=0, *, contract_exists=True): + def __init__( + self, rows=(), total_count=0, withheld=0, status_counts=None, *, contract_exists=True + ): self.rows = list(rows) - self.counts = {"total_count": total_count, "outcomes_withheld_count": withheld} + self.counts = { + "total_count": total_count, + "outcomes_withheld_count": withheld, + "not_started": 0, + "in_progress": 0, + "passed": 0, + "certified": 0, + **(status_counts or {}), + } self.contract_exists = contract_exists self.calls = [] @@ -122,6 +132,12 @@ async def test_envelope_withholds_outcomes_and_counts_them(app): assert body["as_of"] == "2026-09-15T06:00:00Z" assert body["total_count"] == 12 assert body["outcomes_withheld_count"] == 12 + assert body["completion_status_counts"] == { + "not_started": 0, + "in_progress": 0, + "passed": 0, + "certified": 0, + } [row] = body["data"] assert row["enrolled_on"] == "2026-02-03T14:22:11Z" assert row["email"] == "rgarcia@contoso.example" @@ -172,6 +188,43 @@ async def test_consent_fail_open_discloses_outcomes(app, monkeypatch): assert "SUM(CASE WHEN TRUE THEN 0 ELSE 1 END)" in pool.count_call()[0] +async def test_completion_status_counts_reported_from_the_count_query(app): + pool = _FakePool( + total_count=10, + withheld=1, + status_counts={"not_started": 2, "in_progress": 3, "passed": 1, "certified": 4}, + ) + response = await _get(app, pool) + + assert response.json()["completion_status_counts"] == { + "not_started": 2, + "in_progress": 3, + "passed": 1, + "certified": 4, + } + + +async def test_completion_status_counts_share_the_response_filters(app): + pool = _FakePool() + await _get(app, pool, params={"completion_status": ["passed"]}) + count_query, _ = pool.count_call() + # Buckets come off the same WHERE clause as total_count, so they narrow + # along with the rest of the envelope rather than staying contract-wide. + assert count_query.count("WHERE") == 2 + for status in ("not_started", "in_progress", "passed", "certified"): + assert ( + f"SUM(CASE WHEN FALSE AND completion_status = '{status}' THEN 1 ELSE 0 END)" + f" AS {status}" in count_query + ) + + +def test_completion_status_buckets_are_mutually_exclusive_and_exhaustive(): + # Each row's completion_status is exactly one CASE branch + # (learner_queries._COMPLETION_STATUS), so the four buckets never overlap + # and, with outcomes_withheld_count, always sum to total_count. + assert learner_queries._STATUSES == ("not_started", "in_progress", "passed", "certified") # noqa: SLF001 + + async def test_search_is_bound_with_wildcards_escaped(app): pool = _FakePool() await _get(app, pool, params={"search": "Garcia_50%"}) From d13840b6a14f980b3360241c73a2b88eff71e64f Mon Sep 17 00:00:00 2001 From: Tobias Macey Date: Tue, 22 Sep 2026 10:53:37 -0400 Subject: [PATCH 2/6] test(b2b_dashboard): cover CompletionStatusCounts field descriptions Extends the manager-facing-description parametrization to the new CompletionStatusCounts model, so its fields are held to the same no-field-name-references constraint as LearnerProgress and LearnerProgressResponse instead of relying on manual inspection. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01N1inM1db2mZzptMmC9VJfs --- tests/test_dashboard_learner_progress.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/tests/test_dashboard_learner_progress.py b/tests/test_dashboard_learner_progress.py index e015e8a..2b5c29e 100644 --- a/tests/test_dashboard_learner_progress.py +++ b/tests/test_dashboard_learner_progress.py @@ -20,6 +20,7 @@ from ol_analytics_api.tenants.b2b_dashboard import learner_queries from ol_analytics_api.tenants.b2b_dashboard.config import settings from ol_analytics_api.tenants.b2b_dashboard.learner_models import ( + CompletionStatusCounts, LearnerProgress, LearnerProgressResponse, ) @@ -282,12 +283,18 @@ def test_every_outcome_column_is_consent_gated_in_the_query(): assert f"CASE WHEN FALSE THEN {name} END AS {name}" in query.page -@pytest.mark.parametrize("model", [LearnerProgress, LearnerProgressResponse]) +@pytest.mark.parametrize( + "model", [LearnerProgress, LearnerProgressResponse, CompletionStatusCounts] +) def test_every_field_has_a_manager_facing_description(model): # The dashboard can show these as help text to a manager, who never sees # field names, so every field needs one and none may lean on another field's # name. - field_names = set(LearnerProgress.model_fields) | set(LearnerProgressResponse.model_fields) + field_names = ( + set(LearnerProgress.model_fields) + | set(LearnerProgressResponse.model_fields) + | set(CompletionStatusCounts.model_fields) + ) for name, field in model.model_fields.items(): assert field.description, f"{name} has no description" named = set(re.findall(r"\b[a-z]+(?:_[a-z]+)+\b", field.description)) & field_names From fed73995fd4f435520352ef5bff53027ba335b41 Mon Sep 17 00:00:00 2001 From: Tobias Macey Date: Tue, 22 Sep 2026 12:44:00 -0400 Subject: [PATCH 3/6] fix(b2b_dashboard): document bucket precedence and fix a test invariant CompletionStatusCounts field descriptions read as if passed and in_progress could overlap with certified, but _COMPLETION_STATUS checks certificate status first: a certified enrollment counts only there even when it also has a passing grade. Each field's description now says what it excludes, and the fields are reordered to match the CASE precedence. test_completion_status_counts_reported_from_the_count_query used total_count=10 with buckets summing to 10 plus withheld=1, violating the buckets + outcomes_withheld_count == total_count invariant the endpoint promises. Derives total_count from the fixture values instead of a hardcoded one, and asserts the sum invariant directly. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01N1inM1db2mZzptMmC9VJfs --- .../tenants/b2b_dashboard/learner_models.py | 32 ++++++++++++++++--- tests/test_dashboard_learner_progress.py | 19 +++++------ 2 files changed, 37 insertions(+), 14 deletions(-) diff --git a/src/ol_analytics_api/tenants/b2b_dashboard/learner_models.py b/src/ol_analytics_api/tenants/b2b_dashboard/learner_models.py index 21f44a1..99650c3 100644 --- a/src/ol_analytics_api/tenants/b2b_dashboard/learner_models.py +++ b/src/ol_analytics_api/tenants/b2b_dashboard/learner_models.py @@ -153,12 +153,34 @@ def _gate_outcomes(self) -> Self: class CompletionStatusCounts(BaseModel): """Matches ``LearnerProgressResponse.total_count``'s own filters, not the - contract as a whole, so it narrows along with the table it summarizes.""" + contract as a whole, so it narrows along with the table it summarizes. - not_started: int = Field(description="Matching enrollments that haven't been started yet.") - in_progress: int = Field(description="Matching enrollments with a nonzero grade so far.") - passed: int = Field(description="Matching enrollments with a currently passing grade.") - certified: int = Field(description="Matching enrollments with an unrevoked certificate.") + Each field counts a disjoint slice of the matching enrollments: every + enrollment falls into exactly one, in the order below (certificate beats + grade beats no grade), so summing the four plus ``outcomes_withheld_count`` + always equals ``total_count``. An unrevoked certificate always wins even + when the same enrollment also carries a passing or in-progress grade, + which is why each field's own description calls out what it excludes. + """ + + certified: int = Field( + description="Matching enrollments with an unrevoked certificate, whatever their grade." + ) + passed: int = Field( + description=( + "Matching enrollments with a currently passing grade, other than those already " + "counted as certified above." + ) + ) + in_progress: int = Field( + description=( + "Matching enrollments with a nonzero grade so far that isn't yet passing, other " + "than those already counted as certified or passed above." + ) + ) + not_started: int = Field( + description="Matching enrollments with no certificate and no grade recorded yet." + ) class LearnerProgressResponse(BaseModel): diff --git a/tests/test_dashboard_learner_progress.py b/tests/test_dashboard_learner_progress.py index 2b5c29e..c20a16b 100644 --- a/tests/test_dashboard_learner_progress.py +++ b/tests/test_dashboard_learner_progress.py @@ -190,19 +190,20 @@ async def test_consent_fail_open_discloses_outcomes(app, monkeypatch): async def test_completion_status_counts_reported_from_the_count_query(app): + status_counts = {"not_started": 2, "in_progress": 3, "passed": 1, "certified": 4} + withheld = 1 pool = _FakePool( - total_count=10, - withheld=1, - status_counts={"not_started": 2, "in_progress": 3, "passed": 1, "certified": 4}, + # The buckets plus outcomes_withheld_count sum to total_count (11), the + # invariant the endpoint promises; keep this fixture consistent with it. + total_count=sum(status_counts.values()) + withheld, + withheld=withheld, + status_counts=status_counts, ) response = await _get(app, pool) - assert response.json()["completion_status_counts"] == { - "not_started": 2, - "in_progress": 3, - "passed": 1, - "certified": 4, - } + body = response.json() + assert body["completion_status_counts"] == status_counts + assert sum(status_counts.values()) + body["outcomes_withheld_count"] == body["total_count"] async def test_completion_status_counts_share_the_response_filters(app): From 63eb5e70a90df9056345bb1460675a956ada2557 Mon Sep 17 00:00:00 2001 From: Tobias Macey Date: Tue, 22 Sep 2026 14:22:28 -0400 Subject: [PATCH 4/6] chore(b2b_dashboard): regenerate OpenAPI spec after rebase main picked up openapi-spec-export (#70) after this branch forked, so the committed spec predates completion_status_counts. Regenerated with bin/generate-openapi-spec after rebasing onto main. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01N1inM1db2mZzptMmC9VJfs --- openapi/specs/b2b_dashboard.yaml | 51 ++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/openapi/specs/b2b_dashboard.yaml b/openapi/specs/b2b_dashboard.yaml index e03fb12..10f6452 100644 --- a/openapi/specs/b2b_dashboard.yaml +++ b/openapi/specs/b2b_dashboard.yaml @@ -634,6 +634,52 @@ components: on a schedule after grading, and audit-mode enrollments never certify.' + CompletionStatusCounts: + properties: + certified: + type: integer + title: Certified + description: Matching enrollments with an unrevoked certificate, whatever + their grade. + passed: + type: integer + title: Passed + description: Matching enrollments with a currently passing grade, other + than those already counted as certified above. + in_progress: + type: integer + title: In Progress + description: Matching enrollments with a nonzero grade so far that isn't + yet passing, other than those already counted as certified or passed above. + not_started: + type: integer + title: Not Started + description: Matching enrollments with no certificate and no grade recorded + yet. + type: object + required: + - certified + - passed + - in_progress + - not_started + title: CompletionStatusCounts + description: 'Matches ``LearnerProgressResponse.total_count``''s own filters, + not the + + contract as a whole, so it narrows along with the table it summarizes. + + + Each field counts a disjoint slice of the matching enrollments: every + + enrollment falls into exactly one, in the order below (certificate beats + + grade beats no grade), so summing the four plus ``outcomes_withheld_count`` + + always equals ``total_count``. An unrevoked certificate always wins even + + when the same enrollment also carries a passing or in-progress grade, + + which is why each field''s own description calls out what it excludes.' CompletionStatusFilter: type: string enum: @@ -1550,6 +1596,10 @@ components: title: Outcomes Withheld Count description: How many of those enrollments have progress hidden because the learner hasn't agreed to share it. + completion_status_counts: + $ref: '#/components/schemas/CompletionStatusCounts' + description: How many of those enrollments are in each stage of completion. + Enrollments with hidden progress aren't counted in any stage. data: items: $ref: '#/components/schemas/LearnerProgress' @@ -1562,6 +1612,7 @@ components: - as_of - total_count - outcomes_withheld_count + - completion_status_counts - data title: LearnerProgressResponse description: 'The org envelope (``organization_id``, ``as_of``, ``total_count``, From 3bc81c6b34c5a0d3a1efba0990f05d83a4fdd03d Mon Sep 17 00:00:00 2001 From: Tobias Macey Date: Fri, 25 Sep 2026 14:28:11 -0400 Subject: [PATCH 5/6] docs(b2b_dashboard): note why completion_status_counts skips cohort suppression CompletionStatusCounts is the only b2b_analytics aggregate that never applies a cohort_policy floor. Document that it's intentional, since the learner-progress endpoint already exposes the individual matching rows, so there's nothing left to hide by suppressing the summary. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01VfT3zzM5d6LQ5BQvkJhN5N --- .../tenants/b2b_dashboard/learner_models.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/ol_analytics_api/tenants/b2b_dashboard/learner_models.py b/src/ol_analytics_api/tenants/b2b_dashboard/learner_models.py index 99650c3..fb00fc5 100644 --- a/src/ol_analytics_api/tenants/b2b_dashboard/learner_models.py +++ b/src/ol_analytics_api/tenants/b2b_dashboard/learner_models.py @@ -161,6 +161,12 @@ class CompletionStatusCounts(BaseModel): always equals ``total_count``. An unrevoked certificate always wins even when the same enrollment also carries a passing or in-progress grade, which is why each field's own description calls out what it excludes. + + These counts are never suppressed for small cohorts, unlike the + ``cohort_policy``-gated aggregates elsewhere in b2b_analytics (e.g. + ``ContractUtilization``, ``EnrollmentCompletionFunnel``): this endpoint's + ``data`` already exposes the individual matching rows, so there's nothing + left to hide by suppressing the summary. """ certified: int = Field( From fcbcf4f13483ddfa381f79bea343899e22bada7f Mon Sep 17 00:00:00 2001 From: Tobias Macey Date: Fri, 25 Sep 2026 14:29:26 -0400 Subject: [PATCH 6/6] chore(b2b_dashboard): regenerate OpenAPI spec for docstring update Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01VfT3zzM5d6LQ5BQvkJhN5N --- openapi/specs/b2b_dashboard.yaml | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/openapi/specs/b2b_dashboard.yaml b/openapi/specs/b2b_dashboard.yaml index 10f6452..2091fd7 100644 --- a/openapi/specs/b2b_dashboard.yaml +++ b/openapi/specs/b2b_dashboard.yaml @@ -679,7 +679,18 @@ components: when the same enrollment also carries a passing or in-progress grade, - which is why each field''s own description calls out what it excludes.' + which is why each field''s own description calls out what it excludes. + + + These counts are never suppressed for small cohorts, unlike the + + ``cohort_policy``-gated aggregates elsewhere in b2b_analytics (e.g. + + ``ContractUtilization``, ``EnrollmentCompletionFunnel``): this endpoint''s + + ``data`` already exposes the individual matching rows, so there''s nothing + + left to hide by suppressing the summary.' CompletionStatusFilter: type: string enum: