# Sprint: Question picker — restrict to qa_status = 'approved'

Branch off master. Single PR.

## Background

Audit revealed that question pickers across diagnostic, kiasu, and track modes filter on `questions.status_id = 3` (active) but not consistently on `questions.qa_status = 'approved'`. Unapproved questions (`unreviewed`, `flagged`, `needs_revision`, `ai_generated`) can leak into user-facing assessments via fallback branches that loosen filters to satisfy the "always return a question" invariant.

**Rule:** every question served to a user — diagnostic, kiasu, or track — must have `qa_status = 'approved'`. Fallbacks that loosen this filter are forbidden. If no approved question exists at the target level, fall back to approved questions at adjacent levels, NOT to unapproved questions.

The `qa_status` enum values are: `unreviewed`, `approved`, `flagged`, `needs_revision`, `ai_generated`. Only `approved` qualifies.

## Autonomy directive

Work without asking questions. Document decisions in the PR description.

## Tasks

### Task 1: Inventory all question-pick paths

Grep the codebase for every place a question is selected for a user. Search patterns:

```bash
grep -rn "Question::" app/ --include="*.php" | grep -E "(where|whereHas|first|get|inRandomOrder)"
grep -rn "is_diagnostic" app/ --include="*.php"
grep -rn "getQuestionAtLevel" app/ --include="*.php"
grep -rn "getKiasuPathQuestions" app/ --include="*.php"
grep -rn "uncompletedQuestions" app/ --include="*.php"
```

Produce a list in the PR description: file, method, line numbers, what mode it serves.

### Task 2: Add qa_status filter to each picker

For each pick path identified in Task 1, add:

```php
->where('qa_status', 'approved')
```

to the question query. Apply consistently across:

**a. DiagnosticService::getNextQuestionBatch** (`app/Services/DiagnosticService.php`)

The main `Question::where('is_diagnostic', true)->whereHas(...)` query AND any fallback branches must filter on `qa_status = 'approved'`.

**b. DiagnosticController::getNextQuestionBatch** (`app/Http/Controllers/DiagnosticController.php`)

If this controller method still has its own picker (vs delegating to DiagnosticService), apply the same filter to the main query and the fallback. The `getNextQuestionBatch` method in the controller has at least two branches per the 13 May review:
- Targeted level query via levels join
- Last-resort fallback: `Question::whereHas('skill.tracks', ...)->where('is_diagnostic', true)->inRandomOrder()->first()`

Both branches need the filter.

**c. AdaptiveLevelService::getQuestionAtLevel** (`app/Services/AdaptiveLevelService.php`)

If this is the centralized picker called from multiple places, add the filter here once and remove redundant filters from callers if they exist. Verify by grep that all callers route through this method; if not, leave caller-side filters in place.

**d. KiasuPathService::getKiasuPathQuestions** (`app/Services/KiasuPathService.php`)

The weighted-selection query and all fallback levels (target maxile → step up → any unpassed track → any question) all need `qa_status = 'approved'`.

**e. Test::firstOrCreateTrackPractice** (or wherever track-practice question selection lives)

The untested → wrong-answered → any-pool cascade per spec §5.1 — every step filters on approved.

### Task 3: Centralize via a query scope

Add a local scope on the `Question` model:

```php
// app/Models/Question.php

public function scopeApproved($query)
{
    return $query->where('qa_status', 'approved');
}
```

Then replace each `->where('qa_status', 'approved')` call from Task 2 with `->approved()`. Reads better, single source of truth, easier to audit later.

Add a corresponding test query: `Question::approved()->where('status_id', 3)->count()` should match the count of currently-approved-and-active questions.

### Task 4: Update the fallback invariant

Per spec §5.1, the picker has a fallback cascade ending in "any question." That fallback must now end in "any approved question." If the question bank has zero approved questions for a given field, the picker should return null (NOT an unapproved question), and the caller logs a `WARNING` for question-bank gap analysis. Update the spec doc:

In `docs/algorithms/maxile-and-question-selection.md` §5.1, after the existing fallback cascades, add:

