Uh oh!
There was an error while loading. Please reload this page.
Quiz grounding in course material + rigorous eval - #318
Merged
Conversation
…aterial Mirrors the tutor's grounding pattern (routes/learn.py::_chat_via_agent): resolve the course's BU course_code, pull the catalog chunk plus top document-RAG chunks for the target concept, and prepend them as a COURSE MATERIAL block ahead of the routing message. Grounding is best-effort — any missing course_id/bu_code or retrieval failure degrades silently to the ungrounded prompt, never a 502.
Adds the deterministic fixture course (TEST QG 101) and manifest that Layer-1/Layer-2 quiz-grounding evals (Tasks 3-5) run against, plus an idempotent script that indexes it into staging course_chunks via index_document_chunks.
Implements Layer 1 of the quiz grounding benchmark: - score_retrieval() scorer with case-insensitive substring matching - unit tests for perfect/zero recall scenarios - benchmark script that reads fixture manifest and reports per-concept metrics - --chunks-only CLI mode for Layer 1 only (Layer 2 in Task 4) - staging run shows mean recall=1.0, mean precision=0.56 across test concepts Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds the LLM-judge panel (majority_vote/judge_question/aggregate) and a Layer 2 pass to benchmark_quiz.py that drives the real quiz agent against the seeded staging fixture and scores generated questions for grounding, scope, and answer correctness. Closes the grounding-wiring gap: seed_quiz_fixture.py now upserts a fixture `courses` row (FIXTURE_COURSE_ID) so _resolve_bu_code can map it to the fixture bu_code, letting _course_material_block inject real course material instead of running an ungrounded quiz through the judge. Layer 2 now runs the whole loop inside one asyncio.run() and isolates each concept's generation in a try/except, since per-concept asyncio.run() calls broke the Gemini SDK's async client on the 2nd+ call, and a single generation failure shouldn't crash the rest of the report. Also fixes call_gemini (single-turn) to cap thinking_budget the same way call_gemini_multiturn already does: gemini-2.5-pro 400s on thinking_budget=0, which broke the judge model (JUDGE_MODEL = gemini-2.5-pro, deliberately different from the quiz generator's gemini-2.5-flash-lite to avoid self-preference bias).
…sh-lite
Quiz generation 400'd on gemini-2.5-flash-lite/-flash ("schema produces
a constraint that has too many states for serving") because the
QuizQuestion/Quiz structured-output schema combined string max_length
constraints with nested array length bounds. Only gemini-2.5-pro
served it, but flash-lite is model_for("quiz")'s production default.
Drop max_length on question/correct_answer/explanation/concept (length
now governed by the system prompt), cap Quiz.questions at 10 (down
from 20), and pin options to exactly 4 (matches the existing "4
options, exactly one correct" prompt instruction). Field names/types
are unchanged so the route's wire mapping and frontend are unaffected.
Verified live against staging: flash-lite now generates a quiz
successfully. Bisected the fix — string-length removal alone still
400s; also capping questions at 10 is what clears it, per Gemini's
constrained-decoding automaton-count limit.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>Add calibration_agreement()/run_calibrate() and --calibrate CLI mode to benchmark_quiz.py, wired to print judge-vs-human agreement per dimension using gold_labels.json. gold_labels.json is intentionally left empty (human labeling is a manual step, not something an LLM should fabricate); --calibrate correctly detects the empty set and prints a reminder instead of running the judge. README.md documents the labeling rubric and the exact gold_labels.json entry shape.
…ad of silently truncating Previously, GenerateQuizBody.num_questions had no upper bound while Quiz.questions was capped at max_length=10. Requests with num_questions > 10 silently returned ≤10 questions instead of erroring. Now bounded with Field(default=5, ge=1, le=10) to enforce validation at the pydantic layer, returning 422 for invalid requests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Deploying with |
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs | frontend-staging | 5f02753 | Commit Preview URL Branch Preview URL | Jul 15 2026, 04:20 AM |
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughQuiz generation now supports optional course-material grounding, tighter quiz limits, centralized Gemini settings, RAG safeguards, deterministic grounding benchmarks, and agent-first concept scanning with legacy fallback. ChangesQuiz Grounding Feature
Concept Scan Agent Migration
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant QuizRoute
participant Courses
participant Catalog
participant RAG
participant QuizAgent
Client->>QuizRoute: Submit quiz generation request
QuizRoute->>Courses: Resolve course code
QuizRoute->>Catalog: Retrieve catalog context
QuizRoute->>RAG: Retrieve concept chunks
QuizRoute->>QuizAgent: Run with optional course material
QuizAgent-->>Client: Return generated quiz
sequenceDiagram
participant Client
participant DocumentsRoute
participant ConceptScanAgent
participant LegacyExtension
Client->>DocumentsRoute: Submit concept scan request
DocumentsRoute->>ConceptScanAgent: Run agent-first scan
ConceptScanAgent-->>DocumentsRoute: Concepts or error
DocumentsRoute->>LegacyExtension: Fallback on agent error
LegacyExtension-->>Client: Return compatible concept response
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
backend/services/gemini_service.py (1)
64-80: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract duplicated
thinking_budgetlogic into a shared helper.The exact expression
thinking_budget = 2048 if "pro" in model else 0now appears in bothcall_gemini(Line 74) andcall_gemini_multiturn(Line 122). Consolidating avoids the two call sites drifting out of sync if the pro/flash cap or model-matching logic changes again.♻️ Proposed refactor
+def _thinking_budget_for(model: str) -> int:+ """gemini-2.5-pro rejects thinking_budget=0 ("This model only works in+ thinking mode" — 400 INVALID_ARGUMENT); Flash/Flash-Lite are fine with+ it disabled for latency.+ """+ return 2048 if "pro" in model else 0++ def call_gemini(prompt: str, retries: int = 1, json_mode: bool = False, model: str = MODEL_DEFAULT) -> str: - """Single-turn call to Gemini with a plain string prompt.-- gemini-2.5-pro rejects thinking_budget=0 ("This model only works in- thinking mode" — 400 INVALID_ARGUMENT); Flash/Flash-Lite are fine with- it disabled for latency. Mirrors the same cap already applied in- call_gemini_multiturn (PR `#74`) — that fix never propagated to this- single-turn path, so any caller passing model="gemini-2.5-pro" here- (e.g. an LLM-judge model override) always 400'd.- """- thinking_budget = 2048 if "pro" in model else 0+ """Single-turn call to Gemini with a plain string prompt."""+ thinking_budget = _thinking_budget_for(model)And in
call_gemini_multiturn:- thinking_budget = 2048 if "pro" in model else 0+ thinking_budget = _thinking_budget_for(model)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/services/gemini_service.py` around lines 64 - 80, The thinking budget cap is duplicated between call_gemini and call_gemini_multiturn, so the model-based logic can drift out of sync. Extract the `thinking_budget = 2048 if "pro" in model else 0` decision into a shared helper in gemini_service.py, then have both call_gemini and call_gemini_multiturn use that helper when building their GenerateContentConfig/ThinkingConfig.backend/scripts/seed_quiz_fixture.py (1)
50-58: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winChunk fixture docs independently before indexing. The current
"\n\n".join(texts)letschunk_document()merge across adjacent files when a boundary block is short; one chunk already spans the end of02-hash-tables.mdand the heading of03-amortized-analysis.md. Chunk each doc separately, then concatenate the chunk lists before callingindex_document_chunks().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/scripts/seed_quiz_fixture.py` around lines 50 - 58, The current main flow in seed_quiz_fixture.py joins all doc texts before calling chunk_document(), which can let chunks cross file boundaries. Update main() to chunk each file from FIX / "docs" independently, then combine the resulting chunk lists and pass that merged list to index_document_chunks(); keep the existing seed_fixture_course, BU_CODE, DOC_ID, and UPLOADER flow intact.backend/scripts/fixtures/quiz_grounding/README.md (1)
61-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd language tags to the shell fences.
Markdownlint flags these command blocks; tagging them as
powershell(or your chosen shell) keeps the README lint-clean.♻️ Proposed fix
-```+```powershell cd backend .\venv\Scripts\python.exe scripts/seed_quiz_fixture.py```diff -``` +```powershell .\venv\Scripts\python.exe -c "from dotenv import load_dotenv; load_dotenv('.env.staging'); import sys; sys.path.insert(0,'.'); from services.rag_service import retrieve_chunks; print(len(retrieve_chunks('dynamic programming', course_id='TEST QG 101', k=5)))"```diff -``` +```powershell .\venv\Scripts\python.exe scripts/benchmark_quiz.py --calibrate</details> Also applies to: 75-77, 160-162 <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@backend/scripts/fixtures/quiz_grounding/README.mdaround lines 61 - 64, Add
language tags to the markdown code fences in the quiz_grounding README so
markdownlint stops flagging them; update each shell command block to use a
specific fence like powershell (or the appropriate shell) for the commands
around the fixture seed, retrieve_chunks one-liner, and benchmark_quiz
invocation. Keep the command contents unchanged and make sure all affected
fences in this document use the same tagged style.</details> <!-- cr-comment:v1:ecd57e48aa2d724958df11db --> _Source: Linters/SAST tools_ </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.Inline comments:
In@backend/routes/quiz.py:
- Around line 161-184: The quiz RAG lookup can still hang because
retrieve_chunks()depends on agenai.Clientcreated without an explicit
timeout. Update the Gemini client setup inbackend/services/rag_service.pyto
passHttpOptions.timeoutso embedding requests are bounded, and keep the
existingretrieve_chunks()call in_course_material_block()unchanged since
its call shape is already correct.In
@backend/scripts/seed_quiz_fixture.py:
- Around line 14-16: The dotenv loading in seed_quiz_fixture.py and
benchmark_quiz.py currently preserves existing shell credentials, which can
point the scripts at the wrong Supabase project. Update the load_dotenv call in
each script to force .env.staging values to replace any pre-set environment
variables, and keep the change near the BASE setup so the script behavior is
consistent regardless of the caller’s shell. Use the existing load_dotenv usage
in the script entrypoints as the place to make this change.In
@docs/superpowers/plans/2026-07-03-quiz-grounding.md:
- Around line 161-196: Make the grounding helper truly best-effort by preventing
upstream lookup failures from escaping. In_resolve_bu_code, wrap thetable("courses").select(...)call and returnNoneon any exception; in_course_material_block, also guard_get_catalog_chunk(bu_code)with the same
fallback-to-empty-string behavior used forretrieve_chunks. Keep the existingbest-effortcontract so_course_material_blocknever throws and quiz
generation can continue even when course lookup or catalog retrieval fails.- Around line 677-693: Use the seeded fixture course UUID in generate_quiz_for
instead of passing course_id=None, so _quiz_via_agent and _course_material_block
can resolve the real bu_code for grounding. Thread the course identifier through
the benchmark/fixture entrypoint from the seeding step, and update the
generate_quiz_for call site to use that real course_id. If the fixture course
lookup in courses is missing, fix the fixture data path rather than relying on
the fallback patch.In
@docs/superpowers/specs/2026-07-03-quiz-grounding-design.md:
- Around line 61-90: The spec example for _course_material_block currently omits
the runtime error-swallowing behavior, so it no longer matches the intended
best-effort contract. Update the example to mirror the route implementation by
guarding both the course lookup in _resolve_bu_code and the _get_catalog_chunk /
retrieve_chunks calls with try/except, returning None or an empty block on
failure so _course_material_block remains “never raises.”Nitpick comments:
In@backend/scripts/fixtures/quiz_grounding/README.md:
- Around line 61-64: Add language tags to the markdown code fences in the
quiz_grounding README so markdownlint stops flagging them; update each shell
command block to use a specific fence like powershell (or the appropriate shell)
for the commands around the fixture seed, retrieve_chunks one-liner, and
benchmark_quiz invocation. Keep the command contents unchanged and make sure all
affected fences in this document use the same tagged style.In
@backend/scripts/seed_quiz_fixture.py:
- Around line 50-58: The current main flow in seed_quiz_fixture.py joins all doc
texts before calling chunk_document(), which can let chunks cross file
boundaries. Update main() to chunk each file from FIX / "docs" independently,
then combine the resulting chunk lists and pass that merged list to
index_document_chunks(); keep the existing seed_fixture_course, BU_CODE, DOC_ID,
and UPLOADER flow intact.In
@backend/services/gemini_service.py:
- Around line 64-80: The thinking budget cap is duplicated between call_gemini
and call_gemini_multiturn, so the model-based logic can drift out of sync.
Extract thethinking_budget = 2048 if "pro" in model else 0decision into a
shared helper in gemini_service.py, then have both call_gemini and
call_gemini_multiturn use that helper when building their
GenerateContentConfig/ThinkingConfig.</details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro Plus **Run ID**: `c0d09923-893e-451a-8f21-15daa3d6a079` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between cc1634a327fa0a0f58d65ed6b3b0d9535dc73314 and 23730399567ba3ed0d6de85b1a5fafc97ca60db3. </details> <details> <summary>📒 Files selected for processing (19)</summary> * `backend/agents/quiz.py` * `backend/models/__init__.py` * `backend/routes/quiz.py` * `backend/scripts/benchmark_quiz.py` * `backend/scripts/fixtures/quiz_grounding/README.md` * `backend/scripts/fixtures/quiz_grounding/docs/01-dynamic-programming.md` * `backend/scripts/fixtures/quiz_grounding/docs/02-hash-tables.md` * `backend/scripts/fixtures/quiz_grounding/docs/03-amortized-analysis.md` * `backend/scripts/fixtures/quiz_grounding/docs/04-graph-coloring.md` * `backend/scripts/fixtures/quiz_grounding/docs/05-binary-search-trees.md` * `backend/scripts/fixtures/quiz_grounding/gold_labels.json` * `backend/scripts/fixtures/quiz_grounding/manifest.json` * `backend/scripts/seed_quiz_fixture.py` * `backend/services/gemini_service.py` * `backend/tests/test_benchmark_quiz.py` * `backend/tests/test_gemini_service.py` * `backend/tests/test_quiz_routes.py` * `docs/superpowers/plans/2026-07-03-quiz-grounding.md` * `docs/superpowers/specs/2026-07-03-quiz-grounding-design.md` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Comment on lines
+161
to
+196
| ```python | ||
| def _resolve_bu_code(course_id: str | None) -> str | None: | ||
| """Resolve a Sapling course UUID to its BU course_code (course_chunks | ||
| partition key). None if unresolvable. Mirrors routes/documents.py.""" | ||
| if not course_id: | ||
| return None | ||
| rows = table("courses").select( | ||
| "course_code", filters={"id": f"eq.{course_id}"}, limit=1 | ||
| ) | ||
| return (rows[0].get("course_code") if rows else None) or None | ||
| def _course_material_block(course_id: str | None, concept_name: str) -> str: | ||
| """Best-effort catalog + document-chunk context for a concept. | ||
| Returns "" if nothing is available (no course, no bu_code, no chunks) or | ||
| if retrieval raises — grounding must never break quiz generation. | ||
| """ | ||
| bu_code = _resolve_bu_code(course_id) | ||
| if not bu_code: | ||
| return "" | ||
| blocks: list[str] = [] | ||
| try: | ||
| catalog = _get_catalog_chunk(bu_code) | ||
| except Exception: | ||
| catalog = "" | ||
| if catalog: | ||
| blocks.append("COURSE CATALOG (official BU course data):\n\n" + catalog) | ||
| try: | ||
| chunks = retrieve_chunks(concept_name, course_id=bu_code, k=5) | ||
| except Exception: | ||
| chunks = [] | ||
| rag_block = format_rag_context(chunks) | ||
| if rag_block: | ||
| blocks.append(rag_block) | ||
| return "\n\n".join(blocks) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Keep the sample grounding helper truly best-effort.
The snippet still lets table("courses").select(...) and _get_catalog_chunk(...) raise. That contradicts the best-effort contract above: a transient lookup/catalog failure would now bubble out and block quiz generation.
🔧 Proposed fix
def _resolve_bu_code(course_id: str | None) -> str | None:
if not course_id:
return None
- rows = table("courses").select(- "course_code", filters={"id": f"eq.{course_id}"}, limit=1- )+ try:+ rows = table("courses").select(+ "course_code", filters={"id": f"eq.{course_id}"}, limit=1+ )+ except Exception:+ return None
return (rows[0].get("course_code") if rows else None) or None
@@
- catalog = _get_catalog_chunk(bu_code)+ try:+ catalog = _get_catalog_chunk(bu_code)+ except Exception:+ catalog = ""🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/superpowers/plans/2026-07-03-quiz-grounding.md` around lines 161 - 196,
Make the grounding helper truly best-effort by preventing upstream lookup
failures from escaping. In `_resolve_bu_code`, wrap the
`table("courses").select(...)` call and return `None` on any exception; in
`_course_material_block`, also guard `_get_catalog_chunk(bu_code)` with the same
fallback-to-empty-string behavior used for `retrieve_chunks`. Keep the existing
`best-effort` contract so `_course_material_block` never throws and quiz
generation can continue even when course lookup or catalog retrieval fails.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (2)
docs/superpowers/specs/2026-07-08-scan-concepts-agent-migration-design.md (1)
157-164: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd fallback telemetry.
With
except Exceptiondropping to the legacy path, you’ll want a counter/log on that branch; otherwise the migration can look healthy while every request is silently served by the old implementation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/superpowers/specs/2026-07-08-scan-concepts-agent-migration-design.md` around lines 157 - 164, The generic fallback path in the concept scan migration is missing telemetry, so add a counter or equivalent log in the broad except Exception branch alongside the existing logger.exception call. Update the fallback handling in the extend/scan flow (where run_agent_sync(_extend_via_agent(...)) falls back to _extend_course_concepts(...)) so failures landing on the legacy path are explicitly recorded and can be tracked during migration.backend/routes/documents.py (1)
159-194: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract shared
notes_blockformatting to avoid divergence between agent and legacy paths.The
notes_blockconstruction at lines 179-185 is duplicated verbatim from the legacy_extend_course_conceptsat lines 120-126. If one copy is updated and the other is not, the agent and legacy paths will see different document context, causing subtle migration regressions.♻️ Proposed helper extraction
+def _format_concept_notes(notes: list[dict] | None) -> str:+ """Format document concept notes for prompt inclusion."""+ return (+ "\n".join(+ f" - {n.get('name', '?')}: {n.get('description', '')[:200]}"+ for n in (notes or [])+ )+ or " (none)"+ )+Then use it in both
_scan_user_messageand_extend_course_concepts:def _scan_user_message( *, course_label: str, existing_concepts: list[str], doc_filename: str | None = None, doc_summary: str | None = None, doc_concept_notes: list[dict] | None = None, ) -> str: ... if doc_filename or doc_summary or doc_concept_notes: - notes_block = (- "\n".join(- f" - {n.get('name', '?')}: {n.get('description', '')[:200]}"- for n in (doc_concept_notes or [])- )- or " (none)"- )+ notes_block = _format_concept_notes(doc_concept_notes) lines += [And similarly in
_extend_course_concepts:def _extend_course_concepts( *, ... ) -> list[str]: ... if doc_filename or doc_summary or doc_concept_notes: - notes_block = (- "\n".join(- f" - {n.get('name', '?')}: {n.get('description', '')[:200]}"- for n in (doc_concept_notes or [])- )- or " (none)"- )+ notes_block = _format_concept_notes(doc_concept_notes)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/routes/documents.py` around lines 159 - 194, The document notes formatting is duplicated between _scan_user_message and the legacy _extend_course_concepts path, which risks the agent and legacy flows diverging over time. Extract the shared notes_block construction into a small helper (or equivalent shared formatter) and have both _scan_user_message and _extend_course_concepts use that same symbol so any future change to concept note rendering is applied consistently in both paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@backend/routes/documents.py`:
- Around line 159-194: The document notes formatting is duplicated between
_scan_user_message and the legacy _extend_course_concepts path, which risks the
agent and legacy flows diverging over time. Extract the shared notes_block
construction into a small helper (or equivalent shared formatter) and have both
_scan_user_message and _extend_course_concepts use that same symbol so any
future change to concept note rendering is applied consistently in both paths.
In `@docs/superpowers/specs/2026-07-08-scan-concepts-agent-migration-design.md`:
- Around line 157-164: The generic fallback path in the concept scan migration
is missing telemetry, so add a counter or equivalent log in the broad except
Exception branch alongside the existing logger.exception call. Update the
fallback handling in the extend/scan flow (where
run_agent_sync(_extend_via_agent(...)) falls back to
_extend_course_concepts(...)) so failures landing on the legacy path are
explicitly recorded and can be tracked during migration.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a2d7947b-9ca6-458e-bf1f-d8a1a53d079b
📒 Files selected for processing (6)
backend/agents/_providers.pybackend/agents/concept_scan.pybackend/routes/documents.pybackend/tests/test_concept_scan.pydocs/superpowers/plans/2026-07-08-scan-concepts-agent-migration.mddocs/superpowers/specs/2026-07-08-scan-concepts-agent-migration-design.md
✅ Files skipped from review due to trivial changes (1)
- docs/superpowers/plans/2026-07-08-scan-concepts-agent-migration.md
- gemini_service: extract shared _thinking_budget_for() so call_gemini and call_gemini_multiturn can't drift on the Pro thinking cap - rag_service: bound the embedding client with an explicit HTTP timeout so inline retrieve_chunks() can't hang the quiz/tutor request path - seed_quiz_fixture: chunk each fixture doc independently (no cross-file chunk boundaries); force .env.staging with override=True - benchmark_quiz: force .env.staging with override=True - README: tag shell code fences as powershell for markdownlint - docs: sync stale plan/spec snippets to the shipped best-effort contract (_resolve_bu_code/_get_catalog_chunk guarded; generate_quiz_for uses the seeded FIXTURE_COURSE_ID instead of None) - tests: pin _thinking_budget_for, rag timeout + best-effort empty return, and per-file chunking in seed_quiz_fixture Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Uh oh!
There was an error while loading. Please reload this page.
Resolve the CodeQL/github-code-quality finding that services.rag_service was imported both as `import services.rag_service as rag` and `from services.rag_service import ...` in the same file. Switch the timeout test to `from services.rag_service import _HTTP_TIMEOUT_MS` — behavior-preserving. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
_course_material_block does blocking network I/O (a Gemini embedding call, now bounded at 60s) plus sync Supabase reads, and it was called directly inside the async _quiz_via_agent. On an async handler that runs on the event loop, so a slow or stalled retrieval froze the whole worker's loop — every concurrent request stalled with it, up to the 60s embedding timeout. Wrap the call in asyncio.to_thread so grounding runs in a worker thread, matching the pattern the agent read tools already use (graph_read, quiz_history, chat_context). Behaviour is unchanged; only the thread it runs on differs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`"pro" in model` raises TypeError if a caller ever passes model=None (the parameter is typed str, but nothing enforces it at runtime). Guard with `model and ...` so a None/empty model falls through to budget 0 instead of crashing the call. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
_course_material_block injects the catalog chunk as a COURSE CATALOG block and separately injects semantic retrieve_chunks results. Because catalog chunks live in the same course_chunks store, a catalog chunk can rank into the semantic results and get sent to the model a second time. Filter retrieved chunks whose text equals the already-injected catalog so the same course-description text isn't paid for twice in the prompt. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Grounds quiz generation in the same catalog + document-RAG context the tutor uses, so quiz questions reflect what the class is actually learning from uploaded course material — and adds a two-layer eval to verify it. Also fixes a pre-existing production bug where quiz generation 400'd on the default model.
Base:
main· Head:rag· 12 commits.What changed
Feature — grounding (
routes/quiz.py,agents/quiz.py)_quiz_via_agentnow resolves the BU course code and prepends aCOURSE MATERIALblock (catalog chunk +retrieve_chunks(concept_name)) before generating — the same proven pattern as the tutor's_chat_via_agent.Production fix — quiz schema (
agents/quiz.py)gemini-2.5-flash-lite(the default) andgemini-2.5-flash— onlyproserved the old schema, and there's no model override in staging, so quiz generation was broken on the default path. Root cause: the output schema's constrained-decoding state count. Fix: cappedQuiz.questions20→10 (the decisive lever), pinnedoptionsto 4, removed stringmax_lengths. Flash-lite now serves. Boundednum_questionsto 1–10 so over-cap requests 422 instead of silently truncating.Eval — two layers (
scripts/benchmark_quiz.py, fixtures)--calibrate) to validate the judge against a human-labeled gold set (labeling is a manual step —gold_labels.jsonintentionally ships empty).TEST QG 101course.Validation
Follow-ups (not in this PR)
gold_labels.jsonwith ~20 human labels, then run--calibrateto confirm the judge agrees with human judgment before trusting Layer 2 numbers.graph_nodeslookup atquiz.pybefore the try block can 500 (unrelated to grounding).Summary by CodeRabbit