feat(b2b_dashboard): distinct-learner needs-attention count per contract

[daniellefrappier18]

What are the relevant tickets?

N/A. Tracked as witan tk-add-a-distinct-learner-needs-attention-count-to--4a57a6; unblocks the Needs attention KPI tile in mit-learn.

Description (What does it do?)

Adds two endpoints returning how many distinct learners on a contract need attention:

• GET /organizations/{org}/needs-attention — one row per contract
• GET /organizations/{org}/contracts/{id}/needs-attention — the single row

This backs a fourth KPI tile beside Seat utilization, Active learners and Completion rate. learner-progress's existing needs_attention_count can't: it is a SUM over a learner x course-run grain, so a learner behind in three courses counts three times, sitting next to active_learners, which is a distinct-learner count.

Computed in this service from learner_queries._needs_attention rather than as a column on mv_b2b_contract_utilization. The task left that choice open. A dbt column would restate the 30-day rule and the completion-status CASE in a second repo, and freeze the cutoff at MV-refresh time, so the tile and the learner directory could disagree about the same learner.

Implementation details

• Row: contract_id, learners_considered (primary cohort, gates the row), learners_needing_attention and learners_outcomes_withheld (secondary, floored).
• Scoped to active enrollments with no caller override, matching learner_progress's include_inactive=false default, so the tile and the drill-down count one population.
• Consent gates the two outcome counts but not learners_considered — being enrolled is not an outcome. The three deliberately do not sum.
• Its own endpoint rather than a field on contract-utilization: different MV, own refresh schedule. as_of comes from mv_b2b_learner_enrollment.
• Cutoff resolved once per request via NEEDS_ATTENTION_CUTOFF_QUERY and never cached, as in routers/learners.py.
• Unlike the rest of learner_queries.py this goes through fetch_and_suppress, since it returns no learner rows. Module docstring amended to say so.
• ContractNeedsAttention is not in test_column_contract.py's cases (those are built from build_select), so its two model-level governance checks are re-asserted in the new test file.

How can this be tested?

uv run pytest — 348 pass. uv run mypy src and all pre-commit hooks clean.

The seven sqlite tests execute the production query strings verbatim: the in-memory databases are ATTACHed under the schema name the query itself names, so only %s -> ? is translated. The main one asserts the distinct-learner count (2) against the old enrollment-grain SUM (4) over the same six enrollments, since the change of units is the point and no assertion on SQL text can show it.

Not exercised against a live StarRocks cluster. COUNT(DISTINCT CASE WHEN ... THEN learner_id END) and the count query's HAVING COUNT(DISTINCT ...) parse in sqlite but are unverified on StarRocks — worth a QA smoke check.

Additional Context

Merge-order note with #36 (complement suppression). Once that lands, this model's two secondary cohorts need a contained_in declaration or _validate_containment raises at import. CI catches it on rebase, and the exact two lines are in a comment on #36.

🤖 Generated with Claude Code

[github-actions]

OpenAPI Changes

Show/hide changes

## Changes for b2b_dashboard.yaml:
2 changes: 0 error, 0 warning, 2 info
info	[endpoint-added] at head/openapi/specs/b2b_dashboard.yaml
	in API GET /api/v1/analytics/organizations/{organization_id}/contracts/{contract_id}/needs-attention
		endpoint added

info	[endpoint-added] at head/openapi/specs/b2b_dashboard.yaml
	in API GET /api/v1/analytics/organizations/{organization_id}/needs-attention
		endpoint added



## Changes for b2b_learner_records.yaml:
No changes detected

Unexpected changes? Ensure your branch is up-to-date with main (consider rebasing).

[Copilot]

Copilot review overview

🟡 Changes recommended

Pagination currently exposes suppressed-row positioning, and complement suppression must be applied before release.

Review effort: Balanced
Findings: 1 · 1

Open (2)

• Prevent secondary suppression from exposing complementary sub-floor cohorts · New
• Apply suppression before pagination to prevent empty pages and count leakage · New

What changed in this PR

Adds contract-level distinct-learner needs-attention metrics for the B2B dashboard.

Changes:

• Adds organization- and contract-scoped endpoints.
• Implements distinct-learner aggregation, consent gating, and suppression.
• Adds OpenAPI schemas and comprehensive tests.

