Uh oh!
There was an error while loading. Please reload this page.
Batch: social-directory scoping (#342), auth stub promotion (#285), CodeRabbit on integration branches (#343) - #353
Merged
Conversation
CodeRabbit reviews only the default branch (main) unless base branches are listed explicitly, and this repo had no .coderabbit.yaml at all — so every PR targeting a long-lived integration branch got "Review skipped" while CI still went green, making the absence of a review read as "nothing to flag." The gap that still matters is `production`: main -> production promotion PRs (#305, #314) are the last gate before prod and were never machine-reviewed. `staging` is listed defensively — that branch was deleted 2026-07-15 (after #334 merged / #337 closed) and no open PR currently targets a non-main base, but it was a real integration branch and should be covered if recreated. Patterns are anchored regex so `staging` can't also match feature branches like `docs/staging-environment-plan`. Validated against coderabbit schema.v2.json. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Google sign-in 500'd whenever a stub `users` row existed (id set, google_id
NULL) — created by `graph_service.ensure_user_exists` for any authenticated
graph request, and reachable after a row delete because the HMAC-signed
`sapling_session` cookie outlives it. The new-user branch looked users up by
google_id only (NULL never matches), fell through to a blind INSERT on the
deterministic id `user_{google_id}`, and collided on users_pkey -> unhandled
409 -> permanent sign-in 500 loop.
Swap both blind inserts (users and user_profiles — the latter's user_id is also
a PK and 409s for a stub that reached onboarding) for upserts. on_conflict is
the primary key, not google_id: the stub's google_id is NULL and NULLs never
conflict, so only an id-conflict resolves it; merge-duplicates fills the stub's
NULL auth columns in, promoting it to a real user. The existing-user branch is
left untouched so a legacy row whose id != user_{google_id} keeps its id.
Tests drive the real /google/callback with the mocked users/user_profiles
tables raising the exact production 409 on insert, so any regression to a blind
insert fails loudly.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>get_students returned a profile for every user in the DB — name, streak,
courses, and per-concept mastery — to any authenticated caller; the session
user_id was bound and never used, so it authenticated without authorizing.
Two boundaries now apply, matching what the UI already claims ("Students at
your school") and the existing profile_visibility precedent:
- School scope via a new bulk helper `academics.school_peer_user_ids`, which
walks enrollments -> course_offerings.course_id -> courses.school_id and back
to every user enrolled at those schools. Multi-step reads per the module's
house style; not cached (it's a mutable visibility boundary); fails closed on
an empty scope, mirroring the enrollment-scoping pattern in calendar.py.
- profile_visibility: 'private' users are dropped from the listing. 0031 widens
the user_settings CHECK to allow the 'school' tier the Settings UI has always
offered but the DB rejected (a latent 500), and update_settings now validates
the value so a bad one returns 400 instead of a raw CHECK violation.
The payload is trimmed to name/streak/courses — the mastery histogram and top
concepts are academic-performance data that belong on the profile page (already
gated on profile_visibility), not in a browsable directory. Frontend directory,
StudentRow type, and local-mode fixture updated to match.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Free Run ID: 📒 Files selected for processing (12)
📝 WalkthroughWalkthroughThe PR adds school-scoped, visibility-aware student directories with reduced mastery-free payloads, updates frontend rendering and local data, makes Google OAuth provisioning conflict-safe for stub users, validates profile visibility values, and configures automated review branches. ChangesSchool Directory Visibility
OAuth Stub Promotion
Review Configuration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Viewer
participant StudentsRoute
participant Academics
participant Database
Viewer->>StudentsRoute: Request student directory
StudentsRoute->>Academics: Resolve school peers
Academics->>Database: Traverse enrollment and school relationships
Database-->>Academics: Peer user IDs
StudentsRoute->>Database: Read visibility and directory fields
Database-->>StudentsRoute: Visible users and courses
StudentsRoute-->>Viewer: Reduced student directory
Note 🎁 Summarized by CodeRabbit FreeYour organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login. Comment |
Deploying with |
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs | frontend-staging | 6a99676 | Commit Preview URL Branch Preview URL | Jul 17 2026, 03:15 PM |
AndresL230
marked this pull request as ready for review
July 17, 2026 16:20
Uh oh!
There was an error while loading. Please reload this page.
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.
Three independent fixes from the issue batch. Each is a self-contained commit; reviewable commit-by-commit.
Closes#342
Closes#285
Closes#343
#342 — scope
/api/social/studentsto the viewer's school (security, P2)get_studentsreturned a profile for every user in the DB — name, streak, courses, and per-concept mastery — to any authenticated caller. The sessionuser_idwas bound and never used, so the endpoint authenticated without authorizing.Decision (product call, confirmed): scope to school + honor
profile_visibility; trim mastery from the payload.academics.school_peer_user_ids(user_id)— walksenrollments → course_offerings.course_id → courses.school_idand back to every user enrolled at those schools. Multi-step reads per the module's house style; not cached (mutable visibility boundary); fails closed on empty scope, mirroring the enrollment-scoping pattern incalendar.py.profile_visibility == 'private'users are dropped from the listing. Migration0031widens theuser_settingsCHECK to allow the'school'tier the Settings UI has always offered but the DB rejected (a latent 500 on selecting it);update_settingsnow validates the value → 400 instead of a raw CHECK violation.name/streak/courses. The mastery histogram + top concepts are academic-performance data that belong on the profile page (already gated onprofile_visibility), not a browsable directory. Frontend directory,StudentRowtype, and local-mode fixture updated to match.0031is applied per-environment, not by merge:schema_migrationsledger; only0031was pending. Constraint is nowCHECK (profile_visibility = ANY (ARRAY['public','school','private'])). Safe to have applied ahead of the code — it only removes the pre-existing'school'500 in Settings.db.migrateagainst prod. Prod has noschema_migrationsledger (Prod migration ledger may have drifted from repo (staging did — 4 unrecorded migrations) #317), so the runner would treat all 34 migrations as pending and try to re-run them from scratch.--baselinealone is also wrong (it would mark0031applied without running it). Correct sequence as part of themain→productionpromotion: (1) baseline0001–0030on prod so the already-present schema is recorded, (2) then apply only0031for real. This fixes the Prod migration ledger may have drifted from repo (staging did — 4 unrecorded migrations) #317 gap for prod as a side effect and needs a human watching.#285 — promote stub users on sign-in instead of 409'ing (auth)
Sign-in 500'd whenever a stub
usersrow existed (id set,google_idNULL — created bygraph_service.ensure_user_exists, reachable after a row delete because the HMACsapling_sessioncookie outlives it). The new-user branch looked up bygoogle_idonly (NULL never matches), fell through to a blind INSERT on the deterministic iduser_{google_id}, and collided onusers_pkey→ unhandled 409 → permanent sign-in 500 loop.usersanduser_profiles— itsuser_idis also a PK and 409s for a stub that reached onboarding) become upserts.on_conflictis the primary key, notgoogle_id: the stub'sgoogle_idis NULL and NULLs never conflict, so only an id-conflict resolves it;merge-duplicatesfills the NULL auth columns in.user_{google_id}keeps its id./google/callbackwith the mocked tables raising the exact production 409 on insert — a regression to a blind insert fails loudly.#343 — enable CodeRabbit on long-lived integration branches (CI, P2)
CodeRabbit reviews only the default branch unless base branches are listed, and the repo had no
.coderabbit.yamlat all — so PRs targeting an integration branch got "Review skipped" while CI stayed green..coderabbit.yamllisting^production$and^staging$(anchored regex, validated againstschema.v2.json).production:main → productionpromotion PRs (Promote staging → production: design-system, agents, calendar rewire, storage lockdown #305, chore(deploy): trigger prod frontend rebuild (bake NEXT_PUBLIC_API_URL into client) #314) are the last gate before prod and were never machine-reviewed.stagingis listed defensively (that branch was deleted 2026-07-15; no open PR currently targets a non-main base).Verification
test_storage_service×2,test_ocr_pipelineevent-loop error).ruff checkclean on all touched files.tsc --noEmitclean,eslintclean on touched files, 88 vitest tests pass.test_auth_stub_promotion.py(7), rewrittentest_social_students.py(scope/visibility/payload +school_peer_user_idstraversal),test_profile_routes.pyvisibility-validation.Not included
?resume=/?session=Learn links) is intentionally deferred: it lives inLearn.tsx, which draft PR feat(learn): stream tutor replies over SSE with live graph deltas (#70, #74) #349 rewrites (+408/−46), and feat(learn): stream tutor replies over SSE with live graph deltas (#70, #74) #349 does not fix it. Doing it now guarantees a conflict and throwaway work; it should be a feat(learn): stream tutor replies over SSE with live graph deltas (#70, #74) #349 follow-up. Details on the issue.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes