Phase 2 complete — Cohort Dashboard + Aggregation: - Cohort aggregation pipeline (k-anon ≥10 write-time suppression, async hook, nightly 03:00 CT reconcile) - 4 auth-gated operator API endpoints (cohort, mastery, failure-patterns, credentials) - React cohort dashboard (BrowserRouter, login, 3 views, inline SVG sparklines, auth gate) - SPA fallback via SpaStaticFiles subclass (G-041 — NOT catch-all route) - G-038 differencing-attack test (unit + API e2e) - 317 pytest pass, 36 skip, 0 fail; 17/17 vitest pass; npm build + typecheck clean ---ci--- project: praxis phase: 2 milestone: v0.4 status: complete requirements: covered: [REQ-DASH-01, REQ-NFR-DASH-01, REQ-NFR-DASH-02, REQ-MT-02] partial: [] ---/ci---
36 KiB
Praxis — v0.4 Phase 2 Verification (Cohort Dashboard + Aggregation)
Summary
- Verdict: APPROVE_WITH_NOTES
- Layers: structural PASS, behavioral PASS, security PASS, quality PASS
- REQ coverage: 4/4 (REQ-DASH-01, REQ-NFR-DASH-01, REQ-NFR-DASH-02, REQ-MT-02 pipeline completion)
- Grill MUSTs honored: 2/2 (G-038 differencing-attack test, G-041 SPA fallback via custom StaticFiles subclass)
- P0 fixes applied: 0 (none needed — no P0 issues found)
- P1+ flagged: 4 (non-blocking, for post-hoc review in P3)
Phase 2 (P2) of the v0.4 milestone covers SLICE-07..10 (23 tasks): cohort aggregation pipeline, operator API endpoints, React cohort dashboard, and P2 integration. 4 commits since
milestone/v0.4-operator-tier:c396ded(SLICE-07),a7f7c4e(SLICE-08),d39bd14(SLICE-09),de2020e(SLICE-10).This report supersedes the prior TASK-10-05 verification matrix (preserved in §REQ-ID Coverage Matrix below).
Layer 1 — Structural
1.1 File existence (all P2 files present)
| File | Exists | LOC | Notes |
|---|---|---|---|
server/cohort/__init__.py |
YES | 0 | package marker |
server/cohort/aggregator.py |
YES | 230 | k-anon suppression, 7-day window, metric cells |
server/cohort/hook.py |
YES | 44 | fire-and-forget on_session_end, no-op if no Postgres |
server/cohort/nightly.py |
YES | 232 | NightlyScheduler, 03:00 CT, R-DASH-04 retry |
server/operator/__init__.py |
YES | 0 | package marker |
server/operator/_common.py |
YES | 93 | shared Cell/PathView/ViewResponse models, require_pg_store, all_recent_aggregates |
server/operator/cohort.py |
YES | 42 | GET /api/operator/cohort (practice volume) |
server/operator/mastery.py |
YES | 45 | GET /api/operator/mastery (mastery progression) |
server/operator/failure_patterns.py |
YES | 44 | GET /api/operator/failure-patterns |
server/operator/credentials.py |
YES | 78 | GET /api/operator/credentials + POST /{id}/revoke |
client/src/operator/Login.tsx |
YES | 93 | login form, 429 handling, keyboard-accessible |
client/src/operator/Dashboard.tsx |
YES | 120 | auth gate, 3 view tabs, freshness, logout |
client/src/operator/Sparkline.tsx |
YES | 49 | inline SVG polyline, zero deps |
client/src/operator/views/PracticeVolume.tsx |
YES | 81 | practice volume view + sparklines |
client/src/operator/views/MasteryProgression.tsx |
YES | 84 | mastery progression view |
client/src/operator/views/FailurePatterns.tsx |
YES | 94 | failure patterns view |
client/src/operator/views/_viewCommon.ts |
YES | 60 | shared Cell type, suppressedLabel, formatFreshness |
client/src/operator/__tests__/Dashboard.test.tsx |
YES | 193 | 17 vitest tests |
tests/test_cohort_aggregation.py |
YES | 246 | k-anon threshold, idempotency, G-038 |
tests/test_cohort_nightly.py |
YES | 199 | scheduler timing, R-DASH-04, reconcile |
tests/test_operator_endpoints.py |
YES | 304 | 401/200 auth, suppressed cells, revoke, R-DASH-02 |
tests/test_p2_aggregation_integration.py |
YES | 236 | e2e aggregation→endpoint (skips without Postgres) |
tests/test_p2_spa_fallback.py |
YES | 128 | 9 SPA fallback assertions (G-041) |
client/vitest.config.ts |
YES | 13 | vitest config |
client/src/App.tsx (extended) |
YES | 27 | BrowserRouter routes, voice UI at / unchanged |
client/src/VoiceSession.tsx |
YES | 177 | extracted voice session (unchanged behavior) |
server/session_recorder.py (extended) |
YES | +52 | aggregation hook chained, off voice path |
server/__main__.py (extended) |
YES | +61 | operator routers + SpaStaticFiles + nightly scheduler |
1.2 Imports resolve
python3 -c "import server.__main__"→ OK (server imports cleanly, logs "SPA fallback enabled")python3 -c "import server.cohort.aggregator, server.cohort.hook, server.cohort.nightly, server.operator.cohort, server.operator.mastery, server.operator.failure_patterns, server.operator.credentials"→ OK (all 7 new P2 modules import)
1.3 No stubs/TODOs in new P2 code
grep -r "TODO|FIXME|stub|placeholder|NotImplemented" server/cohort/ server/operator/→ No matches (zero stubs, zero TODOs in new P2 server code)
1.4 Deps + build
pip install -e . --break-system-packages→ OK (praxis-server 0.1.0 installed; P1 deps asyncpg/argon2-cffi/slowapi present)docker compose config→ OK (validates, praxis-data volume present)cd client && npm run build→ OK (vite v8.2.0, 168 modules, built in 547ms; bundle 662KB / 186KB gzip — within react-router-dom budget)cd client && npm run typecheck→ OK (tsc -b --noEmit, no errors)
1.5 Router mount order (critical for R-DASH-03)
Verified in server/__main__.py diff (lines 256-298):
app.include_router(auth_router)—/api/operator/login|logout|meapp.include_router(cohort_router)—/api/operator/cohortapp.include_router(mastery_router)—/api/operator/masteryapp.include_router(failure_router)—/api/operator/failure-patternsapp.include_router(credentials_router)—/api/operator/credentialsapp.mount("/", SpaStaticFiles(...), name="spa")— SPA fallback (AFTER all API routes)
Order is correct: API routes take precedence over the SPA fallback mount. R-DASH-03 verified.
Layer 1 verdict: PASS — all structural checks pass.
Layer 2 — Behavioral
2.1 Test results
| Suite | Result | Notes |
|---|---|---|
python3 -m pytest tests/ |
317 passed, 36 skipped, 0 failed | matches expected (Postgres-requiring tests skip gracefully — PRAXIS_PG_DSN unset) |
cd client && npx vitest run |
17/17 passed | Dashboard auth gate, login form (200/401/429), sparkline (empty/dot/polyline/flat), suppressedLabel, formatFreshness, no-PII-in-DOM |
cd client && npm run build |
PASS | 168 modules, 547ms |
cd client && npm run typecheck |
PASS | tsc clean |
P2-specific (test_p2_spa_fallback.py + test_operator_endpoints.py + test_cohort_aggregation.py + test_cohort_nightly.py) |
45/45 passed | full P2 unit + SPA fallback coverage |
test_p2_aggregation_integration.py |
3 skipped | gracefully skipped (no PRAXIS_PG_DSN) — e2e aggregation→endpoint path covered by unit tests with mocked PgStore |
2.2 P2 SLICE acceptance criteria
SLICE-07 (aggregation pipeline):
- ✅ k-anon threshold exactly 10 —
test_k_anon_threshold_at_10assertsK_ANON_THRESHOLD == 10;test_9_learners_suppressed(9 → suppressed),test_10_learners_not_suppressed(10 → not suppressed, value non-null),test_11_learners_not_suppressed(11 → not suppressed) - ✅ Idempotent upsert —
test_idempotent_same_session_twice(ON CONFLICT at DB layer) - ✅ 7-day window —
test_rolling_window_7_days(2026-08-04 → start=2026-07-29, 6-day span) - ✅ All metrics computed —
test_multiple_metrics_computed(sessions_count, active_learners_count, gate_open_rate, median_mastery_score, rubric_criterion_mean:, failure_mode:, branch:*) - ✅ No PII in upserts —
test_no_pii_in_upsert_calls(raw learner_ref not in any cell arg; cell_count is int) - ✅ Hook non-blocking —
server/cohort/hook.pyusesasyncio.create_taskinsession_recorder.py:161; hook swallows exceptions (test_hook_failure_logs_does_not_raise) - ✅ Hook no-op without Postgres —
test_hook_no_postgres_is_noop - ✅ Nightly scheduler timing —
test_seconds_until_next_03_ct_future_today+test_seconds_until_next_03_ct_past_today_wraps_tomorrow - ✅ R-DASH-04 nightly failure retry —
test_r_dash_04_nightly_failure_does_not_crash_scheduler - ✅ Nightly reconcile recomputes —
test_reconcile_recomputes_all_paths - ✅ Scheduler lifecycle —
test_scheduler_start_stop_lifecycle
SLICE-08 (operator API endpoints):
- ✅ All 4 endpoints auth-gated (401 without cookie) —
test_cohort_401_without_cookie,test_mastery_401_without_cookie,test_failure_patterns_401_without_cookie,test_credentials_401_without_cookie,test_revoke_401_without_cookie - ✅ All 4 endpoints 200 with cookie —
test_cohort_200_with_cookie,test_mastery_200_with_cookie,test_failure_patterns_200_with_cookie,test_credentials_200_with_cookie - ✅ Suppressed cells value=null —
test_suppressed_cells_value_null(cell_suppressed=true → value=null) - ✅ last_updated = max(updated_at) —
test_last_updated_is_max - ✅ Credential revoke —
test_credential_revoke_sets_status_revoked(status='revoked', set_credential_status awaited) +test_credential_revoke_404_unknown(404 for unknown) - ✅ No per-learner data (R-DASH-02) —
test_no_per_learner_data_in_cohort_response(no "learner-1", no "learner_ref" in response) - ✅ 503 when no Postgres —
test_cohort_503_no_postgres(graceful degradation)
SLICE-09 (React dashboard):
- ✅ react-router-dom@^7 added (
client/package.json) - ✅ BrowserRouter wrapper + route switch —
client/src/App.tsx:/→ VoiceSession (unchanged),/operator/login→ Login,/operator/dashboard→ Dashboard,*→ VoiceSession (fallback) - ✅ Login form — Login.tsx, 429 handling (
test shows rate-limit message on 429), keyboard-accessible (label associations) - ✅ Dashboard shell + auth gate — Dashboard.tsx, 401 on /me → redirect (
test redirects to /operator/login on 401), 3 view tabs, freshness indicator, logout - ✅ Inline SVG sparkline — Sparkline.tsx (49 LOC, zero deps), empty/dot/polyline/flat-line cases tested
- ✅ 3 view components — PracticeVolume, MasteryProgression, FailurePatterns (read-only, no drill-down)
- ✅ Suppressed cell display — "— (<10 learners)" (
suppressedLabeltest) - ✅ Freshness indicator — formatFreshness (m/h/d ago)
- ✅ No PII in DOM —
test does not render learner_ref fields
SLICE-10 (P2 integration):
- ✅ SPA fallback (G-041) — custom
SpaStaticFilessubclass in__main__.py:279-289, NOT a catch-all route; 9 assertions intest_p2_spa_fallback.pyall pass - ✅ Voice UI at
/unchanged (R-DASH-05) —test_root_serves_voice_ui(200, text/html,<div id="root">) - ✅ API routes return JSON not HTML —
test_api_operator_cohort_is_json_not_html,test_health_is_json,test_vc_verify_nonexistent_is_404 - ✅ Assets served by StaticFiles —
test_assets_served_by_staticfiles_not_spa_fallback(/assets/index.js→ javascript content-type, not index.html) - ✅ Nightly scheduler starts in lifespan —
server/__main__.py:116await nightly.start(app.state.pg_store); cancelled on shutdown (await nightly.stop()line 121) - ✅ E2e aggregation→endpoint —
test_p2_aggregation_integration.py::test_aggregation_to_endpoint_e2e(skips without Postgres; logic covered by unit tests with mocked store)
2.3 REQ coverage
| REQ-ID | Covered by | Status |
|---|---|---|
| REQ-DASH-01 (cohort dashboard, 3 views, k-anon, React under /operator/*) | SLICE-08 (4 endpoints), SLICE-09 (React UI), SLICE-10 (integration). test_operator_endpoints.py (all 4 endpoints 200/401), Dashboard.test.tsx (auth gate, login, 3 views), test_p2_spa_fallback.py (SPA serves /operator/*) |
COVERED |
| REQ-NFR-DASH-01 (k-anonymity ≥ 10) | SLICE-07 (write-time suppression in aggregator.py), SLICE-08 (query returns value=null for suppressed), SLICE-09 (display "— (<10 learners)"), SLICE-10 (e2e). test_cohort_aggregation.py (threshold at 10, 9/10/11 learners), test_operator_endpoints.py::test_suppressed_cells_value_null, Dashboard.test.tsx::suppressedLabel, G-038 differencing-attack |
COVERED |
| REQ-NFR-DASH-02 (freshness ≤ 24h) | SLICE-07 (nightly job + on-session-end hook), SLICE-10 (e2e). test_cohort_nightly.py (scheduler timing, reconcile, R-DASH-04), test_operator_endpoints.py::test_last_updated_is_max, test_p2_aggregation_integration.py::test_nightly_reconciliation_updates_last_updated (skips without Postgres) |
COVERED |
| REQ-MT-02 (pipeline completion — schema P1, pipeline P2) | SLICE-07 (aggregator + hook + nightly), SLICE-10 (e2e). test_cohort_aggregation.py (idempotent, multiple metrics, hook no-op/failure), test_cohort_nightly.py (reconcile), test_p2_aggregation_integration.py::test_aggregation_to_endpoint_e2e (skips without Postgres) |
COVERED |
4/4 P2 REQ-IDs covered.
2.4 Grill MUSTs honored
G-038 (differencing-attack test) — HONORED:
- Unit layer:
test_cohort_aggregation.py::test_g038_differencing_attack_cannot_isolate_dropped_learner— seeds 10 learners in window A, 9 in window B (learner-9 dropped), asserts window B is FULLY suppressed (value=NULL) so the dropped learner's contribution is not recoverable via subtraction. Verifies no per-learner ref leaks in either window's aggregate cells. - API e2e layer:
test_p2_aggregation_integration.py::test_g038_differencing_attack_api_layer— 10 learners on path diff_a, 9 on diff_b, asserts "a-9" not in response text and diff_b cells all suppressed with value=None. (Skips without Postgres — logic verified at unit layer.)
G-041 (SPA fallback via custom StaticFiles subclass) — HONORED:
- Implementation:
server/__main__.py:279-289definesclass SpaStaticFiles(StaticFiles)withget_responseoverride that returnsFileResponse("index/dist/index.html")only on 404 (non-file paths). This is the custom subclass approach mandated by G-041, NOT a@app.get("/{path:path}")catch-all (which would shadow asset serving per the grill's analysis). - Test:
test_p2_spa_fallback.py::test_assets_served_by_staticfiles_not_spa_fallbackverifies/assets/index.jsreturns javascript content (not index.html) — the critical assertion 8 from TASK-10-04.
2.5 Voice UI at / unchanged (R-DASH-03, R-DASH-05)
- Server:
SpaStaticFilesmount at/withhtml=Trueservesindex.htmlfor/(unchanged from v0.3 StaticFiles behavior). API routes registered before the mount take precedence.test_root_serves_voice_uiconfirms 200 + text/html +<div id="root">. - Client:
client/src/App.tsxroute/→<VoiceSession />(the existing voice session UI, extracted from the old App.tsx to VoiceSession.tsx — behavior unchanged). The*catch-all also serves VoiceSession (R-DASH-05: unknown routes fall back to learner surface, not a 404). - No regression: 317 passed, 0 failed — all v0.1/v0.2/v0.3 tests still pass.
Voice UI at / unchanged: CONFIRMED.
Layer 2 verdict: PASS — all behavioral checks pass.
Layer 3 — Security (STRIDE)
Spoofing
- Operator endpoints auth-gated via
current_operatordependency. - Verified: all 4 operator routers (
cohort.py,mastery.py,failure_patterns.py,credentials.py) importcurrent_operatorfromserver.auth.dependenciesand applyop: Operator = Depends(current_operator)on every endpoint. - Test coverage: 5 tests assert 401 without cookie (
test_cohort_401_without_cookie,test_mastery_401_without_cookie,test_failure_patterns_401_without_cookie,test_credentials_401_without_cookie,test_revoke_401_without_cookie). - Disposition: low (accept). No bypass path found — every
/api/operator/*route (except/loginwhich is rate-limited, not auth-gated) requires the dependency.
Tampering
- Aggregation pipeline — k-anon suppression at write time.
server/cohort/aggregator.py:87suppressed = active_count < K_ANON_THRESHOLD(K_ANON_THRESHOLD=10, module constant). Suppression applied beforeupsert_cohort_aggregate— value set toNonewhen suppressed (lines 90, 94, 103, etc.).- Nightly reconciliation (
nightly.py:127) re-applies the same threshold:suppressed = active_count < K_ANON_THRESHOLD. - Suppression cannot be bypassed via the API: endpoints read
cohort_aggregatesrows as-is (no post-processing that could un-suppress); suppressed cells havevalue=nullin the DB (enforced at write time). - Disposition: low (accept). Write-time suppression is server-side, not display-only.
Repudiation
- Credential revoke (POST /api/operator/credentials/{id}/revoke).
- The revoke endpoint sets
status='revoked'+revoked_at=now()in Postgres (pg_store.py:224extra = ", revoked_at = now()" if status == 'revoked'). Therevoked_attimestamp is an audit trail. - GAP (P1+ flagged): The revoke endpoint does NOT log the revocation event at the application level, and the
operator_idof the revoking operator is available viacurrent_operatorbut is NOT recorded against the credential revocation. Theissued_credentials.operator_idcolumn tracks the issuer, not the revoker. There is no revocation audit log linking operator→action→credential→timestamp. - Mitigation: the
revoked_attimestamp + the signed session cookie (which recordsoperator_idinrequest.session) provide a partial audit trail, but correlating them requires cross-referencing session logs. - Disposition: medium (mitigate — P1+ flagged). Add application-level logging of revocation events (operator_id, credential_id, timestamp) in P3.
Info Disclosure
- k-anonymity ≥ 10 enforced (REQ-NFR-DASH-01).
- Write-time suppression: cells with < 10 distinct learners →
cell_suppressed=TRUE,value=NULL. Verified bytest_9_learners_suppressed,test_10_learners_not_suppressed. - No per-learner drill-down (R-DASH-02): endpoints return only aggregate cells (path, metric, value, cell_count, cell_suppressed) — no
learner_refin cohort/mastery/failure responses. Verified bytest_no_per_learner_data_in_cohort_response(no "learner_ref" string, no "learner-1" in response). - G-038 differencing-attack defense: window B (9 learners) is fully suppressed (value=NULL), so subtracting B from A is not possible. Verified at unit + API layers.
- No PII in Postgres aggregates (D-031): only opaque
learner_reffor distinct counting, never stored in aggregate cells. Verified bytest_no_pii_in_upsert_calls. - Disposition: low (accept). k-anon defense-in-depth is sound; G-038 explicitly tested.
Denial of Service
- Aggregation hook is async fire-and-forget (non-blocking).
server/session_recorder.py:161asyncio.create_task(self._run_cohort_aggregation(session_outcome))— hook runs off the voice path (C-8, D-054). Voice loop latency unaffected.server/cohort/hook.py:37except Exception: log.exception(...)— hook failure does not propagate; nightly job reconciles.test_hook_failure_logs_does_not_raiseconfirms no exception propagation.- Nightly job doesn't block the event loop:
NightlyScheduler._run_loopusesasyncio.sleep(secs)(cooperative); reconciliation is a sequence ofawait pg_store.upsert_cohort_aggregate(...)calls (yields between each). - Disposition: low (accept). Hook failure → log + nightly reconcile (R-DASH-04). No crash path.
Elevation of Privilege
- Single operator role. No RBAC bypass.
- All 4 operator endpoints + credential management use
Depends(current_operator). Thecurrent_operatordependency (server/auth/dependencies.py) checksrequest.session["operator_id"]→ fetches operator → checksis_active=True→ returnsOperator. No role-based dispatch exists (single role). - The
current_operatordependency never trusts the client (D-057) — it validates the signed session cookie server-side. - Disposition: low (accept). No RBAC to bypass; single operator role; auth-gated everywhere.
Layer 3 verdict: PASS — all STRIDE categories low except Repudiation (medium, mitigated, P1+ flagged). No high-severity findings.
Layer 4 — Quality (multi-persona review)
Correctness
- k-anon threshold (exactly 10):
K_ANON_THRESHOLD = 10module constant; 9 → suppressed, 10 → not suppressed, 11 → not suppressed. Tests cover all three boundaries. ✅ - Aggregation idempotency: ON CONFLICT upsert at the DB layer (PgStore); hook is deterministic (same learner produces same distinct-count + counter state in cache).
test_idempotent_same_session_twicepasses. ✅ - Nightly scheduler timing:
seconds_until_next_03_ctcomputes seconds until 03:00 CT (fixed UTC-5 offset, documented DST approximation — acceptable for nightly reconciliation).test_seconds_until_next_03_ct_future_today+test_seconds_until_next_03_ct_past_today_wraps_tomorrowpass. ✅ - SPA fallback (G-041): Custom
SpaStaticFilessubclass, NOT catch-all route. Serves assets normally (JS/CSS), falls back to index.html only on 404.test_assets_served_by_staticfiles_not_spa_fallbackconfirms assets are not shadowed. ✅
Testing
- Coverage gaps: Postgres-requiring tests (
test_p2_aggregation_integration.py,test_pg_store.py) skip gracefully whenPRAXIS_PG_DSNunset — 36 skipped total, 0 failed. The e2e aggregation→endpoint→dashboard path is covered by unit tests with mocked PgStore (45/45 P2 tests pass). ✅ - Client tests (vitest): 17/17 pass — auth gate, login (200/401/429), sparkline (4 cases), suppressedLabel, formatFreshness, no-PII-in-DOM. ✅
- G-038 differencing-attack coverage: Unit layer (
test_g038_differencing_attack_cannot_isolate_dropped_learner) + API e2e layer (test_g038_differencing_attack_api_layer). The unit test is the primary proof (runs without Postgres); the e2e test is a bonus that skips without Postgres. ✅
Security
- SQL injection in PgStore queries: All queries use asyncpg parameterized placeholders (
$1,$2, etc.). Verified inpg_store.py(operator CRUD, cohort upsert, credential methods, gate events) andserver/operator/_common.py::all_recent_aggregates(WHERE window_start >= $1). One f-string interpolation inset_credential_status(f"UPDATE ... SET status = $1{extra} WHERE id = $2") — butextrais a hardcoded constant (, revoked_at = now()or empty) derived from thestatusvalue comparison, NOT user input. Safe. ✅ - k-anon suppression enforced server-side: Suppression is applied in
aggregator.py(write time) and re-applied innightly.py(reconcile). The API endpoints read cells as-is — no client-side or display-only suppression. ✅ - No PII in API responses: Cohort/mastery/failure endpoints return only (path, metric, value, cell_count, cell_suppressed, updated_at). Credentials endpoint returns (id, learner_ref, vc_type, status, issued_at, revoked_at) —
learner_refis an opaque string (D-031), not PII. ✅
Performance
- Aggregation hook non-blocking:
asyncio.create_taskinsession_recorder.py:161— fire-and-forget, off the voice path (C-8). ✅ - Nightly job doesn't block event loop:
asyncio.sleep(secs)+ sequentialawaitcalls (cooperative). Runs at 03:00 CT (low activity). ✅ - SPA fallback doesn't add latency to API routes: API routes are registered before the StaticFiles mount — FastAPI matches API routes first (no fallback overhead). ✅
Maintainability
- SpaStaticFiles subclass: Clean 11-line override (
get_responsecatches 404 → FileResponse). Well-commented with G-041 rationale. ✅ - 3 view components consistent: All 3 (PracticeVolume, MasteryProgression, FailurePatterns) share
_viewCommon.ts(Cell type, suppressedLabel, formatFreshness) and follow the same fetch→render pattern. ✅ - Router mounting order: API routes → SPA fallback mount. Documented in
__main__.py:256-298comments. ✅
Adversarial
- What if an attacker calls /api/operator/cohort with a path that doesn't exist? The endpoint takes no path parameter — it returns all paths' aggregates from the last 30 days. A non-existent path simply returns no rows (no error, no leak). ✅
- What if k-anon threshold is lowered via config?
K_ANON_THRESHOLD = 10is a module constant inaggregator.py, NOT configurable via env. Changing it requires a code change + redeploy. This is correct for a privacy control — it should not be runtime-configurable. ✅ - What if the aggregation hook runs before Postgres is healthy? The hook checks
pg_store is None→ no-op + WARNING (hook.py:27-32). If Postgres is unhealthy mid-session,upsert_cohort_aggregateraises → caught byhook.py:37except Exception: log.exception(...)→ nightly job reconciles. ✅
Layer 4 verdict: PASS — no quality issues found. Code is clean, well-commented, consistently structured, and adversarially sound.
P0 Fixes Applied
None. No P0 issues (broken tests, missing REQ coverage, security holes) were found. The P2 implementation is correct, complete, and secure.
P1+ Flagged for Post-Hoc Review
The following non-blocking issues are flagged for review in the final phase (P3):
P1+-01: Credential revocation lacks application-level audit log (Repudiation)
- File:
server/operator/credentials.py - Issue: The
revoke_credentialendpoint setsstatus='revoked'+revoked_at=now()in Postgres but does NOT log the revocation event at the application level, and the revokingoperator_id(available viacurrent_operator) is not recorded against the revocation action. Theissued_credentials.operator_idcolumn tracks the issuer, not the revoker. - Risk: An operator who revokes a credential leaves a DB timestamp but no application log linking who revoked which credential when. Correlating requires cross-referencing session logs.
- Mitigation present:
revoked_attimestamp in DB + signed session cookie (operator_id in session). - Recommended fix (P3): Add
log.info("credential revoked: operator=%s cred_id=%s", op.id, cred_id)inrevoke_credential, and consider anaudit_logtable orrevoked_by_operator_idcolumn onissued_credentials.
P1+-02: Nightly scheduler uses fixed UTC-5 offset (not true America/Winnipeg DST)
- File:
server/cohort/nightly.py:27CT = _dt.timezone(_dt.timedelta(hours=-5), "CT") - Issue: The CT timezone is approximated as a fixed UTC-5 offset. America/Winnipeg observes CST (UTC-6) in winter + CDT (UTC-5) in summer. The scheduler will drift by 1 hour across DST boundaries (the nightly job runs at 02:00 or 04:00 local instead of 03:00).
- Risk: Low — the nightly job runs once/day; a 1-hour drift is acceptable for a reconciliation job (on-session-end hook keeps data fresh ≤ 24h).
- Mitigation present: Documented in
nightly.py:36-41comments ("drift of ≤1h over DST boundaries is acceptable... a future hardening would use zoneinfo.ZoneInfo"). - Recommended fix (P3): Replace
CTconstant withzoneinfo.ZoneInfo("America/Winnipeg")for proper DST handling.
P1+-03: Aggregation in-memory cache is per-PgStore-instance (lost on restart)
- File:
server/cohort/aggregator.py:162-170_cache(pg_store) - Issue: The aggregator maintains a per-PgStore-instance in-memory cache (
_agg_cache) for running counters + distinct learner sets. On server restart, the cache is lost — the next on-session-end hook starts fresh, and the active_learners_count may reset to 1 (under-counting distinct learners until the nightly job reconciles frommastery_gate_events). - Risk: Low — the nightly job reconciles the true distinct count from the audit log (
mastery_gate_events). Between restart and nightly reconcile, cells may be incorrectly suppressed (under-count → over-suppression, which is privacy-safe but value-destroying). - Mitigation present: Nightly reconciliation recomputes from
mastery_gate_events(the source of truth). - Recommended fix (P3): Document that the in-memory cache is best-effort + nightly reconcile is authoritative, OR persist the distinct-learner set to Postgres (adds a table — may not be worth the complexity for pilot scale).
P1+-04: set_credential_status uses f-string interpolation in SQL (code smell, not vulnerability)
- File:
db/pg_store.py:227f"UPDATE issued_credentials SET status = $1{extra} WHERE id = $2" - Issue: The
extravariable (, revoked_at = now()or empty string) is interpolated via f-string into the SQL query. Whileextrais a hardcoded constant (not user input) andstatus/cred_idare parameterized, f-strings in SQL are a code smell that future maintainers might copy incorrectly. - Risk: None (current code is safe —
extrais derived fromstatus == "revoked"comparison, not user input). - Recommended fix (P3): Refactor to two explicit queries:
UPDATE ... SET status = $1 WHERE id = $2andUPDATE ... SET status = $1, revoked_at = now() WHERE id = $2, eliminating the f-string.
REQ-ID Coverage Matrix (from TASK-10-05, preserved)
REQ-DASH-01 — Cohort dashboard (3 views + auth gate)
| Test file | Test | What it verifies |
|---|---|---|
| tests/test_operator_endpoints.py | test_cohort_200_with_cookie | GET /api/operator/cohort returns practice volume |
| tests/test_operator_endpoints.py | test_mastery_200_with_cookie | GET /api/operator/mastery returns mastery progression |
| tests/test_operator_endpoints.py | test_failure_patterns_200_with_cookie | GET /api/operator/failure-patterns returns failure data |
| tests/test_operator_endpoints.py | test_credentials_200_with_cookie | GET /api/operator/credentials lists VCs |
| tests/test_operator_endpoints.py | test_cohort_401_without_cookie (+ 4 others) | All endpoints auth-gated (401) |
| client/src/operator/tests/Dashboard.test.tsx | Dashboard auth gate | React auth gate redirects on 401 from /me |
| client/src/operator/tests/Dashboard.test.tsx | Login form | POST /api/operator/login → dashboard |
| tests/test_p2_spa_fallback.py | test_operator_dashboard_spa_fallback | /operator/dashboard serves index.html (SPA) |
| tests/test_p2_spa_fallback.py | test_operator_login_spa_fallback | /operator/login serves index.html (SPA) |
REQ-NFR-DASH-01 — k-anonymity ≥ 10 (write-time suppression + query + display + e2e)
| Test file | Test | What it verifies |
|---|---|---|
| tests/test_cohort_aggregation.py | test_k_anon_threshold_at_10 | K_ANON_THRESHOLD == 10 |
| tests/test_cohort_aggregation.py | test_9_learners_suppressed | 9 learners → cell_suppressed=TRUE, value=NULL |
| tests/test_cohort_aggregation.py | test_10_learners_not_suppressed | 10 learners → non-suppressed, value non-null |
| tests/test_cohort_aggregation.py | test_11_learners_not_suppressed | 11 learners → non-suppressed |
| tests/test_cohort_aggregation.py | test_no_pii_in_upsert_calls | No raw learner_ref in aggregate cell args |
| tests/test_cohort_aggregation.py | test_g038_differencing_attack_cannot_isolate_dropped_learner | G-038: 10 in window A, 9 in B → dropped learner not isolatable |
| tests/test_operator_endpoints.py | test_suppressed_cells_value_null | API: suppressed cells have value=null |
| tests/test_operator_endpoints.py | test_no_per_learner_data_in_cohort_response | API: no per-learner data (R-DASH-02) |
| client/src/operator/tests/Dashboard.test.tsx | suppressedLabel | UI: suppressed cells render "— (<10 learners)" |
| tests/test_p2_aggregation_integration.py | test_aggregation_to_endpoint_e2e | E2e: 12 learners non-suppressed, 5 suppressed (skips without Postgres) |
| tests/test_p2_aggregation_integration.py | test_g038_differencing_attack_api_layer | G-038 e2e at API layer (skips without Postgres) |
REQ-NFR-DASH-02 — Freshness ≤ 24h (nightly job + on-session-end hook)
| Test file | Test | What it verifies |
|---|---|---|
| tests/test_cohort_nightly.py | test_seconds_until_next_03_ct_future_today | Scheduler computes correct seconds until 03:00 CT |
| tests/test_cohort_nightly.py | test_seconds_until_next_03_ct_past_today_wraps_tomorrow | Wraps to next day correctly |
| tests/test_cohort_nightly.py | test_reconcile_recomputes_all_paths | Nightly recomputes all (path, window) cells |
| tests/test_cohort_nightly.py | test_r_dash_04_nightly_failure_does_not_crash_scheduler | R-DASH-04: failure logs + retries |
| tests/test_cohort_nightly.py | test_scheduler_start_stop_lifecycle | Scheduler starts + stops cleanly |
| tests/test_operator_endpoints.py | test_last_updated_is_max | API: last_updated = max(updated_at) |
| tests/test_p2_aggregation_integration.py | test_nightly_reconciliation_updates_last_updated | E2e: nightly reconcile refreshes last_updated (skips without Postgres) |
| tests/test_p2_aggregation_integration.py | test_aggregation_to_endpoint_e2e (assertion 8) | E2e: last_updated ≤ 24h (skips without Postgres) |
REQ-MT-02 — Cohort aggregation pipeline (schema in P1, pipeline in P2)
| Test file | Test | What it verifies |
|---|---|---|
| tests/test_cohort_aggregation.py | test_multiple_metrics_computed | Pipeline computes all metric types |
| tests/test_cohort_aggregation.py | test_idempotent_same_session_twice | Idempotent upsert |
| tests/test_cohort_aggregation.py | test_rolling_window_7_days | 7-day rolling window computation |
| tests/test_cohort_aggregation.py | test_hook_no_postgres_is_noop | Graceful no-op without Postgres |
| tests/test_cohort_aggregation.py | test_hook_failure_logs_does_not_raise | Hook failure does not propagate |
| tests/test_cohort_nightly.py | test_reconcile_no_events_no_op | Nightly no-op when no events |
| tests/test_p2_aggregation_integration.py | test_aggregation_to_endpoint_e2e | Full pipeline e2e (skips without Postgres) |
G-038 (binding — differencing-attack test)
| Test file | Test | What it verifies |
|---|---|---|
| tests/test_cohort_aggregation.py | test_g038_differencing_attack_cannot_isolate_dropped_learner | Unit: 10 in A, 9 in B → B suppressed, dropped learner not isolatable |
| tests/test_p2_aggregation_integration.py | test_g038_differencing_attack_api_layer | E2e at API layer (skips without Postgres) |
G-041 (binding — SPA fallback via custom StaticFiles subclass)
| Test file | Test | What it verifies |
|---|---|---|
| tests/test_p2_spa_fallback.py | test_root_serves_voice_ui | Voice UI at / unchanged (R-DASH-05) |
| tests/test_p2_spa_fallback.py | test_operator_dashboard_spa_fallback | /operator/dashboard → index.html |
| tests/test_p2_spa_fallback.py | test_assets_served_by_staticfiles_not_spa_fallback | /assets/index.js served by StaticFiles (NOT catch-all) — G-041 critical assertion |
| tests/test_p2_spa_fallback.py | test_api_operator_cohort_is_json_not_html | API routes return JSON (not index.html) |
| tests/test_p2_spa_fallback.py | test_health_is_json | /health JSON |
R-DASH-05 (voice UI at / unchanged)
| Test file | Test | What it verifies |
|---|---|---|
| tests/test_p2_spa_fallback.py | test_root_serves_voice_ui | / → index.html with |
| client/src/operator/tests/Dashboard.test.tsx | (no PII in dashboard DOM) | Voice UI path unchanged |
Test Results Summary
| Suite | Pass | Skip | Fail |
|---|---|---|---|
python3 -m pytest tests/ (full) |
317 | 36 | 0 |
tests/test_p2_spa_fallback.py |
9 | 0 | 0 |
tests/test_operator_endpoints.py |
15 | 0 | 0 |
tests/test_cohort_aggregation.py |
12 | 0 | 0 |
tests/test_cohort_nightly.py |
9 | 0 | 0 |
tests/test_p2_aggregation_integration.py |
0 | 3 | 0 (Postgres-requiring, skip gracefully) |
cd client && npx vitest run |
17 | 0 | 0 |
cd client && npm run build |
PASS | — | — |
cd client && npm run typecheck |
PASS | — | — |
pip install -e . --break-system-packages |
PASS | — | — |
docker compose config |
PASS | — | — |
python3 -c "import server.__main__" |
PASS | — | — |
python3 -c "import ...all P2 modules" |
PASS | — | — |
Voice UI at / Unchanged — Confirmation
CONFIRMED. Three layers of evidence:
-
Server (
server/__main__.py): TheSpaStaticFilesmount at/withhtml=Trueservesindex.htmlfor/— identical to the v0.3StaticFilesbehavior. The custom subclass only changes behavior for non-file paths (404 → index.html), not for/(which StaticFiles already serves as index.html withhtml=True).test_root_serves_voice_uiconfirms 200 + text/html +<div id="root">. -
Client (
client/src/App.tsx): Route/→<VoiceSession />. The VoiceSession component was extracted from the old App.tsx (behavior unchanged — same voice session UI). The*catch-all also serves VoiceSession (R-DASH-05: unknown routes fall back to learner surface). -
Test suite: 317 passed, 0 failed — all v0.1/v0.2/v0.3 tests (voice loop, WebRTC, scenarios, mastery, VC) still pass. No regression in the learner surface.
Bottom Line
Phase 2 (Cohort Dashboard + Aggregation) is APPROVE_WITH_NOTES. All 4 layers pass. All 4 P2 REQ-IDs are covered. Both grill MUSTs (G-038 differencing-attack test, G-041 SPA fallback via custom StaticFiles subclass) are honored. Zero P0 issues. Four P1+ issues flagged for post-hoc review in P3 (credential revocation audit log, nightly scheduler DST, in-memory cache persistence, f-string SQL code smell) — all non-blocking, all with mitigations present.
The P2 implementation is shippable as v0.1.8 pending the final P3 review + ship phase.