Uh oh!
There was an error while loading. Please reload this page.
fix(quiz/graph): route mastery writes through one path + refresh course context (#128) - #242
fix(quiz/graph): route mastery writes through one path + refresh course context (#128)#242AndresL230 wants to merge 3 commits into
Conversation
…se context (#128) Knowledge-graph write-integrity fixes from the backend audit (#128): - #6 (HIGH): quiz submit no longer writes graph_nodes directly while re-implementing mastery/event logic. Quiz scoring and apply_graph_update's updated_nodes loop now both route through a single sanctioned primitive, graph_service.apply_mastery_event, and quiz submit backgrounds update_course_context() so the per-course aggregate no longer goes stale after a quiz. - #9 (MEDIUM): deferred per decision. The read-modify-write is still non-atomic, but now lives on one code path with an inline TODO; the DB-side atomic append is a follow-up tied to the migration-runner (#197). - #24 (LOW): edge dedup now ignores orientation for symmetric relationship types (related/similar) while keeping directional types (prerequisite/builds_on) distinct. apply_mastery_event takes an optional user_id so the write stays owner-scoped, preserving the IDOR defense-in-depth from b3952f0. Tests: +10 (apply_mastery_event incl. user_id scoping, edge orientation, quiz course-context refresh). Full backend suite green (718 passed); ruff clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughIntroduces ChangesMastery centralization, edge dedup, and quiz route wiring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Uh oh!
There was an error while loading. Please reload this page.
Deploying with |
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs | frontend | ac25d0e | Commit Preview URL Branch Preview URL | Jun 22 2026, 02:54 AM |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
backend/routes/quiz.py (1)
441-446: 💤 Low valueSilent exception swallowing may hinder debugging.
The
except Exception: passpattern loses valuable diagnostic information ifupdate_course_contextfails. Consider logging atdebugorwarninglevel to aid troubleshooting without blocking the response.This mirrors the pattern in
apply_graph_update(lines 598-599), so it's consistent with existing code, but both locations could benefit from minimal logging.🤖 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/quiz.py` around lines 441 - 446, The bare `except Exception: pass` in the `_refresh_course_ctx` function silently swallows any exceptions from `update_course_context`, making debugging difficult. Replace the `pass` statement with a logging call at debug or warning level that captures the exception details. Include the exception information in the log message to provide diagnostic context. Consider also applying the same logging pattern to the similar exception handling in `apply_graph_update` for consistency across the codebase.backend/services/graph_service.py (1)
418-418:datetime.utcnow()is deprecated in Python 3.12+ (project target).Refactor to
datetime.now(timezone.utc).isoformat()for forward compatibility. Note that this changes the ISO format from2024-01-01T12:00:00to2024-01-01T12:00:00+00:00, which may impact downstream consumers. This issue affects 14+ locations acrossgraph_service.py,social_cache_service.py,quiz_context_service.py,calendar.py,quiz.py,learn.py, andflashcards.py—coordinate the refactor across all services to ensure consistent timestamp format.🤖 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/graph_service.py` at line 418, Replace the deprecated datetime.utcnow().isoformat() call in the "ts" field (line 418) with datetime.now(timezone.utc).isoformat() to ensure Python 3.12+ compatibility. First import timezone from the datetime module at the top of the file. Then locate all 14+ occurrences of datetime.utcnow() across graph_service.py, social_cache_service.py, quiz_context_service.py, calendar.py, quiz.py, learn.py, and flashcards.py and apply the same replacement pattern consistently. Note that this will change the timestamp format from 2024-01-01T12:00:00 to 2024-01-01T12:00:00+00:00 (with timezone offset), so ensure all downstream consumers that parse or validate these timestamps are aware of this format change before deploying.
🤖 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/quiz.py`:
- Around line 441-446: The bare `except Exception: pass` in the
`_refresh_course_ctx` function silently swallows any exceptions from
`update_course_context`, making debugging difficult. Replace the `pass`
statement with a logging call at debug or warning level that captures the
exception details. Include the exception information in the log message to
provide diagnostic context. Consider also applying the same logging pattern to
the similar exception handling in `apply_graph_update` for consistency across
the codebase.
In `@backend/services/graph_service.py`:
- Line 418: Replace the deprecated datetime.utcnow().isoformat() call in the
"ts" field (line 418) with datetime.now(timezone.utc).isoformat() to ensure
Python 3.12+ compatibility. First import timezone from the datetime module at
the top of the file. Then locate all 14+ occurrences of datetime.utcnow() across
graph_service.py, social_cache_service.py, quiz_context_service.py, calendar.py,
quiz.py, learn.py, and flashcards.py and apply the same replacement pattern
consistently. Note that this will change the timestamp format from
2024-01-01T12:00:00 to 2024-01-01T12:00:00+00:00 (with timezone offset), so
ensure all downstream consumers that parse or validate these timestamps are
aware of this format change before deploying.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e508a01f-98ee-482a-8438-12623e5dba95
📒 Files selected for processing (4)
backend/routes/quiz.pybackend/services/graph_service.pybackend/tests/test_graph_service.pybackend/tests/test_quiz_routes.py
…swallowing The background _refresh_course_ctx task swallowed all exceptions with a bare pass, hiding aggregation/summary failures. Log via logger.exception so failures are observable while keeping the submit flow non-blocking.
Deploying with |
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs | frontend-staging | 8b19719 | Commit Preview URL Branch Preview URL | Jun 24 2026, 02:44 PM |
AndresL230
commented
Jun 25, 2026
Superseded by the DB modular redesign (#279), which closes #128: quiz mastery now routes through |
What & why
Knowledge-graph write-integrity fixes from the backend audit — issue #128 (findings #6/#9/#24).
#6 (HIGH) — quiz bypassed
apply_graph_update+ left course context stalesubmit_quizwrotegraph_nodesdirectly and re-implemented mastery/event logic, and never calledupdate_course_context, so the shared per-course aggregate went stale after every quiz.graph_service.apply_mastery_event(node, delta, *, reason, event_type, user_id)— clamp → cappedmastery_eventsappend (effective delta) → bumptimes_studied/last_studied_at.submit_quizandapply_graph_update'supdated_nodesloop now route through it → mastery logic lives in one place (honors the "graph writes go through one path" convention).submit_quiznow backgroundsupdate_course_context(course_id)so the aggregate refreshes without blocking the response.#9 (MEDIUM) — non-atomic mastery read-modify-write — deferred
Still a non-atomic RMW, but now on a single code path with an inline
NOTE. The DB-side atomic append (Postgres RPC) needs migration-runner plumbing and is a follow-up tied to #195/#197. (Deferred deliberately — see discussion on #128.)#24 (LOW) — directional edge dedup
Dedup now ignores orientation for symmetric relationship types (
related/similar) while keeping directional types (prerequisite/builds_on) distinct.Security note
This branch is rebased on top of the quiz IDOR fix (
b3952f0).apply_mastery_eventtakes an optionaluser_idso the write stays owner-scoped — the IDOR defense-in-depth is preserved, not weakened, andapply_graph_updatenow passesuser_idtoo.Testing
apply_mastery_event(incl. clamping, capped events, effective delta,user_idscoping), edge-orientation dedup, quiz course-context refresh.ruff check .clean (CI ratchet).Follow-up
Addresses #128.
🤖 Generated with Claude Code
Summary by CodeRabbit
Improvements
Tests