Uh oh!
There was an error while loading. Please reload this page.
chore(quiz): delete the dead effective_explanations read (#572) - #587
Conversation
`_parse_quiz_context_to_arrays` read quiz_context.context_json's effective_explanations key, but no writer (agents/quiz_context.py:: QuizContext writes weak_areas/common_mistakes/questions_seen_summary/ recommended_difficulty/notes only) ever produces it -- the read has been silently dead since offering_concept_stats.effective_explanations existed, always persisting []. Delete the read and its now-noise seed write in seed_local_rich.py; the column stays on its DB default per the issue's own decision framing (no migration). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe quiz-context aggregation now handles only common misconceptions and prerequisite gaps. Concept-statistics upserts and local seed rows omit ChangesCourse context aggregation cleanup
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk:⚪ Minimal · up to This localized cleanup stops emitting an unused empty field while preserving the database column and its default behavior; no actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Description checkExplanation The description clearly explains the dead read, lists the code and seed changes, identifies issue ✨ 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 |
Deploying with |
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs | frontend-staging | 4b38b13 | Commit Preview URL Branch Preview URL | Aug 26 2026, 05:27 AM |
This pull request has been ignored for the connected project Preview Branches by Supabase. |
/code-review on PR #587 (the effective_explanations dead-read deletion, #572) reviewed the core deletion clean and turned up five in-PR nits + six pre-existing debts. This lands the in-PR nits; the pre-existing debts are filed as #595. - F5 (tripwire): test_effective_explanations_never_emitted_even_when_source_has_it pinned its premise (QuizContext never emits effective_explanations) only in prose. Add the structural assertion (assertNotIn on QuizContext.model_fields) so a future field-without-rollup regression goes red instead of just documenting the drop as correct. - F6/F7 (comment precision): the canonical #572 comment in _parse_quiz_context_to_arrays overstated the DB default's coverage (only true on a fresh INSERT — the on_conflict merge-duplicates UPDATE path never touches the column, so an out-of-band value would persist) and omitted that every stored context_json row also carries a server-stamped schema_version. - F9 (prose dedup): the #572 story was told in full three times (the service comment, the test docstring, db/seed_local_rich.py). Keep one canonical telling at the deletion site; the other two are now one-line pointers at it. - F8/F10 (test-helper dedup): extended TestUpdateCourseContext._table_factory with an optional quiz_rows param and converted all four ad hoc `_table` dispatch closures (three pre-existing + the new tripwire test) to use it; deleted the dead module-level _make_table_mock (zero call sites). Refs #572. Follow-up: #595.
Uh oh!
There was an error while loading. Please reload this page.
Summary
services/course_context_service.py::_parse_quiz_context_to_arraysreadquiz_context.context_json'seffective_explanationskey alongside its twolegitimate siblings (
common_mistakes,weak_areas) — but no writeranywhere in the codebase (
agents/quiz_context.py::QuizContextwrites onlyweak_areas/common_mistakes/questions_seen_summary/recommended_difficulty/notes) ever produces that key. The read has beensilently dead since the column existed, meaning
offering_concept_stats.effective_explanationshas persisted analways-empty array for every offering — indistinguishable from "this class
produced no effective explanations" (the #548/#529 failure class, found by
the code review that specifically went looking for it, #554 H2).
Per the issue's own decision framing, this takes option 2: delete the
read (option 1, making the field real, needs #558-style consent framing
before it can widen what class-derived text the tutor surfaces — out of
scope for a cleanup task).
effective_explanationsaccumulation from_parse_quiz_context_to_arrays(now returns a 2-tuple) and its write intothe
offering_concept_statsupsert."effective_explanations": []fromdb/seed_local_rich.py's seed — noise now that nothing reads it; thecolumn's own DB default (
'{}', unchanged in0022_analytics.sql)produces the identical value.
Grep verification
/usr/bin/grep -rn effective_explanationsacrossbackend/,frontend/src,frontend/e2e,docs/— every hit accounted for (full table intask-8-report.md): 2 immutable migrations (kept, column stays), 2 docs(kept, doc-historical/still-accurate), 5 code sites in
course_context_service.py+ 1 inseed_local_rich.py(all deleted), zerofrontend or admin-analytics consumers.
Test plan
TestUpdateCourseContext::test_effective_explanations_never_emitted_even_when_source_has_it— plants the key in source data, asserts it's absent from the upsert
payload. RED confirmed against pre-fix code, GREEN after.
venv/bin/ruff check .— All checks passed2224 passed, 79 skippedCloses#572
Lane runs (overnight 2026-08-23, local stack under the flock, function mode)
Merge-gate review (2026-08-26)
/code-review(medium) on this PR: the core deletion reviewed clean (5 angles). It alsosurfaced 5 in-PR polish nits and 6 pre-existing debts in the touched function.
(
assertNotIn("effective_explanations", QuizContext.model_fields)) so the regression testenforces its premise in code, not just prose; tightened two comments (the DB-default claim only
holds on a fresh INSERT, not on the upsert's merge-duplicates UPDATE path; every stored
context_jsonrow also carries a server-stampedschema_version); deduplicated the quiz: offering_concept_stats.effective_explanations is a dead read — no agent writes that key #572 story toone canonical telling with two one-line pointers; extended the test's
_table_factoryhelper anddeleted a dead
_make_table_mock. Full suite re-verified green (2224 passed / 79 skipped), ruffclean.
update_course_context(not fixed here — out of scope for this cleanup):filed as course_context rollup: opt-out purge gap, silent-empty overwrite, unguarded legacy shapes, N+1 reads/writes #595 (opt-out purge gap on two early exits, a silent-empty catch whose overwrite can wipe
previously-good aggregates on a transient failure, missing legacy-shape guards in the parse loop,
and N+1 reads/writes in the per-concept loop).
Summary by CodeRabbit
Bug Fixes
Tests