Uh oh!
There was an error while loading. Please reload this page.
fix(explore): resolve F1–F8 from the Chapter 2 exploration sweep - #462
Merged
Conversation
Fixes the eight findings from the 2026-07-29 /explore session (.explore/findings.md): - F6 study_guide: query `assignments` by enrollment_id, not the phantom user_id/course_id columns (enrollment-keyed table) — get_exams, _generate_and_insert, and get_courses all 500'd, bricking the whole study-guide feature. get_courses now delegates to graph_service.get_courses. - F5a agents/function_handlers_e2e: register note_summary / note_concepts / note_chat handlers — these are request-path agent tasks that 500'd with UnregisteredHandlerError in function mode. - F4 notes: /api/notes/user, the single-note read, and create now return the abstract course_id + course_code/name resolved from the offering (every note showed "Unknown course"). - F1/F3 graph_service.get_courses: collapse the per-enrollment fan-out to one row per course_id (most-recent enrollment as representative; node_count counted once; additive enrollment_ids/terms lists). Fixes the dashboard count, /tree chips, and every course picker (#449). - F2 onboarding.search_courses: dedup catalog results by course code so the rich/base seed same-code courses don't show as indistinguishable dupes. - F7 auth: fire an idempotent login-streak achievement check on approved Google sign-in so "First Steps" is actually granted (test-login left untouched — it contractually performs no DB writes). - F5b notetaker: surface toast.error(humanizeError(...)) on failed agent actions (Summarize/Extract/Generate quiz/Send to tutor were silent no-ops). - F8 Settings: prefill the profile form from the profile fetch so name/username aren't blank (data-loss risk on save). Verification: 1325 backend tests pass (+14 new regression tests), ruff + tsc clean; live backend re-check of F1/F2/F4/F5a/F6 all pass and the e2e oracles return 0 findings (down from 6). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Warning Review limit reached
Next review available in:45 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe PR updates enrollment-based course and exam resolution, adds first-login achievement checks and deterministic notetaker handlers, enriches note responses, supports multi-term frontend course filtering, improves Settings profile fallback, deduplicates onboarding courses, and adds regression coverage. ChangesCourse and enrollment resolution
First-login achievement grant
Notetaker action handling
Frontend course and semester scoping
Settings profile fallback
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
⚔️ Resolve merge conflicts 💡
🧪 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 | 9a4f63e | Commit Preview URL Branch Preview URL | Jul 30 2026, 05:49 AM |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/routes/onboarding.py (1)
30-47: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDedup runs after the DB
limit=20, so results can be silently truncated.
limit=20(line 34) caps rows fetched before the Python-sidecourse_codecollapse (lines 37-45) runs. If several of those 20 rows share acourse_code— exactly the scenario this PR's own tests describe (seed-* and rich-* demo schools both defining CS101/BIO110) — the returned list can end up far shorter than 20 distinct courses, even though more distinct courses exist beyond the fetch window. This defeats the purpose of the dedup fix for the very case it targets.🐛 Proposed fix: over-fetch, then truncate after dedup
rows = table("courses").select( "id,course_code,course_name", filters=filters, order="course_name.asc", - limit=20,+ limit=100, # over-fetch so post-filter dedup still yields up to PAGE_SIZE distinct codes ) deduped = [] seen_codes = set() for row in rows: code = (row.get("course_code") or "").strip().casefold() if code: if code in seen_codes: continue seen_codes.add(code) deduped.append(row) + if len(deduped) >= 20:+ break return {"courses": deduped}🤖 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/onboarding.py` around lines 30 - 47, Adjust the courses query and post-processing in the onboarding route so deduplication occurs over an over-fetched result set, then truncate the deduplicated list to 20 entries before returning. Preserve the existing case-insensitive, whitespace-normalized course_code handling in the dedup loop and ensure the final courses list never exceeds 20 distinct entries.
🧹 Nitpick comments (1)
backend/tests/test_auth_first_login_achievement.py (1)
41-127: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the shared mock Supabase fixture.
Replace the local table factory and dual
tablemonkeypatches with the shared fixture fromtests/conftest.py, then configure its rows for each scenario. This keeps route and service mocks aligned with the repository contract.As per coding guidelines, “Backend tests belong under
backend/tests/and run with pytest; use shared mock Supabase and mock Gemini fixtures fromtests/conftest.py.”🤖 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/tests/test_auth_first_login_achievement.py` around lines 41 - 127, Update drive_callback and _make_factory to use the shared mock Supabase fixture from tests/conftest.py instead of creating a local table factory and patching auth_module.table and ach_module.table independently. Configure the fixture’s users, achievement_triggers, user_achievements, and related table rows for each scenario while preserving insert capture for First Steps assertions.Source: Coding guidelines
🤖 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.
Inline comments:
In `@backend/routes/study_guide.py`:
- Around line 51-73: Scope the exam lookup in _generate_and_insert to the
requested offering_id as well as the user’s enrollments, using the existing
enrollment/offering relationship or established offering-scoping helper. Ensure
an exam from another offering is rejected even when it belongs to the same user,
while preserving the current cross-user authorization check.
In `@frontend/src/components/screens/Settings.tsx`:
- Around line 94-98: Update the settings initialization fields in Settings.tsx
to use nullish fallback instead of truthiness fallback, replacing || with ?? for
display_name, username, bio, location, and website so explicitly empty strings
remain preserved while null or undefined values still use the profile fallback.
---
Outside diff comments:
In `@backend/routes/onboarding.py`:
- Around line 30-47: Adjust the courses query and post-processing in the
onboarding route so deduplication occurs over an over-fetched result set, then
truncate the deduplicated list to 20 entries before returning. Preserve the
existing case-insensitive, whitespace-normalized course_code handling in the
dedup loop and ensure the final courses list never exceeds 20 distinct entries.
---
Nitpick comments:
In `@backend/tests/test_auth_first_login_achievement.py`:
- Around line 41-127: Update drive_callback and _make_factory to use the shared
mock Supabase fixture from tests/conftest.py instead of creating a local table
factory and patching auth_module.table and ach_module.table independently.
Configure the fixture’s users, achievement_triggers, user_achievements, and
related table rows for each scenario while preserving insert capture for First
Steps assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 806d99e6-f9e1-47ca-b2af-d8bb9e66c1b0
📒 Files selected for processing (15)
backend/agents/function_handlers_e2e.pybackend/routes/auth.pybackend/routes/notes.pybackend/routes/onboarding.pybackend/routes/study_guide.pybackend/services/graph_service.pybackend/tests/test_auth_first_login_achievement.pybackend/tests/test_e2e_function_handlers.pybackend/tests/test_graph_service.pybackend/tests/test_notes_routes.pybackend/tests/test_onboarding_routes.pybackend/tests/test_study_guide_routes.pyfrontend/src/app/(shell)/notetaker/page.tsxfrontend/src/components/screens/Settings.test.tsxfrontend/src/components/screens/Settings.tsx
Comment on lines
51
to
73
| def _generate_and_insert(user_id: str, offering_id: str, exam_id: str) -> dict: | ||
| """Generate a study guide, insert it into study_guides, and return | ||
| {content, generated_at}. | ||
| Study guides + the documents that feed them key on the OFFERING (0025); | ||
| the caller resolves the abstract course id to an offering first. | ||
| """ | ||
| # 1. Fetch exam info | ||
| exams = table("assignments").select( | ||
| "id,user_id,title,due_date,assignment_type,course_id", | ||
| filters={"id": f"eq.{exam_id}", "user_id": f"eq.{user_id}"}, | ||
| limit=1, | ||
| # 1. Fetch exam info. Assignments key on enrollment_id (no user_id/course_id | ||
| # column); scope to the user's own enrollments so one user can't generate a | ||
| # guide off another's exam. | ||
| enrollment_ids = [e["id"] for e in user_enrollment_ids(user_id)] | ||
| exams = ( | ||
| table("assignments").select( | ||
| "id,enrollment_id,title,due_date,assignment_type", | ||
| filters={ | ||
| "id": f"eq.{exam_id}", | ||
| "enrollment_id": f"in.({','.join(enrollment_ids)})", | ||
| }, | ||
| limit=1, | ||
| ) | ||
| if enrollment_ids | ||
| else [] | ||
| ) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
Exam lookup isn't scoped to the requested offering_id, allowing cross-course exam/offering mismatch.
_generate_and_insert receives offering_id and correctly scopes the documents query (line 90) and the persisted study_guides row (line 160) to it, but the exam lookup (lines 61-72) filters assignments by enrollment_id in (ALL of user_enrollment_ids(user_id)) — every enrollment across every course the user has, not just the one matching offering_id. A request with course_id=A (→ offering_id=X) plus an exam_id belonging to a different course the same user is enrolled in will pass this check and get persisted as a study_guides row tagged offering_id: X with content generated from the wrong course's exam. The comment above only guards against cross-user access, not cross-course mismatch for the same user.
🐛 Proposed fix: scope the exam lookup to the requested offering
- enrollment_ids = [e["id"] for e in user_enrollment_ids(user_id)]+ enrollment_ids = [+ e["id"] for e in user_enrollment_ids(user_id)+ if e.get("offering_id") == offering_id+ ]Also worth adding a regression test with multiple enrollments across different offerings to catch this class of bug — none of the current TestGetGuide/TestRegenerateGuide/TestGenerationFailure tests exercise more than one enrollment.
As per coding guidelines, "study/analytics data on offering_id" should govern how study-guide data is resolved and scoped.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def_generate_and_insert(user_id: str, offering_id: str, exam_id: str) ->dict: | |
| """Generateastudyguide, insertitintostudy_guides, andreturn | |
| {content, generated_at}. | |
| Studyguides+thedocumentsthatfeedthemkeyontheOFFERING (0025); | |
| thecallerresolvestheabstractcourseidtoanofferingfirst. | |
| """ | |
| # 1. Fetch exam info | |
| exams=table("assignments").select( | |
| "id,user_id,title,due_date,assignment_type,course_id", | |
| filters={"id": f"eq.{exam_id}", "user_id": f"eq.{user_id}"}, | |
| limit=1, | |
| # 1. Fetch exam info. Assignments key on enrollment_id (no user_id/course_id | |
| # column); scope to the user's own enrollments so one user can't generate a | |
| # guide off another's exam. | |
| enrollment_ids= [e["id"] foreinuser_enrollment_ids(user_id)] | |
| exams= ( | |
| table("assignments").select( | |
| "id,enrollment_id,title,due_date,assignment_type", | |
| filters={ | |
| "id": f"eq.{exam_id}", | |
| "enrollment_id": f"in.({','.join(enrollment_ids)})", | |
| }, | |
| limit=1, | |
| ) | |
| ifenrollment_ids | |
| else [] | |
| ) | |
| def_generate_and_insert(user_id: str, offering_id: str, exam_id: str) ->dict: | |
| """Generateastudyguide, insertitintostudy_guides, andreturn | |
| {content, generated_at}. | |
| Studyguides+thedocumentsthatfeedthemkeyontheOFFERING (0025); | |
| thecallerresolvestheabstractcourseidtoanofferingfirst. | |
| """ | |
| # 1. Fetch exam info. Assignments key on enrollment_id (no user_id/course_id | |
| # column); scope to the user's own enrollments so one user can't generate a | |
| # guide off another's exam. | |
| enrollment_ids= [ | |
| e["id"] foreinuser_enrollment_ids(user_id) | |
| ife.get("offering_id") ==offering_id | |
| ] | |
| exams= ( | |
| table("assignments").select( | |
| "id,enrollment_id,title,due_date,assignment_type", | |
| filters={ | |
| "id": f"eq.{exam_id}", | |
| "enrollment_id": f"in.({','.join(enrollment_ids)})", | |
| }, | |
| limit=1, | |
| ) | |
| ifenrollment_ids | |
| else [] | |
| ) |
🤖 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/study_guide.py` around lines 51 - 73, Scope the exam lookup in
_generate_and_insert to the requested offering_id as well as the user’s
enrollments, using the existing enrollment/offering relationship or established
offering-scoping helper. Ensure an exam from another offering is rejected even
when it belongs to the same user, while preserving the current cross-user
authorization check.
Source: Coding guidelines
Comment on lines
+94
to
+98
| display_name: s.display_name || profile?.name || null, | ||
| username: s.username || profile?.username || null, | ||
| bio: s.bio || profile?.bio || null, | ||
| location: s.location || profile?.location || null, | ||
| website: s.website || profile?.website || null, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Preserve explicitly empty settings values.
|| treats "" as missing, so an intentionally cleared settings field is replaced by the public-profile value. Because these values seed the form, a later blur can write the fallback back and undo the user’s explicit value. Use ?? instead.
Proposed fix
- display_name: s.display_name || profile?.name || null,- username: s.username || profile?.username || null,- bio: s.bio || profile?.bio || null,- location: s.location || profile?.location || null,- website: s.website || profile?.website || null,+ display_name: s.display_name ?? profile?.name ?? null,+ username: s.username ?? profile?.username ?? null,+ bio: s.bio ?? profile?.bio ?? null,+ location: s.location ?? profile?.location ?? null,+ website: s.website ?? profile?.website ?? null,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| display_name: s.display_name||profile?.name||null, | |
| username: s.username||profile?.username||null, | |
| bio: s.bio||profile?.bio||null, | |
| location: s.location||profile?.location||null, | |
| website: s.website||profile?.website||null, | |
| display_name: s.display_name??profile?.name??null, | |
| username: s.username??profile?.username??null, | |
| bio: s.bio??profile?.bio??null, | |
| location: s.location??profile?.location??null, | |
| website: s.website??profile?.website??null, |
🤖 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 `@frontend/src/components/screens/Settings.tsx` around lines 94 - 98, Update
the settings initialization fields in Settings.tsx to use nullish fallback
instead of truthiness fallback, replacing || with ?? for display_name, username,
bio, location, and website so explicitly empty strings remain preserved while
null or undefined values still use the profile fallback.
AndresL230
commented
Jul 29, 2026
CollaboratorAuthor
Code reviewFound 1 issue:
Sapling/backend/services/graph_service.py Lines 347 to 349 in 8324873 The single Sapling/backend/services/graph_service.py Lines 376 to 379 in 8324873 while consumers still filter by it per active semester, e.g.: Sapling/frontend/src/components/screens/Dashboard.tsx Lines 338 to 340 in 8324873 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
…ive term (PR #462 review) The #449 get_courses collapse keeps one row per abstract course_id with only the most-recent enrollment's singular `term`. But six screens filtered `c.term === activeSemester` and the semester tab bar was built from that singular `term`, so a course enrolled across two terms (CS101 in Fall 2025 + Spring 2026) dropped off its older-term tab — a regression of #360 semester scoping (caught by the adversarial review on #462, confirmed live: the Fall 2025 dashboard showed "1 course" while rendering two course hubs). - api.ts: document `terms`/`enrollment_ids` on EnrolledCourse (backend already returns them since the collapse). - useActiveSemester.ts: add `courseInTerm(course, activeSemester)` (term MEMBERSHIP; "" = all) and flatten `distinctTerms` over the `terms[]` array. - Dashboard/Study/Tree/Quiz/Learn/ManageCoursesModal: filter via `courseInTerm`; Quiz/Learn concepts now inherit the course `terms[]` so their (defensive) semester filter matches too. - useActiveSemester.test.ts: regression tests for membership + terms[] flatten. Also closes the F4 follow-up: the POST /api/notes create route now returns the resolved course_id/labels too (was "Unknown course" on a fresh note until reload). Verified live (Fall 2025 tab): CS101 + BIO110 both show, "2 courses"; all tabs resolve correctly (Fall→CS101,BIO110 / Spring→CS101,MATH210 / Summer→ENG150). tsc clean, affected vitest 16 passed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
AndresL230
commented
Jul 30, 2026
CollaboratorAuthor
Review finding addressed (ba6409e)The term-collapse regression is fixed.
Verified live in the browser (in-app date 2026-03-11, rich-user-active):
CS101 (enrolled Fall 2025 + Spring 2026) now correctly appears on both tabs. Also folded in the F4 create-route follow-up (a freshly created note now returns its resolved 🤖 Generated with Claude Code |
…findings-f1-f8 # Conflicts: # backend/tests/test_e2e_function_handlers.py
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Jul 30, 2026
AndresL230 added a commit
that referenced
this pull request
Jul 30, 2026
…view findings F1 (major): the recent-guides rail lists guides from ANY term, but opening one resolved with the ACTIVE semester under #141's strict mode — a multi-term course + other-term entry cache-missed on (offering, exam) and silently generated-and-PERSISTED a mismatched row; a course absent from the active term 404'd for a guide visibly in the sidebar. Invariant now: a recent entry opens AS ITS OWN TERM. - backend GET /{user}/cached: each entry carries its own `semester` label (term_for_offering, lru-cached, offering ids deduped in the existing enrichment loop); ETag key bumped to guides.v2 so bodies cached under the old shape revalidate. - frontend: StudyGuideCacheEntry.semester; openRecent records the entry's term ("" = term-less entry -> explicitly unscoped) in a ref consumed by exactly one load; loadGuide takes a per-load term override; the failed state stores the term so retry replays the exact load. Picker-driven loads keep following the active selector (unit-tested by driving the real CustomSelects). The #476 emergent examId-clear behavior is untouched and its tests stay green. F3: _generate_and_insert scopes the exam lookup to the RESOLVED offering's enrollment (the #462 CodeRabbit fix) — a two-term user can no longer generate a guide keyed on one term's offering from another term's exam (regression test: 404, nothing persisted). F2: flashcards _get_course_documents — a course-name miss WITH an explicit semester now contributes no documents (an explicit term gives the all-docs fallback nothing to anchor to); without a semester the pre-existing all-docs fallback is byte-identical (both pinned). F4: the guide 404 branch no longer hardcodes the exam-deleted copy — the server's detail renders when it isn't the exam-deleted sentence (no-offering-in-term case), still guidance, never a toast. F5: comment truth fixes — notes PATCH re-home relabeled (re-home, not create; still deliberately current-term); api.ts study-guide block now documents the pre-existing exams-list asymmetry (omitted semester = current-term resolution for guide/regenerate but ALL terms for getStudyGuideExams); Study.semester.test.tsx attributes the exam-clear to the courseId-keyed effect racing openRecent (#476), not to openRecent. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
AndresL230 added a commit
that referenced
this pull request
Jul 30, 2026
* feat(study): semester-scope the study-tool reads (#141) The approved reframe: NO Archive toggle. The existing semester selector (Courses & Semesters hub -> lib/useActiveSemester, "" = All semesters DEFAULT — untouched, e2e-pinned per #360) now scopes the STUDY-TOOL reads the same way it already scopes the graph. The study endpoints used to hardcode current-term resolution (resolve_offering(course_id)), so under the frozen e2e clock (spring-2026 current) fall-2025 study material was unreachable regardless of the user's selection. Backend — optional `semester` (term LABEL via term_id_for_label) on the course-scoped READ paths, resolved STRICTLY: an unknown label or a term with no offering of the course degrades to each route's empty/404 behavior, never a silent fall-back to another term. New `fallback=False` mode on academics.resolve_offering carries that rule (the default create=False path used to silently resolve ANY offering of the course on a term miss). - study_guide: GET /{user}/guide + POST /regenerate (404 on a term miss, and never generates for an offering that isn't there); GET /{user}/exams scopes the enrollment set to the selected term. - flashcards: GET /user/{user} filters cards to the selected term's offerings (term-LESS cards stay visible under any selection); POST /generate grounds its docs context in the selected term's offering (a term miss contributes no docs — not all-docs, not current term). import/commit stays a CREATE path: current term by design. - notes: GET /user/{user} course-filtered read takes `semester` (API completeness — the notetaker UI carries no semester context and is deliberately NOT wired); the create/re-home paths stay current-term by design (commented). - quiz: untouched (no term resolution; scoping is client-side). Frontend — Study.tsx threads `activeSemester || undefined` into getStudyGuideExams/getStudyGuide/regenerateStudyGuide/getFlashcards/ generateFlashcards, gated on the useActiveSemester hydrated flag exactly like Dashboard (call-count pinned: one scoped fetch, never unscoped-then-scoped). Tests: backend route + resolver coverage for (a) no semester = existing current-term behavior, (b) explicit term threads (course, term, fallback=False), (c) unknown/no-offering term = empty/404 not 500; vitest Study.semester.test.tsx pins the scoped/unscoped fetch args and the single-fetch hydration gate. New journey e2e/study-semester.spec.ts (authored, not run here): All semesters shows the fall AND spring decks together; picking Fall 2025 in the hub surfaces the fall-2025 CS Basics deck and hides the spring-only one. No new agent tasks introduced; the journey triggers no generation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(review): recent-guides open as their own term + close the #475 review findings F1 (major): the recent-guides rail lists guides from ANY term, but opening one resolved with the ACTIVE semester under #141's strict mode — a multi-term course + other-term entry cache-missed on (offering, exam) and silently generated-and-PERSISTED a mismatched row; a course absent from the active term 404'd for a guide visibly in the sidebar. Invariant now: a recent entry opens AS ITS OWN TERM. - backend GET /{user}/cached: each entry carries its own `semester` label (term_for_offering, lru-cached, offering ids deduped in the existing enrichment loop); ETag key bumped to guides.v2 so bodies cached under the old shape revalidate. - frontend: StudyGuideCacheEntry.semester; openRecent records the entry's term ("" = term-less entry -> explicitly unscoped) in a ref consumed by exactly one load; loadGuide takes a per-load term override; the failed state stores the term so retry replays the exact load. Picker-driven loads keep following the active selector (unit-tested by driving the real CustomSelects). The #476 emergent examId-clear behavior is untouched and its tests stay green. F3: _generate_and_insert scopes the exam lookup to the RESOLVED offering's enrollment (the #462 CodeRabbit fix) — a two-term user can no longer generate a guide keyed on one term's offering from another term's exam (regression test: 404, nothing persisted). F2: flashcards _get_course_documents — a course-name miss WITH an explicit semester now contributes no documents (an explicit term gives the all-docs fallback nothing to anchor to); without a semester the pre-existing all-docs fallback is byte-identical (both pinned). F4: the guide 404 branch no longer hardcodes the exam-deleted copy — the server's detail renders when it isn't the exam-deleted sentence (no-offering-in-term case), still guidance, never a toast. F5: comment truth fixes — notes PATCH re-home relabeled (re-home, not create; still deliberately current-term); api.ts study-guide block now documents the pre-existing exams-list asymmetry (omitted semester = current-term resolution for guide/regenerate but ALL terms for getStudyGuideExams); Study.semester.test.tsx attributes the exam-clear to the courseId-keyed effect racing openRecent (#476), not to openRecent. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(e2e): disambiguate the Linear Algebra pill (course pill + topic pill share the name under All semesters) Co-Authored-By: Claude Fable 5 <noreply@anthropic.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
1 participant
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.
What
Fixes all eight findings from the 2026-07-29 Chapter 2
/exploresweep (.explore/findings.md). Each fix was built by an isolated subagent (disjoint files), following systematic-debugging + TDD.routes/study_guide.pyqueriesassignmentsbyenrollment_id(was the phantomuser_id/course_idcolumns on the enrollment-keyed table).get_exams,_generate_and_insert, andget_courses(now delegates tograph_service.get_courses).note_summary/note_concepts/note_chathandlers inagents/function_handlers_e2e.py(request-path tasks were unregistered →UnregisteredHandlerError).routes/notes.pylist + single-read + create now return the abstractcourse_id+course_code/course_nameresolved from the note's offering.graph_service.get_coursescollapses the per-enrollment fan-out to one row percourse_id(most-recent enrollment as representative,node_countcounted once, additiveenrollment_ids/termslists).routes/onboarding.pydedups results by course code (rich/base seeds define same-code courses under different schools).routes/auth.pyfires an idempotent login-streak achievement check on approved Google sign-in. (test-loginuntouched — it contractually performs no DB writes.)notetaker/page.tsxsurfacestoast.error(humanizeError(...))on failed Summarize/Extract/Generate-quiz/Send-to-tutor.Settings.tsxprefills name/username (and bio/location/website) from the profile fetch.Verification
1325 passed(baseline 1311 + 14 new regression tests),ruffclean.tsc --noEmitclean; newSettings.test.tsx(2 cases) +errorMessage(32) pass.MATH210/CS101, F5a note actions → 200, F1/F3 courses → 4 rows/4 unique, F2 onboarding dedup → no code dupes, oracles: 0 findings (down from 6).Follow-ups deliberately left out of scope (flagged for triage)
limit=20cap and the endpoint has no school scoping — a genuine multi-school production catalog with two real same-code courses would over-collapse. Correct long-term fix is a distinguishing school label, not dedup.streak_count = 0still won't be granted on their very first login — thelogin_streakthreshold (1) vs. the "log in for the first time" wording is a mismatch in the 0007 seed trigger definition (needs a migration).createNoteIn,deleteActive, link/unlink concept, autosave) still onlyconsole.erroron failure.🤖 Generated with Claude Code
Summary by CodeRabbit