File	Description
tests/​test_openapi_spec.py	Validates the response schema.
tests/​test_dashboard_needs_attention.py	Tests aggregation, authorization, suppression, and routing.
src/​ol_analytics_api/​tenants/​b2b_dashboard/​routers/​needs_attention.py	Implements both endpoints.
src/​ol_analytics_api/​tenants/​b2b_dashboard/​models.py	Defines the response model and cohort policy.
src/​ol_analytics_api/​tenants/​b2b_dashboard/​learner_queries.py	Builds distinct-learner aggregate queries.
src/​ol_analytics_api/​tenants/​b2b_dashboard/​app.py	Registers the new router.
openapi/​specs/​b2b_dashboard.yaml	Publishes the endpoint contracts.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[daniellefrappier18]

The complement analysis is correct, and it's already tracked — but I'm not taking the "should not ship until" conclusion, for three reasons.

1. contained_in does not exist on main. It arrives with #36. There is no way to declare containment on this branch; the parameter isn't there. So the gate you're proposing can't be satisfied in this PR by any amount of work — only by blocking on #36, which was opened 2026-08-20, last touched 2026-08-31, and still needs a rebase and a first review.

2. This is an instance of a tenant-wide gap, not a new one. ContractUtilization already ships seats_consumed: 42 beside active_learners: 40, naming 2 inactive learners. Six of the seven existing models have the same shape somewhere — that's precisely the gap #36 exists to close, and its own table enumerates them. Holding this one endpoint doesn't make the tenant safer; it singles out the newest instance of a disclosure already live on every other panel.

3. The remediation is pre-derived and CI-enforced. I verified against #36's actual CohortPolicy that this model's policy raises on merge:

ValueError: Secondary cohorts ['learners_needing_attention', 'learners_outcomes_withheld']
are classified neither by contained_in nor by uncontained.

models.py is imported by both mypy src and pytest, so whichever PR rebases second goes red before merge — it cannot reach main unnoticed. The declaration is posted on #36 (#36 (comment)):

contained_in={
    "learners_needing_attention": "learners_considered",
    "learners_outcomes_withheld": "learners_considered",
}

I also confirmed it's semantically right, not just enough to satisfy the check: all three are COUNT(DISTINCT ...) over the identical row set of one subquery, so each is a strict subset of the denominator by construction. With it applied, suppress_small_cohorts nulls learners_needing_attention on a 42/40 row, so the rule does fire on these columns.

One correction to your second example. You're right that publishing learners_outcomes_withheld: 40 of 42 identifies the 2 who shared. But "the org can see who declined" was explicitly ruled out of scope for this service by the maintainer on 2026-09-11 — it's a contractual control, not an API one, on the grounds that an org can diff its own roster regardless of response shape. That was decided for the sibling b2b_learner_records tenant rather than this one, so it's precedent rather than binding here, and worth confirming. It does mean that half of this finding may be a settled non-defect rather than something to fix.

Fixed your other comment in 4e95e64 — that one was a live bug.

[daniellefrappier18]

Confirmed and fixed in 4e95e64. This was a real bug, not just a theoretical probe.

Reproduced before the fix — two sub-floor contracts (2 learners each) ordered ahead of one visible contract (6 learners), limit=2, offset=0:

raw page (limit=2, offset=0): [101, 102]
after Python suppression:     []
total_count reported:         1

So the client got data: [] beside total_count: 1, and contract 103 was reachable only by guessing offset=2. That also breaks what OrgAnalyticsResponse's docstring instructs clients to do — compare total_count against len(data) plus offset.

After the fix:

first page (limit=2, offset=0): [103]
total_count:                    1
offset=1 ->                     []

Implementation follows your suggestion: the gate is written once as HAVING COUNT(DISTINCT learner_id) >= %s and spliced into both the page and the count, so they can't drift. The floor is bound twice per request — once into the page's HAVING, once for fetch_and_suppress, which still nulls sub-floor secondary counts within a surviving row. Regression test is test_page_and_count_describe_the_same_gated_set.

Worth flagging that this is not specific to this endpoint. build_select emits no cohort gate while build_count does, so all five MV-backed endpoints plus the admin one have the identical mismatch:

build_select gates on the primary cohort?  False
build_count  gates on the primary cohort?  True

That one needs a signature change on shared machinery and touches three routers plus test_column_contract.py, so it's out of scope here and tracked separately as tk-build-select-pages-ungated-while-build-count-gat-8e6b21.