praxis/.ciagent/VERIFY-P2.md

# 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):
1. `app.include_router(auth_router)` — `/api/operator/login|logout|me`
2. `app.include_router(cohort_router)` — `/api/operator/cohort`
3. `app.include_router(mastery_router)` — `/api/operator/mastery`
4. `app.include_router(failure_router)` — `/api/operator/failure-patterns`
5. `app.include_router(credentials_router)` — `/api/operator/credentials`
6. `app.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_10` asserts `K_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.py` uses `asyncio.create_task` in `session_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)" (`suppressedLabel` test)
- ✅ 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 `SpaStaticFiles` subclass in `__main__.py:279-289`, NOT a catch-all route; 9 assertions in `test_p2_spa_fallback.py` all 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:116` `await 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-289` defines `class SpaStaticFiles(StaticFiles)` with `get_response` override that returns `FileResponse("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_fallback` verifies `/assets/index.js` returns 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**: `SpaStaticFiles` mount at `/` with `html=True` serves `index.html` for `/` (unchanged from v0.3 StaticFiles behavior). API routes registered before the mount take precedence. `test_root_serves_voice_ui` confirms 200 + text/html + `<div id="root">`.
- **Client**: `client/src/App.tsx` route `/` → `<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_operator` dependency.**
- Verified: all 4 operator routers (`cohort.py`, `mastery.py`, `failure_patterns.py`, `credentials.py`) import `current_operator` from `server.auth.dependencies` and apply `op: 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 `/login` which is rate-limited, not auth-gated) requires the dependency.

### Tampering
- **Aggregation pipeline — k-anon suppression at write time.**
- `server/cohort/aggregator.py:87` `suppressed = active_count < K_ANON_THRESHOLD` (K_ANON_THRESHOLD=10, module constant). Suppression applied before `upsert_cohort_aggregate` — value set to `None` when 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_aggregates` rows as-is (no post-processing that could un-suppress); suppressed cells have `value=null` in 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:224` `extra = ", revoked_at = now()" if status == 'revoked'`). The `revoked_at` timestamp is an audit trail.
- **GAP (P1+ flagged)**: The revoke endpoint does NOT log the revocation event at the application level, and the `operator_id` of the revoking operator is available via `current_operator` but is NOT recorded against the credential revocation. The `issued_credentials.operator_id` column tracks the *issuer*, not the *revoker*. There is no revocation audit log linking operator→action→credential→timestamp.
- Mitigation: the `revoked_at` timestamp + the signed session cookie (which records `operator_id` in `request.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 by `test_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_ref` in cohort/mastery/failure responses. Verified by `test_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_ref` for distinct counting, never stored in aggregate cells. Verified by `test_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:161` `asyncio.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:37` `except Exception: log.exception(...)` — hook failure does not propagate; nightly job reconciles.
- `test_hook_failure_logs_does_not_raise` confirms no exception propagation.
- Nightly job doesn't block the event loop: `NightlyScheduler._run_loop` uses `asyncio.sleep(secs)` (cooperative); reconciliation is a sequence of `await 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)`. The `current_operator` dependency (`server/auth/dependencies.py`) checks `request.session["operator_id"]` → fetches operator → checks `is_active=True` → returns `Operator`. No role-based dispatch exists (single role).
- The `current_operator` dependency 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 = 10` module 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_twice` passes. ✅
- **Nightly scheduler timing:** `seconds_until_next_03_ct` computes 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_tomorrow` pass. ✅
- **SPA fallback (G-041):** Custom `SpaStaticFiles` subclass, NOT catch-all route. Serves assets normally (JS/CSS), falls back to index.html only on 404. `test_assets_served_by_staticfiles_not_spa_fallback` confirms assets are not shadowed. ✅

### Testing
- **Coverage gaps:** Postgres-requiring tests (`test_p2_aggregation_integration.py`, `test_pg_store.py`) skip gracefully when `PRAXIS_PG_DSN` unset — 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 in `pg_store.py` (operator CRUD, cohort upsert, credential methods, gate events) and `server/operator/_common.py::all_recent_aggregates` (`WHERE window_start >= $1`). One f-string interpolation in `set_credential_status` (`f"UPDATE ... SET status = $1{extra} WHERE id = $2"`) — but `extra` is a hardcoded constant (`, revoked_at = now()` or empty) derived from the `status` value comparison, NOT user input. Safe. ✅
- **k-anon suppression enforced server-side:** Suppression is applied in `aggregator.py` (write time) and re-applied in `nightly.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_ref` is an opaque string (D-031), not PII. ✅

### Performance
- **Aggregation hook non-blocking:** `asyncio.create_task` in `session_recorder.py:161` — fire-and-forget, off the voice path (C-8). ✅
- **Nightly job doesn't block event loop:** `asyncio.sleep(secs)` + sequential `await` calls (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_response` catches 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-298` comments. ✅

### 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 = 10` is a module constant in `aggregator.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_aggregate` raises → caught by `hook.py:37` `except 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_credential` endpoint sets `status='revoked'` + `revoked_at=now()` in Postgres but does NOT log the revocation event at the application level, and the revoking `operator_id` (available via `current_operator`) is not recorded against the revocation action. The `issued_credentials.operator_id` column 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_at` timestamp 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)` in `revoke_credential`, and consider an `audit_log` table or `revoked_by_operator_id` column on `issued_credentials`.

### P1+-02: Nightly scheduler uses fixed UTC-5 offset (not true America/Winnipeg DST)
- **File:** `server/cohort/nightly.py:27` `CT = _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-41` comments ("drift of ≤1h over DST boundaries is acceptable... a future hardening would use zoneinfo.ZoneInfo").
- **Recommended fix (P3):** Replace `CT` constant with `zoneinfo.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 from `mastery_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:227` `f"UPDATE issued_credentials SET status = $1{extra} WHERE id = $2"`
- **Issue:** The `extra` variable (`, revoked_at = now()` or empty string) is interpolated via f-string into the SQL query. While `extra` is a hardcoded constant (not user input) and `status`/`cred_id` are parameterized, f-strings in SQL are a code smell that future maintainers might copy incorrectly.
- **Risk:** None (current code is safe — `extra` is derived from `status == "revoked"` comparison, not user input).
- **Recommended fix (P3):** Refactor to two explicit queries: `UPDATE ... SET status = $1 WHERE id = $2` and `UPDATE ... 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 <div id="root"> |
| 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:

1. **Server (`server/__main__.py`):** The `SpaStaticFiles` mount at `/` with `html=True` serves `index.html` for `/` — identical to the v0.3 `StaticFiles` behavior. The custom subclass only changes behavior for *non-file* paths (404 → index.html), not for `/` (which StaticFiles already serves as index.html with `html=True`). `test_root_serves_voice_ui` confirms 200 + text/html + `<div id="root">`.

2. **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).

3. **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.