Uh oh!
There was an error while loading. Please reload this page.
fix(dashboard): don't seed course name as tutor topic on node click (#319) - #369
Merged
Conversation
📝 WalkthroughWalkthroughDashboard and tree navigation now use a shared helper for Learn URLs. Subject-root nodes omit topic parameters, course scope uses ChangesLearning navigation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1⚔️ Resolve merge conflicts 💡
📝 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 | 0062744 | Commit Preview URL Branch Preview URL | Jul 29 2026, 10:59 AM |
AndresL230
marked this pull request as draft
July 27, 2026 23:50
…319) Clicking a course (subject-root) node on the homepage graph opened the AI Tutor with the course name pre-filled as the session topic. The node-click handlers unconditionally set `topic=n.name`; for a course node that name is the course itself. Add a shared `learnHrefForNode` helper in lib/data.ts that omits the topic for subject-root nodes (scoping the tutor to the course with an empty topic picker instead) and seeds `topic` only for concept nodes. Wire it into both Dashboard graph handlers and Tree's "Learn this". Also fixes a latent scope-drop in the same handlers: the course was passed as `course_id`, but the Learn screen reads the `course` param, so course scope was silently lost on every node click. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…319) Follow-up to the tutor fix. The Tree detail panel's "Quick quiz" button ran onQuiz for any node, including subject-root (course) nodes, passing `topic=<course name>`. Unlike the tutor, the Quiz screen resolves `topic` against concepts (roots excluded) so the course name never actually became the quiz topic — but the param was dead and misleading. Guard onQuiz so course nodes open the quiz picker with no seeded topic (mirrors the tutor behavior). Also drop the `course_id` query param: the Quiz screen only reads `topic`/`concept`, so it was dead. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
AndresL230force-pushed
the
fix/319-course-node-topic
branch
from
July 29, 2026 10:57
4a17bc7 to
0062744CompareAndresL230
marked this pull request as ready for review
July 29, 2026 10:57
Uh oh!
There was an error while loading. Please reload this page.
AndresL230 added a commit
that referenced
this pull request
Jul 29, 2026
Rework of PR #360 (feat/semester-scoped-learning) replayed onto current main as a single commit, resolving the drift accumulated since the merge-base (~40 commits, incl. the #349 Learn.tsx SSE rewrite, the #355 subject-root dedup, the #352 content-hash chunk ids, and the #140 current/archive dashboard split, which the semester tabs supersede per the branch's own reconciliation). Backend semester scoping: - services/academics.py: term_id_for_label (semester label -> term id, with term-id fallback) + user_course_ids_for_term (enrollments -> offerings -> term-filtered course ids). - services/graph_service.py: get_graph/get_recommendations accept an optional semester label; nodes/edges/stats and the synthesized subject roots are restricted to that term's courses (composes with the #355 per-course subject-root dedup — roots are built from the already- filtered enrollment list). - add_course: no-retake rule across ALL terms (returns already_existed with the existing term label) and an optional term label so the hub enrolls into the tab being viewed, falling back to the current term. - routes/graph.py: optional ?semester= on GET /api/graph/{user_id} and /recommendations; AddCourseBody gains term. - routes/gradebook.py: private _term_id_for_semester duplicate removed; call sites now use the shared academics.term_id_for_label (follow-up noted on the original PR). Frontend semester scoping: - lib/useActiveSemester.ts: localStorage-backed active-semester hook (cross-tab sync, hydration flag so first fetches are scoped once). - Dashboard/Tree/Learn/Quiz/Study fetch the graph scoped to the active semester and scope course/concept pickers to it; Learn's scoping was redone by hand against main's SSE-streaming Learn.tsx (pass the active semester into the bootstrap getGraph, term on concepts, scopedCourses for the course select, TopicPicker term filter — main's own suggest/highlight logic kept, not duplicated). - ManageCoursesModal becomes the Courses & Semesters hub (per-term tabs, enroll-into-tab, no-retake feedback); the Dashboard archive rail from #140 is replaced by the active-semester scoping. - vitest.setup.ts: install an in-memory localStorage when the jsdom environment exposes none (jsdom 29 under Vitest leaves window.localStorage undefined), so useActiveSemester/useLayoutPref are testable in DOM tests. Per-type document chunking + tutor integrity: - services/chunker.py: chunk_for_category routes prose-like categories (essays/assignments) to a sentence-aware prose chunker, others to the existing chunker; routes/documents.py passes the doc category through; backfill script follows. Composes with #352's content-addressed chunk ids (sha256 over chunk text — only boundaries change). - prompts/preamble.txt: academic-integrity rule for the tutor (guide, never hand over graded-work answers). Conflict resolutions: test_document_indexing.py keeps both main's #439 relevance-gate test (repointed at the chunk_for_category seam) and the branch's category-passthrough test; Dashboard.tsx keeps main's IS_TEST_MODE/now + #369 learnHrefForNode imports alongside courseTermLabels; eslint-suppressions.json re-baselined (Dashboard no-restricted-syntax count drops with the removed archive UI). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
AndresL230 added a commit
that referenced
this pull request
Jul 29, 2026
Rework of PR #360 (feat/semester-scoped-learning) replayed onto current main as a single commit, resolving the drift accumulated since the merge-base (~40 commits, incl. the #349 Learn.tsx SSE rewrite, the #355 subject-root dedup, the #352 content-hash chunk ids, and the #140 current/archive dashboard split, which the semester tabs supersede per the branch's own reconciliation). Backend semester scoping: - services/academics.py: term_id_for_label (semester label -> term id, with term-id fallback) + user_course_ids_for_term (enrollments -> offerings -> term-filtered course ids). - services/graph_service.py: get_graph/get_recommendations accept an optional semester label; nodes/edges/stats and the synthesized subject roots are restricted to that term's courses (composes with the #355 per-course subject-root dedup — roots are built from the already- filtered enrollment list). - add_course: no-retake rule across ALL terms (returns already_existed with the existing term label) and an optional term label so the hub enrolls into the tab being viewed, falling back to the current term. - routes/graph.py: optional ?semester= on GET /api/graph/{user_id} and /recommendations; AddCourseBody gains term. - routes/gradebook.py: private _term_id_for_semester duplicate removed; call sites now use the shared academics.term_id_for_label (follow-up noted on the original PR). Frontend semester scoping: - lib/useActiveSemester.ts: localStorage-backed active-semester hook (cross-tab sync, hydration flag so first fetches are scoped once). - Dashboard/Tree/Learn/Quiz/Study fetch the graph scoped to the active semester and scope course/concept pickers to it; Learn's scoping was redone by hand against main's SSE-streaming Learn.tsx (pass the active semester into the bootstrap getGraph, term on concepts, scopedCourses for the course select, TopicPicker term filter — main's own suggest/highlight logic kept, not duplicated). - ManageCoursesModal becomes the Courses & Semesters hub (per-term tabs, enroll-into-tab, no-retake feedback); the Dashboard archive rail from #140 is replaced by the active-semester scoping. - vitest.setup.ts: install an in-memory localStorage when the jsdom environment exposes none (jsdom 29 under Vitest leaves window.localStorage undefined), so useActiveSemester/useLayoutPref are testable in DOM tests. Per-type document chunking + tutor integrity: - services/chunker.py: chunk_for_category routes prose-like categories (essays/assignments) to a sentence-aware prose chunker, others to the existing chunker; routes/documents.py passes the doc category through; backfill script follows. Composes with #352's content-addressed chunk ids (sha256 over chunk text — only boundaries change). - prompts/preamble.txt: academic-integrity rule for the tutor (guide, never hand over graded-work answers). Conflict resolutions: test_document_indexing.py keeps both main's #439 relevance-gate test (repointed at the chunk_for_category seam) and the branch's category-passthrough test; Dashboard.tsx keeps main's IS_TEST_MODE/now + #369 learnHrefForNode imports alongside courseTermLabels; eslint-suppressions.json re-baselined (Dashboard no-restricted-syntax count drops with the removed archive UI). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
AndresL230 added a commit
that referenced
this pull request
Jul 29, 2026
* feat: semester-scoped learning + Courses & Semesters hub (#360, rebased) Rework of PR #360 (feat/semester-scoped-learning) replayed onto current main as a single commit, resolving the drift accumulated since the merge-base (~40 commits, incl. the #349 Learn.tsx SSE rewrite, the #355 subject-root dedup, the #352 content-hash chunk ids, and the #140 current/archive dashboard split, which the semester tabs supersede per the branch's own reconciliation). Backend semester scoping: - services/academics.py: term_id_for_label (semester label -> term id, with term-id fallback) + user_course_ids_for_term (enrollments -> offerings -> term-filtered course ids). - services/graph_service.py: get_graph/get_recommendations accept an optional semester label; nodes/edges/stats and the synthesized subject roots are restricted to that term's courses (composes with the #355 per-course subject-root dedup — roots are built from the already- filtered enrollment list). - add_course: no-retake rule across ALL terms (returns already_existed with the existing term label) and an optional term label so the hub enrolls into the tab being viewed, falling back to the current term. - routes/graph.py: optional ?semester= on GET /api/graph/{user_id} and /recommendations; AddCourseBody gains term. - routes/gradebook.py: private _term_id_for_semester duplicate removed; call sites now use the shared academics.term_id_for_label (follow-up noted on the original PR). Frontend semester scoping: - lib/useActiveSemester.ts: localStorage-backed active-semester hook (cross-tab sync, hydration flag so first fetches are scoped once). - Dashboard/Tree/Learn/Quiz/Study fetch the graph scoped to the active semester and scope course/concept pickers to it; Learn's scoping was redone by hand against main's SSE-streaming Learn.tsx (pass the active semester into the bootstrap getGraph, term on concepts, scopedCourses for the course select, TopicPicker term filter — main's own suggest/highlight logic kept, not duplicated). - ManageCoursesModal becomes the Courses & Semesters hub (per-term tabs, enroll-into-tab, no-retake feedback); the Dashboard archive rail from #140 is replaced by the active-semester scoping. - vitest.setup.ts: install an in-memory localStorage when the jsdom environment exposes none (jsdom 29 under Vitest leaves window.localStorage undefined), so useActiveSemester/useLayoutPref are testable in DOM tests. Per-type document chunking + tutor integrity: - services/chunker.py: chunk_for_category routes prose-like categories (essays/assignments) to a sentence-aware prose chunker, others to the existing chunker; routes/documents.py passes the doc category through; backfill script follows. Composes with #352's content-addressed chunk ids (sha256 over chunk text — only boundaries change). - prompts/preamble.txt: academic-integrity rule for the tutor (guide, never hand over graded-work answers). Conflict resolutions: test_document_indexing.py keeps both main's #439 relevance-gate test (repointed at the chunk_for_category seam) and the branch's category-passthrough test; Dashboard.tsx keeps main's IS_TEST_MODE/now + #369 learnHrefForNode imports alongside courseTermLabels; eslint-suppressions.json re-baselined (Dashboard no-restricted-syntax count drops with the removed archive UI). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(360): review fixes — race-safe add_course, honest add toasts, single scoped dashboard fetch Review follow-ups on the semester-scoped learning rework (#360): - add_course race (A): new partial unique index (migration 0036) closes the NULL-section gap in course_offerings_unique; academics.resolve_offering (create=True) now catches the 409 from a lost create race and re-selects the winner's offering; the ManageCoursesModal Add button disables while an add request is in flight. The now-unreachable per-offering already-enrolled check in add_course is removed and the single return contract documented. - add toast honesty (B): handleAdd reads the response — already_existed shows an informational "Already taken in <term>" toast instead of a false success; success only toasts when a row was created. - CoursesKey empty state (C): "Nothing enrolled this semester." is reachable — the key stays rendered when courses exist but the active semester scopes to none; still null when there are no courses at all. - dashboard first load (D): with no stored semester, resolve + persist the default (courses + semesters fetch) BEFORE the scoped fetch and early-return; the effect re-run performs the single scoped fetch. Zero-courses stays a single unscoped pass. - cross-screen default (E): new useActiveSemester.ensureDefaultActiveSemester (termLabels) persists a default when none is stored, called from Learn/Quiz/Tree/Study once course term labels are in hand. - prune (F): drop orphaned partitionCurrentAndArchive, groupCoursesByTerm, UNKNOWN_TERM_LABEL, CoursePartition (+ TermGroup) and their test blocks. Tests: resolve_offering conflict-retry + non-409 propagation units; add_course duplicate-path contract; ManageCoursesModal component tests for the already-existed/success/in-flight paths; Dashboard single-scoped-fetch, zero-courses and CoursesKey empty-state renders; ensureDefaultActiveSemester units. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * rework(360): default = All semesters — semester scoping is opt-in (e2e veto) The e2e lane vetoed the auto-default: the rich seed spans Fall 2025 / Spring 2026 / Summer 2026 and 7 of 11 journeys failed because the auto-resolved term silently hid cross-term fixtures (dashboard lost MATH210; the graph shrank to 1 of 17 nodes). New semantics: - Default = ALL SEMESTERS (unscoped). An empty stored active-semester value IS the default and means "All semesters": removed ensureDefaultActiveSemester + its Learn/Quiz/Tree/Study call sites, Dashboard's default-resolution early-return pass (deferredToScopedPass) and its getSemesters/courseTermLabels/resolveActiveSemester usage. resolveActiveSemester itself is deleted (orphaned — the hub now reads the stored value raw). Hook, change events, and hydration gating stay. - Hub UI: ManageCoursesModal grows an explicit "All semesters" tab — active when the stored value is empty, clicking it clears the value; term tabs unchanged (both carry aria-pressed). A picked term still persists and scopes every surface. - Kept from the review batch: CoursesKey empty-state reachability, the already_existed info toast, in-flight Add disable, the 0036 partial unique index + resolve_offering conflict retry, semesters.ts pruning. - New journey frontend/e2e/semester-scope.spec.ts: default shows cross-term courses together (MATH210 + BIO110), picking Fall 2025 in the hub hides Spring-only MATH210, "All semesters" restores it. Hub opens via new dashboard-courses-manage testid (documented in docs/frontend-testids.md); tabs selected by role/name. - Tests reworked: Dashboard default test now asserts one unscoped fetch with both terms visible; /api/semesters-failure test dropped (no semesters fetch remains); ensureDefault unit tests replaced by hub tab component tests (All active by default / term persists / All clears). - Design doc amendment records the veto and the opt-in scoping decision. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: AndresL230 <190146319+AndresL230@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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.
Closes#319.
Problem
Clicking a course node on the homepage knowledge graph redirected to the AI Tutor with the course name pre-filled as the session topic. The node-click handlers unconditionally set
topic=n.name; for a subject-root (course) node that name is the course.Fix
learnHrefForNode(node)infrontend/src/lib/data.ts:topicfor subject-root nodes → tutor opens scoped to the course with an empty topic picker so the learner chooses a concept;topic=nameonly for concept nodes.Dashboard.tsxgraph handlers (compact + fullscreen) andTree.tsx's "Learn this", replacing duplicated inline URL building.course_id, but the Learn screen readssearchParams.get("course")— so course scope was silently dropped on every node click. The helper now passescourse.Tests
data.test.ts(concept seeds topic; course node never does — Clicking on a course node will result in the course name being selected as a course topic automatically #319 guard; scoping viacoursenotcourse_id; param omitted with no course; custom mode).tsc --noEmitandeslinton changed files: clean.styleTextincompatibility, unrelated), so the real compiled helper was verified end-to-end viatsx— all checks pass.Base
Stacked on
feat/semester-scoped-learning(PR #360): the homepage-graph handlers this touches only exist in this form on that branch.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests