[longobucco]

What changed and why

Quiz generation previously made a single schema-validated `/generate_json` call: it guaranteed
well-formed questions, not good ones (guessable distractors, general-knowledge questions,
multiple defensible answers, coverage clustered on one passage, answers not actually grounded in
the cited chunk).

This replaces that single call with a bounded four-stage loop in a new
`app/services/quiz_harness.py`:

1. Plan — one call selects which passages to test and at what cognitive level (recall /
   application / analysis), referring to passages by position rather than by chunk id, so an
   invented or out-of-range reference is corrected deterministically rather than trusted.
2. Generate — one call per planned target, pinned to that target's single source chunk.
3. Critique — one call per generated question, checked against five explicit criteria: the
   answer is entailed by the source, every distractor is plausible but demonstrably wrong, exactly
   one option is defensible, the question isn't answerable from general knowledge, and it doesn't
   duplicate another question in the set. Fails closed (treated as a rejection, not a pass) if the
   critic itself is unavailable.
4. Repair — exactly one regenerate-and-recritique attempt for a question that fails critique;
   still-failing questions are dropped and reported, never silently discarded.

Both existing callers (course-builder per-lesson quizzes, study-page ad-hoc quizzes) now flow
through this harness via `quiz_generation.generate_quiz_questions`, which stays a thin
delegating wrapper with its public signature unchanged.

Call budget: bounded by construction — at most `4 × target_count + 1` calls (worst case 17
for the default `target_count = 4`), enforced by the loop's structure rather than a mid-loop
check. No background job: generation stays synchronous, same request shape as before.

Prerequisite fix (Task 1): the study-page caller's `context_items` never carried a resolvable
chunk id (only `doc_id`/`label`) — switched it to read from `result.evidence_pack.chunks`,
which does, using data it already had in hand.

Trace: `generate_quiz_questions` now always returns `{"questions": [...], "trace": {...}}`
(never `None`), with the trace (targets planned, generated, rejected with reasons, repaired,
final count, calls used/budget) logged whenever generation produces nothing, and threaded onto the
`POST /courses/{course_id}/quizzes/generate` response as an additive `trace` field.

Persistence: `persist_quiz` now sets `Question.source_ref` from each question's `chunk_id`
(the column already existed, unused) — no migration needed.

`docs/specifications.md` §24 updated to describe the new behavior.

Issue

Refs #134. Not closing it yet — the issue's acceptance criteria include a manual review of 20
generated questions against real course material on a live QVAC service, which this PR's
automated tests cannot satisfy. That review is the one remaining step before #134 can close.

How it was verified

• Full backend suite: `cd services/ai && uv run --no-sync pytest` — 1120 passed, 85.54% coverage
  (ratchet 76%).
• `uv run --no-sync mypy app` — clean, 88 source files.
• Built via Subagent-Driven Development: a fresh implementer + fresh task reviewer per task (7
  tasks), then an independent whole-branch review on the most capable model, one consolidated fix
  wave for its 6 Important findings, and a scoped re-review confirming all 6 addressed with no new
  breakage.
• Design doc and implementation plan committed under `docs/superpowers/` for anyone who wants the
  full reasoning trail (call-budget rationale, rejected alternatives, what was explicitly left to
  the builder).

Things reviewers should watch for

