Uh oh!
There was an error while loading. Please reload this page.
chore(hygiene): seam + determinism sweep (#413 #248 #306 #426 #425) - #467
Merged
Conversation
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Warning Review limit reached
Next review available in:17 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 (25)
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 | 6d3bc4f | Commit Preview URL Branch Preview URL | Jul 30 2026, 07:34 AM |
AndresL230
commented
Jul 30, 2026
CollaboratorAuthor
Review pass complete: two agents clean, four sub-80 findings all fixed — e2e-up's resolver now accepts |
AndresL230 added a commit
that referenced
this pull request
Jul 30, 2026
- e2e-up SESSION_SECRET resolver accepts dotenv's optional 'export ' prefix and strips trailing CR from CRLF-edited .env files (both were false preflight failures); harness case added. - backfill_document_chunks now routes through the public rag_service.embed_document_text wrapper instead of the private _embed_document, making the wrapper's exclusivity docstring true. - Stale test docstring updated (the relevance-gate site no longer constructs a raw client since this PR's own #413 change). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Five hygiene issues, one coordinated PR (the utcnow sweep must land whole — the ISO wire format changes from naive to tz-aware): - #413: the documents.py RAG-indexing fallback no longer constructs a raw genai.Client — it routes through rag_service's sanctioned #439 seam (new embed_document_text wrapper: lazy shared client, dummy-key posture, real-mode gate), closing the keyless real-mode hole where a ValueError at construction silently degraded to no-index. scripts/ingest_catalog.py stays a documented exemption (offline ops CLI, meaningless without real embeddings) but gets a lazy client + actionable keyless exit. Grep acceptance: zero raw empty-string-key client constructions remain. - #248: all 16 datetime.utcnow() sites across 7 backend files → datetime.now(timezone.utc). Consumer audit: all swept columns are TIMESTAMPTZ; the frontend parses via new Date() (aware ISO is more correct); no string-prefix consumers affected. The sweep surfaced and fixed a REAL latent bug: end_session's naive-minus-aware arithmetic raised TypeError on PostgREST reads and silently reported time_spent_minutes=0 (_elapsed_minutes now normalizes legacy-naive); _compute_velocity gets the same normalization. Pytest deprecation warnings drop 80 → 5. - #306: the dead trailing JSON-schema block in quiz_context_update.txt removed (output_type owns the shape); instructional text and every placeholder the route fills kept, guarded by new template tests. - #426: test-mode clock leaks — Gradebook Landing's term preselect and the notetaker's edit stamps (plus two more found sweeping: AssignmentList's 'Synced Xm ago', Calendar's export filename) now route through the frozen now(); deliberate skips documented in the tests/PR (presence liveness, session expiry, cooldowns, uniqueness keys must stay real). - #425: e2e-up preflight now verifies the SESSION_SECRET the backend will resolve (env-beats-.env, mirroring load_dotenv) MATCHES the one the frontend's start:test literal pins — mismatch or empty fails fast with sources named and sha256 fingerprints only, never values. Plus an informational note when SAPLING_MODEL_MODE is unset (the live-billing footgun). docs/local-supabase.md updated. Suites: backend 1400 passed + ruff clean (lock-venv seam checks green); frontend 286 passed + tsc + lint clean; script harness 11/11. Closes#413. Closes#248. Closes#306. Closes#426. Closes#425. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- e2e-up SESSION_SECRET resolver accepts dotenv's optional 'export ' prefix and strips trailing CR from CRLF-edited .env files (both were false preflight failures); harness case added. - backfill_document_chunks now routes through the public rag_service.embed_document_text wrapper instead of the private _embed_document, making the wrapper's exclusivity docstring true. - Stale test docstring updated (the relevance-gate site no longer constructs a raw client since this PR's own #413 change). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
AndresL230force-pushed
the
fix/b5-seam-determinism
branch
from
July 30, 2026 07:31
443ddae to
6d3bc4fCompareAndresL230
commented
Jul 30, 2026
CollaboratorAuthor
Pre-merge gate at the rebased head (over B8's #466): full lane 26/26 passed + oracles clean — and the boot itself exercised the new SESSION_SECRET preflight. Merging. |
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
…ing contract (ToastProvider stub, getGpa mock, summary gpa/semester fields) 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>
11 tasks
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 B5 — five hygiene issues in one coordinated PR (the #248 sweep changes the timestamp wire format, so it can't land piecemeal). Per-issue detail in the commit message; highlights:
rag_service.embed_document_textwrapper (the raw-client keyless hole silently degraded to no-index);ingest_catalog.pydocumented as an offline exemption with a lazy client + actionable keyless exit.datetime.utcnow()sites →datetime.now(timezone.utc), with a consumer audit (TIMESTAMPTZ columns,new Date()parsing). The sweep caught a real bug:end_sessionreportedtime_spent_minutes=0whenever PostgREST returned aware timestamps (naive-minus-aware TypeError, swallowed). Deprecation warnings 80 → 5.output_typeowns the shape; template guard tests added.e2e-uppreflight fails fast on backend/frontend SESSION_SECRET mismatch (sources named, fingerprints only — the frontend'sstart:testliteral beats env, which is exactly how the silent break happens) + an unset-model-mode heads-up.Verification
Backend 1400 passed + ruff clean (seam tests re-run green on the lock-pinned pydantic-ai 1.107 venv); frontend 286 passed, tsc + lint clean; the preflight check driven by an 11/11 scratch harness (match/mismatch/empty/env-precedence/quoting cases). Red-first tests throughout. Full local e2e cycle pre-merge (it also exercises the new preflight for real); results below.
Closes#413. Closes#248. Closes#306. Closes#426. Closes#425.
🤖 Generated with Claude Code