```
### 5.1.1 Approval gate (overrides every cascade)

Every fallback level filters on `questions.qa_status = 'approved'`.
A picker returning a question with any other qa_status is a bug.

If the deepest fallback exhausts and no approved question exists, the
picker returns null and logs WARNING with: session_id, field_id (if
applicable), target_level (if applicable), reason. The caller decides
whether to terminate the session, skip the field, or surface an error
to the user.
```

Add a change-log entry to §9:

```
| 13 May 2026 | Pam + Claude | §5.1.1 added — approval gate (qa_status = 'approved') overrides every fallback in every picker. No question is served to a user unless approved. |
```

### Task 5: Verify with SQL

In the PR description, paste outputs of:

```sql
-- Count of approved-and-active questions per field
SELECT 
  f.id AS field_id, 
  f.field, 
  COUNT(*) AS approved_questions
FROM questions q
INNER JOIN skill_track st ON q.skill_id = st.skill_id
INNER JOIN tracks t ON st.track_id = t.id
INNER JOIN fields f ON t.field_id = f.id
WHERE q.qa_status = 'approved'
  AND q.status_id = 3
GROUP BY f.id, f.field
ORDER BY approved_questions ASC;

-- Approved questions per level per field (for diagnostic walk feasibility)
SELECT 
  f.field, 
  l.start_maxile_level, 
  COUNT(*) AS approved_questions
FROM questions q
INNER JOIN skill_track st ON q.skill_id = st.skill_id
INNER JOIN tracks t ON st.track_id = t.id
INNER JOIN levels l ON t.level_id = l.id
INNER JOIN fields f ON t.field_id = f.id
WHERE q.qa_status = 'approved'
  AND q.status_id = 3
  AND q.is_diagnostic = 1
GROUP BY f.field, l.start_maxile_level
ORDER BY f.field, l.start_maxile_level;
```

Flag any field with `approved_questions < 10` and any (field, level) pair with `approved_questions < 2` as a content gap — these are where the picker will hit the null-return case. Add to PR description.

### Task 6: Smoke test

From tinker:

```php
// Pick the field with the most approved questions
$field = ... // top of the count query

// Run a diagnostic session for a fresh test user
$user = User::find($testUserId);
$session = AssessmentSession::create([...]);
$diagService = app(DiagnosticService::class);

// Loop until session completes, asserting each served question is approved
while (!$session->fresh()->isCompleted()) {
    $batch = $diagService->getNextQuestionBatch($session);
    foreach ($batch as $q) {
        $dbQ = Question::find($q['id']);
        assert($dbQ->qa_status === 'approved', "Served question {$q['id']} is {$dbQ->qa_status}, not approved");
    }
    // Submit a correct answer to advance
    $diagService->submitAnswer($session, $dbQ, $dbQ->correct_answer, 1);
}
```

Paste a trace into the PR description showing zero assertion failures.

### Task 7: Flutter side — verify no leak via direct API

Quick grep in the Flutter app for any direct query against `/api/questions` that bypasses the diagnostic/kiasu/track endpoints. Most likely there isn't one — Flutter calls `/api/diagnostic/next`, `/api/kiasu/next`, etc., which all go through the pickers fixed above. If you find a direct `/api/questions` consumer in Flutter, flag it in PR description as out-of-scope follow-up.

## PR description requirements

- Branch name: `fix/picker-approved-only`
- Title: `Restrict every question picker to qa_status = approved`
- Body:
  - Task 1 inventory (every pick path identified)
  - Task 5 SQL output (counts per field, counts per field-level pair, content gaps flagged)
  - Task 6 trace (smoke test showing no leaks)
  - Confirmation that spec doc was patched
  - Any decisions made under autonomy directive

## Out of scope

- The QA workflow itself (how questions get to `approved` — that's admin UI, separate concern)
- AI-generated questions auto-flagging logic (post-beta)
- Backfilling `qa_status = 'unreviewed'` questions to `'approved'` — that's a content review, not an engineering task. The audit will surface what fraction of the bank is unapproved; product decides whether to bulk-review or accept the smaller pool.