• Duplicate-repair risk: the course-builder path caps context at 3 chunks
  (`MAX_CONTEXT_CHUNKS`) while the harness always plans 4 targets, so at least one chunk gets
  reused — a single-source lesson could see duplicate→repair→duplicate cycles that burn the call
  budget without the repair fixing anything, since repair feedback for a duplicate rejection is
  just the word "duplicate" (the model isn't shown what it duplicated). Not fixed here pending
  real-LLM behavior data; flagging for the post-merge manual review to actually observe.
• Rate limit unchanged: `/quizzes/generate` is still `10/minute`, which now permits up to
  170 LLM calls/minute from one user instead of 10. No frontend caller exists for this endpoint
  today, so not urgent, but worth sizing once one does.
• No frontend changes in this PR — the `trace` field is response-only; a debug-page panel to
  visualize it is left to a follow-up issue.
• This is a large PR (11 commits) for one coherent, explicitly-planned feature (the full harness);
  happy to discuss splitting further if that's preferred for review, but the stages are tightly
  interdependent (plan output feeds generate, generate output feeds critique, all three feed the
  repair loop) so there wasn't a natural seam that produces several independently-useful PRs.

[lucaosti]

Solid work: passage references by index with deterministic correction, a critic that fails closed, a call budget bounded by structure, a uniform trace, source_ref finally filled, and real chunk ids from the study caller. The "things to watch" section is very useful. Harness, quiz generation, quizzes API and lesson service tests pass locally (85).

Two changes before merging, both reproduced with a small probe:

1. LlmDisabledError is swallowed. _generate_one's docstring says it propagates to abort the run, but run() catches it at the plan, generate and repair steps, and _critique_one catches it too. With the LLM disabled, run() returns 0 questions and no exception. So the study endpoint answers "could not generate" instead of reporting the LLM as disabled, and the except LlmDisabledError: raise around the quiz step in lesson_service no longer fires. Catch only StructuredGenerationError and httpx.HTTPError for the per-question fallbacks and let LlmDisabledError propagate.
2. The plan does not enforce distinct passages. A plan that returns passage 1 four times gives targets c0, c0, c0, c0. That is the duplicate -> repair -> duplicate cycle you flagged, and it can happen with any context size, not only the 3-chunk lesson cap. Dedupe the indices and fill with unused passages round-robin, reusing a passage only when there are fewer passages than targets. target_count = min(4, len(context_items)) for lessons would also help.

Worth considering (not blocking):

3. trace is returned to any authenticated user, including rejected question previews and chunk ids, and it is outside the response schema. Gate it to reviewer/admin or a debug flag.
4. Up to 17 sequential calls in one request, and 10/minute now allows 170 calls a minute per user. Fine while no frontend calls it; lower the limit or move it to a job before one does.
5. A quiz can now be persisted with a single question (the old schema required 3-4). Worth deciding a minimum.

Nits:

6. docs/superpowers/ adds about 1,300 lines of generated spec and step-by-step plan in a new docs folder, and code comments point there. Keep the design doc under docs/ with the other specs and drop the implementation plan, which goes stale once merged.
7. The generate_quiz_questions docstring says the result "is still truthy, so this never changes caller behaviour". It is the other way round: if quiz_data: is now always true, and it is safe only because persist_quiz returns None for an empty list.

[longobucco]

Addressed both blocking items in b850bf2:

1. LlmDisabledError swallowing — run() and _critique_one now only catch StructuredGenerationError/httpx.HTTPError for per-question fallbacks; LlmDisabledError propagates from planning, generation, repair, and critique, so the existing except LlmDisabledError: raise in lesson_service and the study endpoint's disabled-service handling fire again. Tests updated to assert propagation instead of a swallowed-to-zero-questions result.
2. Duplicate passages in the plan — _plan_targets now tracks used indices and fills any invalid/missing/repeated passage_index round-robin over unused passages, reusing one only after all have been assigned once. Added a test with a model that returns passage_index: 1 four times straight, confirming distinct chunks are assigned until exhaustion.

Also took nit 6: moved the design doc to docs/quiz-agent-harness-design.md (indexed in docs/overview.md) and dropped the implementation plan.

Left 3–5 as flagged, not blocking:

• Trace visibility (3) and the rate limit (4) are both called out already in "things reviewers should watch for" — no frontend caller exists yet for either to bite, so deferring rather than guessing at the right gate/limit now.
• Minimum question count (5): the harness can already report a partial/empty set via trace, and issue Agent harness for quiz generation (plan → generate → critique → repair) #134's manual review will surface whether a 1-question quiz is actually a problem in practice rather than hypothetically.

Didn't take the optional target_count = min(4, len(context_items)) suggestion in (2) — the round-robin dedupe already prevents the duplicate-repair cycle at any passage count, so capping target_count would only reduce quiz length without fixing anything the dedupe doesn't already fix.

Verified: pytest — 1123 passed, 85.48% coverage (ratchet 76%). mypy app — clean, 88 files.

[lucaosti]

Thanks Luca, both blocking points are fixed and I checked them with probes. With the LLM disabled, run() now raises at both the plan and the generation stage, so lesson_service's re-raise works again. A plan that repeats passage 1 four times now gives distinct targets, and reuses a passage only when there are fewer passages than targets. Moving the design doc to docs/ and dropping the plan is much cleaner. Full suite green (1123).

Small leftovers, none blocking:

• The docstring in quiz_generation.py (lines 35-36) still has a fragment of the old sentence ("callers that check ... or do callers must inspect ..."). The old clause just needs deleting.
• The three points from last time are still open: the trace (rejected previews and chunk ids) goes to any logged-in student, the 10/minute limit allows up to 170 LLM calls a minute per user, and a 1-question quiz can be saved. A short reply on what you plan for each is enough, even if it is a follow-up issue before the frontend starts calling the endpoint.
• Optional: the endpoint turns LlmDisabledError into a generic 500, same as before. A 503 with a clear message would help the study page.

Approving with these nits.