Uh oh!
There was an error while loading. Please reload this page.
fix(frontend): surface the five swallowed failures — closes the #113 audit tail (#166 #185 #184 #183 #186) - #463
Merged
Merged
Conversation
…, calendar load, empty quiz, paste-tab wipe, inline course (#166#185#184#183#186) Every remaining #113 audit child is the same shape: a failure rendered as a plausible success. Each fix pairs with a red-first component test pinning the failure path. - #166 gradebook: AssignmentModal/EditWeightsModal/LetterScaleEditor saves (and AssignmentModal delete) catch + toast via humanizeError and STAY OPEN on failure; same treatment for the in-file CurveSettingsModal and LetterScaleEditor's floating reset. Parents keep bare awaits (rejections propagate into the modal's catch — documented). - #185 calendar: load() failure now renders a loadError banner (calendar-load-error / calendar-load-retry, registered in docs/frontend-testids.md) distinct from the legitimate empty state; retry re-runs load through the skeleton. - #184 quiz: an empty generated-questions response warns and stays in the select phase instead of stranding the user on a control-less blank panel. - #183 flashcards: PasteTab's live-reparse effect is gated on a per-mount dirty ref — a bare (re)mount never emits onCards([]) over the shared deck; deleting typed text still clears intentionally. First flashcards test file covers the tab-switch regression + both controls. - #186 upload modal: inline-added courses land in local extraCourses, merged (deduped) into the per-file dropdown, the courses[0] default and the <10 gate; reset on close; parent refresh via onComplete unchanged. Suites: vitest 257 passed (31 files; 15 new tests), tsc clean, lint 0 errors, backend suite untouched-green. Closes#166. Closes#185. Closes#184. Closes#183. Closes#186. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Warning Review limit reached
Next review available in:2 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 (19)
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 | c8f4a3f | Commit Preview URL Branch Preview URL | Jul 29 2026, 08:16 PM |
AndresL230
commented
Jul 29, 2026
CollaboratorAuthor
Code reviewFound 2 issues:
Sapling/frontend/src/components/screens/Calendar.tsx Lines 281 to 292 in cff625f
Sapling/frontend/eslint.config.mjs Lines 54 to 68 in cff625f Lower-confidence items (below threshold, noted for the author): #186 (DocumentUploadModal) and #184 (QuizPanel) are in e2e-covered territory (upload.spec.ts / quiz.spec.ts) but ship Vitest-only regression coverage — the CLAUDE.md convention pairs such fixes with promoted journeys; the PR description names CurveSettingsModal and LetterScaleEditor's "Reset to default" as covered #166 instances but neither catch path has a test; two test-file headers (modals.dialog.test.tsx, Calendar.test.tsx) were made stale by this PR's own additions. 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
- Calendar: gate the full-page error banner to the INITIAL load only (hasLoadedRef) — a failing background reload (post-delete/export, reconnect, upload onComplete) now toasts and keeps the loaded view instead of blanking real data behind the banner; regression test drives a table-row delete whose reload fails. - Register the calendar surface for lint enforcement: Calendar.tsx added to eslint.config.mjs's no-restricted-syntax files array (20 pre-existing untagged elements baselined via lint:baseline) + the missing 'Where the testids live' table row. - Promoted journeys for the two fixes in e2e-covered territory: quiz.spec (#184 — route-intercepted zero-question generation stays on select with the warning) and upload.spec (#186 — inline-add of the unenrolled seeded HIST200 through the REAL catalog search/enroll routes, immediately selectable in the file row's dropdown, enrollment asserted in Postgres). - The two #166 catch paths the PR named but didn't test are now tested: LetterScaleEditor's Reset-to-default rejection, and CurveSettingsModal (exported from Course.tsx for its new dedicated test file). - Refreshed both stale test-file headers (modals.dialog.test.tsx, Calendar.test.tsx). vitest 273 passed (34 files), tsc clean, lint 0 errors. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
AndresL230
commented
Jul 29, 2026
CollaboratorAuthor
All five review findings fixed in c8f4a3f: (1) the error banner is now gated to the initial load — a failing background reload toasts and keeps the loaded view (regression test drives a table delete whose reload fails); (2) Calendar.tsx registered in the eslint testid-enforcement files array (pre-existing untagged elements baselined) + the missing owning-file table row; (3) promoted journeys added for both e2e-covered fixes — quiz.spec.ts (zero-question generation via route interception) and upload.spec.ts (inline-add of unenrolled seeded HIST200 through the real catalog/enroll routes, DB-asserted); (4) LetterScaleEditor Reset-to-default and CurveSettingsModal rejection paths now tested (modal exported for its dedicated test file); (5) both stale test headers refreshed. vitest 273/34 green, tsc clean, lint 0 errors. Full e2e cycle queued; results to follow. |
AndresL230
commented
Jul 30, 2026
CollaboratorAuthor
Pre-merge e2e gate: full lane 19/19 passed (including the two newly promoted journeys — quiz zero-question guard, upload inline-course) + oracles clean (0 findings, 1 allowlisted). Merging. |
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Jul 30, 2026
Closed
Closed
Closed
AndresL230 added a commit
that referenced
this pull request
Jul 30, 2026
The gradebook's term switcher was half-wired: the landing had semester chips, but the course-card link and getGradebookCourse dropped the selected term, so _resolve_enrollment(user, course, None) resolved the CURRENT term — a course also taken in an archived term (rich seed: CS101 in both fall-2025 and spring-2026) 404'd from that term's chip. And the backend's GPA endpoints had no frontend consumer at all. - CourseCard carries the selected term (?semester=<label>) on the card href; the course screen reads it off location (same pattern as Landing's deep-link param) and passes it through getGradebookCourse. - GradebookSummary stops discarding the summary's gpa/semester; the landing surfaces "Term GPA x.xx" next to the chips (gradebook-term-gpa, hidden while null). - New TranscriptModal (gradebook-transcript-open) over the previously unconsumed GET /api/gradebook/gpa: cumulative GPA (gradebook-transcript-gpa) + per-semester sections, in-progress rows listed but excluded from the math. Load failure toasts + inline retry (#463 pattern). - New pure lib/transcript.ts (buildTranscript/weightedGpa) mirroring backend gradebook_service.weighted_gpa exactly (null grade_points skipped, null/zero credits count as 1, empty -> null); term ordering reuses lib/semesters (new compareTermLabels export). - Testid surface `gradebook` registered in docs/frontend-testids.md + the four owning files joined the eslint no-restricted-syntax block; suppressions baseline regenerated for the pre-existing untagged elements (Course.tsx 9, Landing.tsx 1). - e2e/gradebook.spec.ts (authored, not run here): the promoted #139 regression journey (DB-truth precondition via queryRaw, then both chips' CS101 cards resolve their own term's categories) + the transcript journey. - vitest: lib/transcript.test.ts, TranscriptModal.test.tsx (dialog contract + loading/error/retry), Course.semester.test.tsx (param plumbing), Landing.test.tsx extended (term GPA, real CourseCard href under test via a next/link stub). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
AndresL230 added a commit
that referenced
this pull request
Jul 30, 2026
#468) * feat(gradebook): term-aware course links + transcript & GPA (#139) The gradebook's term switcher was half-wired: the landing had semester chips, but the course-card link and getGradebookCourse dropped the selected term, so _resolve_enrollment(user, course, None) resolved the CURRENT term — a course also taken in an archived term (rich seed: CS101 in both fall-2025 and spring-2026) 404'd from that term's chip. And the backend's GPA endpoints had no frontend consumer at all. - CourseCard carries the selected term (?semester=<label>) on the card href; the course screen reads it off location (same pattern as Landing's deep-link param) and passes it through getGradebookCourse. - GradebookSummary stops discarding the summary's gpa/semester; the landing surfaces "Term GPA x.xx" next to the chips (gradebook-term-gpa, hidden while null). - New TranscriptModal (gradebook-transcript-open) over the previously unconsumed GET /api/gradebook/gpa: cumulative GPA (gradebook-transcript-gpa) + per-semester sections, in-progress rows listed but excluded from the math. Load failure toasts + inline retry (#463 pattern). - New pure lib/transcript.ts (buildTranscript/weightedGpa) mirroring backend gradebook_service.weighted_gpa exactly (null grade_points skipped, null/zero credits count as 1, empty -> null); term ordering reuses lib/semesters (new compareTermLabels export). - Testid surface `gradebook` registered in docs/frontend-testids.md + the four owning files joined the eslint no-restricted-syntax block; suppressions baseline regenerated for the pre-existing untagged elements (Course.tsx 9, Landing.tsx 1). - e2e/gradebook.spec.ts (authored, not run here): the promoted #139 regression journey (DB-truth precondition via queryRaw, then both chips' CS101 cards resolve their own term's categories) + the transcript journey. - vitest: lib/transcript.test.ts, TranscriptModal.test.tsx (dialog contract + loading/error/retry), Course.semester.test.tsx (param plumbing), Landing.test.tsx extended (term GPA, real CourseCard href under test via a next/link stub). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(review): term-scope gradebook mutations + transcript credits display (#468 review) Closes the three findings from the #468 5-agent review of the #139 work. 1 (major) — read/write term split: getGradebookCourse was semester-aware but every course-keyed mutation from the Course screen still resolved term-blind, so editing a Fall 2025 CS101 page wrote into the Spring 2026 enrollment silently (backend _resolve_enrollment(course, None) = current term). The backend body models already accept `semester`; now the frontend sends it: optional `semester` on createCategory, bulkUpdateCategories, createGradedAssignment, setLetterScale and setCurveSettings in lib/api.ts (body field, JSON.stringify drops it when unset), passed at every Course.tsx call site (curve toggle + curve settings + weights + create-assignment + letter scale). The id-keyed calls (deleteCategory, update/deleteGradedAssignment) resolve by row ownership — verified against routes/gradebook.py — and stay as-is. - Course.semester.test.tsx: 4 new tests drive the REAL EditWeightsModal/AssignmentModal down to Save and assert bulkUpdateCategories/createGradedAssignment get the URL's semester when ?semester= is set, and undefined when absent. - e2e/gradebook.spec.ts: the Fall leg now also creates an assignment through the UI and queryRaw-polls that the new assignments row hangs off rich-enr-active-cs101-f25, not the spring enrollment. New testids gradebook-add-assignment / gradebook-assignment-title / gradebook-assignment-save; AssignmentList.tsx + AssignmentModal.tsx joined the eslint enforcement array (pre-existing untagged elements baselined: 7 + 12) and the docs inventory. 2 — transcript credits display: the modal showed `credits ?? 1` while the GPA math treated 0/negative as 1. New effectiveCredits() export in lib/transcript.ts is now the ONE place the rule lives, used by both weightedGpa and the "N cr" display; tests cover 0/negative/null and the rendered "1 cr" for a zero-credit row. 3 — ?semester= is read at FETCH time: the mount-frozen useState initializer became currentSemesterParam(), read inside refresh AND inside every mutation callback (keeps the no-Suspense plain-location pattern). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(gradebook): adapt #467's Landing.testmode suite to the #139 Landing contract (ToastProvider stub, getGpa mock, summary gpa/semester fields) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
AndresL230 added a commit
that referenced
this pull request
Jul 30, 2026
… guard, range clamp, testid rename - useAnalyticsQuery: monotonic request sequence so an out-of-order response can't overwrite newer data; background-reload failures keep the loaded view and toast (#463 Calendar convention) instead of vanishing. - Custom range inputs clamp the other edge (backend 422s from > to) and carry min/max bounds. - admin-analytics-costgroup-* -> admin-analytics-cost-group-* per the testids kebab convention (code + doc inventory). - TruncatedBadge copy made meaning-neutral for the errors panel's series-only truncation (#122 note). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
AndresL230 added a commit
that referenced
this pull request
Jul 30, 2026
… raw admin page (#478) * feat(analytics): day bucketing + wire-format fix for the range object (#121 backend) - Optional ?bucket=day on /usage/summary, /llm/cost, /errors adds a sparse UTC-day series (days with no rows omitted; client zero-fills from range). The errors series runs its own capped scan and surfaces truncation. - Range now serializes as {"from", "to"} — the query param and the wire key agree before any TS client freezes on the accidental "from_". Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(frontend): admin analytics data layer + raw /admin/analytics page (#121) - Typed wrappers (adminUsageSummary/ByUser/LlmCost/Errors) over the #120 API, omitting unset params so backend defaults stay server-owned; response types in types.ts named clear of the legacy AnalyticsOverview family. - Data hooks (useAdminAnalytics.ts) on the house useCallback+useEffect pattern; presetRange routes through lib/testMode's clock. - /admin/analytics renders live data as raw tables: usage summary, top users, LLM cost with group-by toggle, error feed. One range drives every panel; per-panel error && !data banners with retry; truncation badges. The admin gate sits OUTSIDE the hook-owning body so a non-admin visit fires zero 403ing requests. - admin-analytics testid surface registered in BOTH docs/frontend-testids.md and the eslint no-restricted-syntax files array. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(analytics): address PR #478 review — reload toast, stale-response guard, range clamp, testid rename - useAnalyticsQuery: monotonic request sequence so an out-of-order response can't overwrite newer data; background-reload failures keep the loaded view and toast (#463 Calendar convention) instead of vanishing. - Custom range inputs clamp the other edge (backend 422s from > to) and carry min/max bounds. - admin-analytics-costgroup-* -> admin-analytics-cost-group-* per the testids kebab convention (code + doc inventory). - TruncatedBadge copy made meaning-neutral for the errors panel's series-only truncation (#122 note). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 30, 2026
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
Bundle B2 of the backlog clear — the last five children of the frontend UI-audit epic #113, all the same shape: a failure rendered as a plausible success. Each fix pairs with a red-first component test pinning the failure path (per-issue detail in the commit message). Implemented as five parallel single-surface changes:
humanizeError) and keep the modal open on failure; covers the two adjacent instances of the same swallow (CurveSettingsModal, LetterScaleEditor reset). Parents keep bare awaits — rejections propagate to the modal's catch (documented in-code).calendar-load-error/calendar-load-retry, registered indocs/frontend-testids.mdas a new surface) instead of a normal-looking empty calendar.extraCoursesmerged + deduped, reset on close); also fixes the zero-course default and the<10gate.Verification
tsc --noEmitclean; lint 0 errors, no stale suppressionsmain@9b000b5(post-fix(learn): consume /learn?resume= deep links (#164) + ADR-0020 interrupted-turn Retry + promoted streaming journeys (#356) #461)upload.spec.ts) exercises the modified DocumentUploadModal directly.Closes#166. Closes#185. Closes#184. Closes#183. Closes#186.
🤖 Generated with Claude Code