Uh oh!
There was an error while loading. Please reload this page.
test(backend): integration fixtures — psycopg raw-SQL seam, truncate isolation, seeded users (#397) - #417
Conversation
…isolation, seeded users (#397) The integration lane existed but only round-tripped through PostgREST both ways (testing the echo, not the DB) or asserted on the app's own JSON. This adds the raw-SQL seam the lane was missing and the fixtures #398 builds on: - db_conn: session-scoped psycopg connection on SUPABASE_DB_URL (dict rows, autocommit) — the raw-SQL assertion seam. Writes go through the app; reads come back through this, never through table(). - _require_local_db_url: the non-negotiable safety gate. SUPABASE_DB_URL is independent of the SUPABASE_URL that _require_local_stack checks, and .env.staging/.env.production hold live direct-Postgres strings, so the truncate could wipe a real project. The gate parses the host (strict, so 127.0.0.1.evil.com is rejected) and RAISES — never skips — on non-local. - _reset_between_tests: autouse truncate of every mutable table + reseed of the rich baseline before each test, making the suite order-independent. The denylist preserves the migration-seeded reference layer + catalog hierarchy (verified to carry no FK to users, so no CASCADE can reach it). - seeded_user factory (distinct approved users) and authed_client / other_user_client, replacing the per-test cookies.set boilerplate. test_local_stack.py is refactored onto the fixtures: the flagship test POSTs a note through the app and asserts the ciphertext at rest via raw SQL; a truncate -isolation pair proves ordering-independence; a distinct-users test and a seeded_user test cover the new fixtures. The safety gate is proven in the DEFAULT hermetic lane (tests/test_integration_ db_guard.py, pure URL logic, no DB) so it gates every PR: +13 tests there (976 → 989), the 9 DB-backed tests skip without RUN_INTEGRATION. No regressions. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018b3MtMEu2htdGVBGRcrmzX
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Free Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe integration test lane now enforces local database targets, resets mutable tables before each test, provides seeded authenticated clients, and validates encryption, authentication, raw SQL persistence, user isolation, and fixture-backed test isolation. ChangesLocal integration test infrastructure
Estimated code review effort: 4 (Complex) | ~45 minutes 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 | d2a32f0 | Commit Preview URL Branch Preview URL | Jul 23 2026, 06:00 PM |
AndresL230
commented
Jul 23, 2026
Merge rationale + honest verification state. Both handoff gates are met: What was NOT executed: the 9 Merging on the strength of the defined gates + the proven safety guard, since these tests are opt-in, test-only, and cannot affect production or the default CI lane. Ask: a maintainer should run the Generated by Claude Code |
Uh oh!
There was an error while loading. Please reload this page.
… (fixes staging session_expired) (#409) * feat(errors): extract FastAPI detail from thrown API errors (#361) `fetchJSON` rejects with `new Error(await res.text())`, so a FastAPI failure surfaces as an Error whose message is the raw JSON body. Add a dependency-free helper that reads the `detail` back out of it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(errors): cover FastAPI detail extraction (#361) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(errors): recover the HTTP status off a thrown error (#361) `fetchJSON` only spells the status out (`HTTP 404`) when the response body is empty, so read it from an attached `status`/`statusCode`, the parsed body, or the `HTTP <code>` message as available. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(errors): cover HTTP status recovery (#361) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(errors): map HTTP statuses to friendly copy (#361) Add humanizeError: status-driven sentences for the cases users can act on (auth, missing, rate limit, 5xx), falling back to caller-supplied copy so it can never surface a raw body. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(api): declare the term label on EnrolledCourse (#140) /api/graph/{user_id}/courses has always returned the offering's term label; the client type never declared it, so every consumer had to cast through any to reach it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(errors): cover the status-to-copy mapping (#361) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(api): add getSemesters() for GET /api/semesters (#140) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(ui): add responsive layout primitives to globals.css (#109) Inline styles can't carry a media query, so the app's fixed multi-column shells (Admin's master/detail panes and metric row, Settings' profile field rows) get class hooks here instead. Driving them from CSS rather than `useIsMobile` also makes the first paint correct, since the hook can only flip after hydration. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(errors): prefer a human-readable server detail (#361) A FastAPI detail like "Exam not found." is better copy than generic status text, so surface it — but only when it reads like a sentence, so a serialized payload, markup or a stack can never reach the UI. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(ui): stack the Admin roles pane on mobile (#109) The role editor rail was pinned at `minmax(280px, 360px) 1fr` with no mobile branch, so the pane overflowed the viewport below ~640px. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(semesters): scaffold the shared term helper module (#140) termRankFromLabel mirrors the sort_key formula from migration 0019 so a label-only fallback orders identically to the server. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(ui): stack the Admin achievements pane on mobile (#109) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(errors): assert no raw body, markup or stack ever reaches the UI (#361) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(ui): stack the Admin cosmetics pane on mobile (#109) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(semesters): resolve the current term by date (#140) Mirrors services/academics.py::current_term — today within [start_date, end_date], else the highest sort_key — so client and server never disagree about which semester is current. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(ui): reflow the Admin overview metric row on mobile (#109) Four fixed metric cards squeezed to ~75px each at 375px. Drops to a 2x2 grid below 900px. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(errors): add an isNotFound predicate (#361) Lets callers branch on "that thing is gone" without string-matching a response body at the call site. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(ui): stack the Settings profile rows on mobile (#109) The username row and the display-name/bio/location/website rows were both hard-coded to `180px 1fr`, leaving ~150px for the input at 375px. They now share the `.settings-field-row` class and collapse to a label-above-control stack below 600px. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(semesters): cover current-term date resolution and the gap fallback (#140) Fixtures are the four terms seeded by migration 0019 verbatim, so a drift between this rule and the backend's shows up here. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(errors): cover isNotFound detection (#361) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(study): humanize the exam-load failure toast (#361) `String(err)` rendered the stringified FastAPI body straight into the toast. Keep the real error on the console and show a sentence instead. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(ui): let Dialog consumers pick the initially focused element (#109) Dialog focuses the first focusable node in the panel, which is always the close button. Form dialogs need their first field instead, and `autoFocus` loses that race — React fires it at mount, before Dialog's focus pass. Opt-in and additive; existing consumers are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(semesters): group courses by term label, most recent first (#140) Ordering keys on sort_key when the semesters payload is available and degrades to the label-derived rank otherwise. Courses with no term go to an 'Other' bucket rather than being dropped. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(study): humanize the guide-load failure toast (#361) Also clear the stale guide so a failed load can't leave the previous exam's content on screen. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(semesters): cover term grouping, ordering and the unknown bucket (#140) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(study): land on inline guidance when the exam is gone (#361) A missing exam is a normal state — a deleted assignment, or a stale "recent guides" entry — not a failure. Show the user where to go next instead of firing a red toast at them. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(ui): move LetterScaleEditor onto the shared Dialog (#109) Drops the hand-rolled portal and its `minWidth: 360` — which overflowed a 360px viewport once the overlay's gutters were counted — for Dialog's `min(420px, 100vw - 32px)` panel. Also picks up the focus trap, Escape handling and scroll lock the hand-rolled version never had. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(semesters): partition courses into current and archive (#140) Only courses that rank strictly below the current term are archived. Undatable courses — and every course when /api/semesters gives us nothing — stay in the default list. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(semesters): cover partition ordering and the no-semesters fallback (#140) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(study): offer a retry when a guide genuinely fails to build (#361) Generation failures (502) are usually transient, so keep the message on screen next to a retry instead of leaving the user on a blank panel after the toast times out. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(study): keep regenerate unreachable without a selected exam (#361) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(semesters): derive ordered term labels for the gradebook chips (#140) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(study): humanize the regenerate failure toast (#361) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(ui): move EditWeightsModal onto the shared Dialog (#109) `minWidth: 520` made this the worst overflow of the four gradebook modals; it now sits in Dialog's `min(640px, 100vw - 32px)` panel. The footer wraps rather than crushing the "Total: n%" readout against the buttons on narrow screens. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(study): humanize the flashcard delete and generate toasts (#361) Last two raw-error toasts on this screen. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(study): sharpen the no-exam empty-state copy (#361) Say why an exam is needed, not just that none exist — that's the whole question a user lands on this state with. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(gradebook): read term (not semester) off the courses payload (#140) /api/graph/{user_id}/courses emits `term`; the landing read `(c as any).semester`, which is always undefined. `distinct` was therefore always empty and every signed-in user silently fell through to the hardcoded SAMPLE_SEMESTERS demo chips. The sample chips are now the logged-out preview only — a signed-in user with no terms gets their own empty state instead of another student's fake grades. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(ui): move SyllabusUploadFlow onto the shared Dialog (#109) Replaces `minWidth: 460` with Dialog's fluid panel, and lets the category/assignment rows shrink (`minWidth: 0` on the flex text inputs, wrapping on the assignment rows) so the date picker can't push them past the panel edge. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(study-guide): make the exam-not-found detail actionable (#361) The frontend now renders a FastAPI detail verbatim when it reads like a sentence, so tell the user what to do next instead of just naming the condition. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(study-guide): pin the 404 detail as user-facing copy (#361) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(gradebook): pin the landing chips to the courses payload term (#140) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore(gradebook): type the CourseCard test stub instead of any (#140) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(study): let the guide problem outrank the generic empty hints (#361) Opening a recent guide clears the exam selection, so a missing exam would otherwise stack "No exams for this course yet" on top of the guidance explaining what actually happened. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(ui): move AssignmentModal onto the shared Dialog (#109) `minWidth: 420` overflowed any phone viewport, and the panel had no max-height at all — with the bell-curve section expanded the footer ran off-screen with nothing to scroll. Dialog fixes both and adds the focus trap, Escape handling and scroll lock. `autoFocus` is swapped for Dialog's `initialFocusRef` so the Title field still takes focus on open rather than the close button. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(study): retry the guide that actually failed (#361) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(a11y): 44px touch targets for SideNav rows (#110) `8px` vertical padding around a 15px icon left the nav links ~31px tall. Collapsed, the rail is 64px wide minus 6px padding, so the `width: 100%` link already clears 44px horizontally — only the height needed a floor. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(a11y): 44px collapse/expand controls in SideNav (#110) The collapse chevron was a 24x24 target and the expand bar 28px tall. Both now match Dialog's 44x44 close button. `flexShrink: 0` keeps the collapse button square when the account name is long — the name block beside it already ellipsizes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(study): cover the missing-exam guidance and retry paths (#361) Drives the screen through the recent-guides rail — the real path to a stale exam id — and asserts a missing exam produces guidance with no toast, while a genuine failure toasts a sentence and keeps a retry. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(ui): make useIsMobile hydration-safe via useSyncExternalStore (#110) `useState(false)` + a `matchMedia` effect meant the value was stale for one paint after every mount, and each consumer registered its own listener. `useSyncExternalStore` pins the SSR/hydration snapshot to `false` (so server and first client render still agree, as React 19 requires) while sharing one `MediaQueryList` per breakpoint and updating as early as React allows. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(gradebook): order the semester chips by the real term calendar (#140) Chips now sort by sort_key from /api/semesters and default to the date-derived current term instead of whichever term the courses payload happened to list first. A failed semesters fetch degrades to the label-derived order. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(gradebook): open the term named by ?semester= (#140) Gives the dashboard archive somewhere to land: selecting an archived class opens that semester's gradebook rather than the current one. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(dashboard): load the term calendar alongside the graph payload (#140) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * polish(study): stop the failure card restating its own title (#361) When no server detail survives, the body falls back to "Couldn't build that study guide" — which was the title too. Give the card a heading that pairs with any reason. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(ui): cover the useIsMobile SSR/hydration contract (#110) Seven cases: the server render reports desktop on a mobile viewport, hydration produces no recoverable error either way, the value flips after commit and tracks later changes, and the queried width matches the `max-width: 767px` rules globals.css relies on. Verified against a naive `useState(matchMedia(...).matches)` implementation — it fails three of them. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(dashboard): partition course progress into current and archive (#140) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(ui): hide the desktop rail pre-hydration on mobile (#110) The SSR shell always assumes desktop, so a phone painted a 232px SideNav rail until hydration swapped in TopNav. A width-based `@media` rule applies to that first frame, which no amount of hook work can reach. Pairs with the useIsMobile breakpoint, asserted in its test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * perf(images): lazy-load and size the remote avatar images (#111) `Avatar` and `AvatarFrame` render user-supplied URLs with no intrinsic dimensions, so every one of them reserved zero space until it decoded. Explicit width/height give the browser the aspect ratio up front; the CSS `100%` sizing still wins for layout. The two `/sapling-icon.svg` logos in TopNav/SideNav are deliberately left eager — they're local, above-the-fold brand marks already sized by inline styles. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(dashboard): extract CourseProgressRow from the courses panel (#140) Same markup, lifted so the current-term list, the archive and the graph overlay can all render a course line without a third copy. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(dashboard): group the my-courses panel by semester with an archive (#140) Current-term courses show by default; earlier terms collapse behind an Archive toggle, grouped by label most recent first. Also covers the mobile 'My Courses' tab, which renders the same panel. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * perf(landing): gate the hero canvas RAF behind prefers-reduced-motion (#111) The hero projects and sorts 226 nodes and runs an O(n^2) edge pass every frame, forever. globals.css only neutralizes CSS animation, so a reduced-motion visitor was still paying for all of it. Now it paints one static frame and parks, repainting on resize (which clears the backing store) and re-arming if the preference flips mid-session. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(dashboard): scope the graph courses key to the current term (#140) The floating course key now lists only current-term courses and offers past terms as a compact Archive that deep-links into each semester's gradebook. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * perf(landing): hoist the floating-card DOM and dataset reads out of the RAF (#111) The tick re-ran `querySelectorAll('.floating-card')` and re-parsed three `dataset` floats per card on every frame. Both are static, so they move to effect setup. The loop also parks under prefers-reduced-motion, keeping each card's resting tilt but dropping the drift, mouse tilt and scroll parallax. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * perf(landing): cache spotlight card rects instead of measuring per mousemove (#111) `getBoundingClientRect()` on every pointer sample forces a layout flush. The rect is now taken on `mouseenter` and dropped on scroll/resize — the only things that can move a card relative to the viewport — so a sweep across a card costs one measurement, not one per sample. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(api): carry the HTTP status on failed requests (#361) fetchJSON discarded the status, so a FastAPI failure — which always has a JSON body — reached callers with no status at all. isNotFound had to infer "missing" from the words "not found", which would silently regress into a red toast the day someone reworded a server message. ApiError keeps `message` as the raw body, so existing callers that stringify or read `.message` are unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(courses): group the manage-courses list by semester (#140) Headings only appear once a student has courses in more than one term. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * perf(landing): rAF-throttle the landing scroll handler (#111) `onScroll` wrote inline styles on the hero, the nav and the ambient glow on every scroll event, which fire well above frame rate. Coalesced to one write per frame; the mousemove and scroll listeners are also marked passive since neither calls preventDefault. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(dashboard): scope the graph legend chips to the current term (#140) Keeps the top-nav legend consistent with the courses key overlay, which already lists only the current semester. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(dashboard): cover semester grouping, archive routing and degradation (#140) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(dashboard): wire the archive toggle to its region for assistive tech (#140) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(gradebook): smoke-cover the four modals moved onto Dialog (#109) The migration is invisible to tsc — a modal that stops opening, loses its Cancel handler, or drops its accessible name still typechecks. These four had no tests at all, so the swap was landing unverified. Also pins initial focus landing on the title field rather than Dialog's close button, which is the specific reason initialFocusRef exists. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore(lint): prune the suppression the Landing fix made stale (#140) Reading `term` instead of `(c as any).semester` removed the only no-explicit-any in Landing.tsx, so its suppression entry no longer matches anything. eslint exits 2 on a stale suppression even with zero errors, which fails the CI lint gate — `main` exits 0, this branch did not. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ci: re-trigger frontend-staging Workers build Empty commit to re-run CI after the frontend-staging Workers build-command config fix (npm run cf:build restored; the broken wrangler deploy --env staging skipped the OpenNext build). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * ci: re-trigger frontend-staging Workers build Empty commit to re-run CI after the frontend-staging Workers build-command config fix (npm run cf:build restored; the broken wrangler deploy --env staging skipped the OpenNext build). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * ci: re-trigger frontend-staging Workers build Empty commit to re-run CI after the frontend-staging Workers build-command config fix (npm run cf:build restored; the broken wrangler deploy --env staging skipped the OpenNext build). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(frontend): make DEPLOY_ENV the single source of truth for env config Staging login bounced to /?error=session_expired: the worker serving staging.saplinglearn.com ran with production config (BACKEND_URL= api.saplinglearn.com), so sign-in round-tripped through the prod backend and came back as a prod-signed .saplinglearn.com cookie that staging's middleware rejected under its own SESSION_SECRET. The deployGuard check that would catch a consistent-but-wrong-target build only arms when DEPLOY_ENV is set, and it wasn't set on either Workers Build. - deployGuard: add resolveFrontendEnv (derive apiUrl/cookieDomain from FRONTEND_ENVS when DEPLOY_ENV is set; fall back to explicit vars otherwise) plus expectedEnvForHost/detectHostConfigMismatch. Unit-tested. - middleware: derive API_URL via the resolver; on a protected route, flag a host/backend mismatch with a loud log + distinct `env_misconfig` code instead of the misleading `session_expired`. - session route: derive cookie Domain from the resolver. - next.config: derive build-time BACKEND_URL/NEXT_PUBLIC_API_URL/COOKIE_DOMAIN from DEPLOY_ENV. - wrangler.toml: set DEPLOY_ENV for [vars] and [env.staging.vars]. - SignInModal: user copy for env_misconfig. - docs: ADR 0020 (root cause + required deploy follow-up). Note: this hardens the repo but does not fix the running deployment — that needs a staging redeploy with DEPLOY_ENV=staging + `wrangler deploy --env staging` and the correct route binding (see ADR 0020). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(deploy): stop the build-command footgun that took staging down ADR 0020's operational follow-up told operators to set a `wrangler deploy --env staging` line and a DEPLOY_ENV build variable, but never said to keep the Build command as `npm run cf:build`. Wiring that up, the frontend-staging Workers Build's *build-command* field got overwritten with `npx wrangler deploy --env staging` — a deploy command in the build slot. That skips `opennextjs-cloudflare build`, so `.open-next/` is never produced and every build failed with "Could not find compiled Open Next config" (~16 red builds across all branches since 2026-07-20). Verified locally: `npm run cf:build` produces `.open-next/worker.js` (the `main` wrangler deploys); `npx wrangler deploy --env staging` alone does not. - ADR 0020: split the two Workers Builds fields explicitly, mandate the Build command stay `npm run cf:build`, and forbid putting a deploy command in it. - wrangler.toml: document the same Build vs Deploy field distinction at the point of configuration. The live fix is still a one-field dashboard revert (Build command back to `npm run cf:build`); this stops the docs from steering anyone into it again. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(frontend): data-testid convention on six core E2E surfaces (#382) (#410) The browser suite (#385) needs stable selectors. Today shipped code has zero data-testid attributes, so Playwright would have to anchor on CSS classes (utility-ish, non-unique) or copy — both churn on every design pass. Adds a kebab-case `<surface>-<element>` convention, applies it to the six surfaces Chapter-1 drives (sign-in, approval gate, upload modal, tutor composer, quiz answer flow, graph container), and gates drift with a per-file ESLint rule. - docs/frontend-testids.md documents the naming rules, how repeated/list items are disambiguated (stable domain id first, render index as the fallback), the full current inventory, and how to onboard a new surface. - Testids land on the file that actually renders the element, which is not always the screen file: the tutor composer lives in ChatPanel.tsx (single consumer: screens/Learn.tsx) and every quiz control lives in QuizPanel.tsx (screens/Quiz.tsx only mounts it). - eslint.config.mjs gets a `no-restricted-syntax` block scoped to those six files: any <button>/<input>/<textarea> there without a data-testid is an error. Deliberately not repo-wide — the rest of the app has no browser coverage to protect. Attributes and lint config only; no behavior, styling, or logic changes. The SignInModal.tsx edit is strictly additive (open PRs #409/#359 touch that file). Closes #382 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * fix(backend): keyless rag_service import + hermetic LLM egress guard (#411) #378 — services/rag_service.py built a module-level genai.Client with api_key=os.getenv("GEMINI_API_KEY", ""), and genai.Client(api_key="") raises ValueError at construction. That broke `import main` outright without a key (routes/quiz.py and routes/learn.py both pull the module in). Fall back to "dummy-key-for-import" the way services/gemini_service.py and agents/_providers.py already do: imports stay clean and the failure moves to call time, where it is actionable. No behaviour change when a real key is present. #379 — add the autouse `_hermetic_llm_transport` fixture to tests/conftest.py, the LLM sibling of `_hermetic_supabase_client`. It patches the google-genai transport CLASS (google.genai._api_client.BaseApiClient) rather than client instances, so every already-constructed module-level client is covered: gemini_service, rag_service, and pydantic-ai's GoogleProvider. Unstubbed calls now raise UnstubbedLLMEgress("unstubbed LLM egress: ...") instead of making a real, billable request. Unary, streaming, sync, async and the File API side channels are all blocked, and the fixture fails loudly if google-genai ever moves the seam rather than silently degrading to a no-op. Exemptions mirror the existing guards (e2e_staging, integration) plus a new `live_llm` marker for the three deliberately-live tests in test_ocr_pipeline.py. Their existing `_requires_gemini` skipif is invisible to `get_closest_marker`, so a real marker was required; the skipif still keeps them from running without a key. Verified: full suite 987 passed / 5 skipped / 1 pre-existing error (test_ocr_pipeline::test_save_to_db, unchanged from main); CI-equivalent lane 929 passed / 5 skipped; ruff clean; keyless `import main` succeeds. Closes #378 Closes #379 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * test(backend): cookie-minting test-auth endpoint for local/test envs (#381) (#412) * test(backend): cookie-minting test-auth endpoint for local/test envs (#381) `GET /api/auth/dev-login` was removed and real Google OAuth is not headless-automatable, so pytest and Playwright had no sanctioned way to obtain an authenticated session. Unify the duplicated minter: - New `backend/services/session_tokens.py` owns the one implementation of the `<payload_b64>.<sig_b64>` format `auth_guard._decode_session` verifies, plus the canonical `SESSION_COOKIE_NAME`. - `db/e2e_staging_http.py` and `tests/integration/conftest.py` now use it instead of carrying verbatim copies; the OAuth-callback redirect handoff token in `routes/auth.py` uses it too (byte-identical output, TTL passed explicitly). `auth_guard` reads the cookie name from it. - `tests/test_auth_session_contract.py::_mint` stays an independent re-implementation on purpose: it pins the wire format from the outside. Add `POST /api/auth/test-login`: - Sets the `sapling_session` cookie with the same attributes as the real session AND returns the token in the body, so Playwright global setup can inject it via `context.addCookies()`. - Hard-gated on `APP_ENV in {"local", "test"}` — narrower than `config.IS_LOCAL`, which also covers `development`/`dev`. - The gate is evaluated per request off the live `config` module attribute and returns a stock 404 `{"detail": "Not Found"}` everywhere else, for every request shape (the body is parsed by hand so FastAPI's pre-handler 422 cannot disclose the route). `include_in_schema=False` keeps it out of /openapi.json in all environments. - No DB access: it does not create users or grant approval/roles. 47 new tests cover the production 404, the request-time gate, the real auth_guard round-trip, and byte-identical minting. Closes #381 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(auth): assert test-login mounting via router.routes, not app.routes `test_route_exists_but_is_gated` walked `client.app.routes` looking for `/api/auth/test-login`. How an included APIRouter flattens into the composed app's route list is not a stable API: under the pinned fastapi 0.138 / starlette 1.3 (CI) the sub-router contributes no `.path` entries there, so the set comprehension silently found nothing and the assertion failed — while every behavioural test against the same endpoint passed, because the route itself was mounted and serving correctly. Assert against `auth_module.router.routes` instead, which is a flat list of APIRoute objects with stable `.path` values across both versions. This keeps the test's original purpose: proving the 404 comes from the environment gate rather than from a route that was never mounted. Caught by CI; the local venv runs fastapi 0.136 / starlette 1.0, where the old introspection happened to work. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * docs(e2e): wave-2 handoff for epic #402 subcutaneous lane (#414) Session prompt for the next wave (#391, #397, #398), committed so a cloud session can pick it up from the repo rather than needing it pasted in. Records what wave 1 established and what it cost to learn: the baseline test counts and the one pre-existing OCR error not to chase, the shadowed grep/find, the missing venv/.env in fresh worktrees, why `env -u GEMINI_API_KEY pytest` can never work, and the local-vs-requirements.lock version skew that made a locally-green test fail CI. Also states the engineering constraints this lane turns on -- assert through a different layer than the one that wrote, make a test fail before trusting it, never weaken a hermetic guard to get green, and treat #398's findings as the deliverable rather than a blocker. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * docs(e2e): add skills + autonomy guidance to the wave-2 handoff (#415) * docs(e2e): add skills + autonomy guidance to the wave-2 handoff The handoff covered environment traps and engineering constraints but said nothing about which skills to reach for or how independently to run, so a session picking it up would default to neither. Splits the tooling by what actually resolves where: /sync-context, the context-curator agent, /recall, /log-decision and /log-attempt are committed under .claude/ and work anywhere, while the superpowers and code-review skills are local plugins that may not exist in a cloud session -- those are listed conditionally with a manual fallback for the review fan-out. Calls out that CLAUDE.md already requires /sync-context before agent-building work, which #391 is, and that context-curator is meant to run before touching LLM integration. Adds an autonomy section: execute the wave without asking permission for reversible work, own CI failures rather than reporting a red PR as done, and never end a turn on a plan instead of doing it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(e2e): make code review gate the merge, not trail it The handoff put /code-review at the end of the wave, after every PR had already merged. That ordering cannot prevent a bad change from landing -- it can only document one after the fact. Wave 1 was run this way and got lucky: the review found nothing above threshold, but anything it had found would already have been on main. Makes review a per-PR merge gate alongside CI, with every finding addressed or explicitly dismissed with a reason. Keeps a wave-end pass, but reframes it as covering interactions between merged PRs rather than as the only review. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(e2e): fix false test claim + add destructive-truncate guardrail Review of PR #415 surfaced two real defects in the handoff: - Claimed all four tests in test_local_stack.py assert via table(); only two do. The other two assert on the app's HTTP response. Corrected so an agent doing find-and-replace isn't misled about the current shape. - #397's autouse truncate runs on a direct psycopg connection over SUPABASE_DB_URL, but the only local guard checks SUPABASE_URL, a separate var. .env.staging and .env.production both hold live direct-Postgres strings. Added a non-negotiable requirement to assert SUPABASE_DB_URL is local and fail loudly before any truncate, so an unsupervised run can't silently wipe real data. Same guardrail added to issue #397 and its acceptance criteria. Also flags the psycopg-in-tests pattern as a deliberate test-only exception to the table()-only rule, so a literal reader doesn't stall on the conflict or treat it as licence for psycopg in app code. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * feat(agents): SAPLING_MODEL_MODE FunctionModel test seam (#391) (#416) * feat(agents): SAPLING_MODEL_MODE FunctionModel test seam (#391) model_for() now dispatches on SAPLING_MODEL_MODE (default 'real', so production and the hermetic unit lane are unchanged): - real → GoogleModel, still honoring the per-task SAPLING_MODEL_<TASK> override from ADR 0008. - function → pydantic-ai FunctionModel bound to a per-task handler tests register via register_function_handler(). Scripted tool calls run through the real tool registration, arg-schema validation, and retry loop. - cassette → reserved (issue scope) but raises NotImplementedError. - anything else → ValueError (a typo'd mode never silently bills Gemini). The FunctionModel substitutes ABOVE the #379 transport guard: it never builds a google.genai request, so a function-mode run needs no hermetic exemption and runs clean in the default lane. Tests pin that invariant (rides-above-guard + the real-mode counter-check that still trips it). AC: an integration-style test drives note_chat_agent with a FunctionModel and asserts on the LLM-chosen search_course_materials_tool arguments after schema validation; a classifier test proves the retry loop runs for real. +13 tests, no regressions (976 → 989 passed in the CI-ignore lane). ADR 0019 records the decision. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018b3MtMEu2htdGVBGRcrmzX * refactor(agents): review polish on the model-mode seam (#391) Self-review follow-ups, no behavior change: - annotate model_for/_function_model_for as -> Model (the pydantic-ai base) instead of GoogleModel + type: ignore — function mode genuinely returns a FunctionModel, so the honest supertype removes the type lie. - drop the unused unregister_function_handler and ModelMode alias to keep the seam's public surface to just register/clear. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018b3MtMEu2htdGVBGRcrmzX --------- Co-authored-by: Claude <noreply@anthropic.com> * test(backend): integration fixtures — psycopg raw-SQL seam, truncate isolation, seeded users (#397) (#417) The integration lane existed but only round-tripped through PostgREST both ways (testing the echo, not the DB) or asserted on the app's own JSON. This adds the raw-SQL seam the lane was missing and the fixtures #398 builds on: - db_conn: session-scoped psycopg connection on SUPABASE_DB_URL (dict rows, autocommit) — the raw-SQL assertion seam. Writes go through the app; reads come back through this, never through table(). - _require_local_db_url: the non-negotiable safety gate. SUPABASE_DB_URL is independent of the SUPABASE_URL that _require_local_stack checks, and .env.staging/.env.production hold live direct-Postgres strings, so the truncate could wipe a real project. The gate parses the host (strict, so 127.0.0.1.evil.com is rejected) and RAISES — never skips — on non-local. - _reset_between_tests: autouse truncate of every mutable table + reseed of the rich baseline before each test, making the suite order-independent. The denylist preserves the migration-seeded reference layer + catalog hierarchy (verified to carry no FK to users, so no CASCADE can reach it). - seeded_user factory (distinct approved users) and authed_client / other_user_client, replacing the per-test cookies.set boilerplate. test_local_stack.py is refactored onto the fixtures: the flagship test POSTs a note through the app and asserts the ciphertext at rest via raw SQL; a truncate -isolation pair proves ordering-independence; a distinct-users test and a seeded_user test cover the new fixtures. The safety gate is proven in the DEFAULT hermetic lane (tests/test_integration_ db_guard.py, pure URL logic, no DB) so it gates every PR: +13 tests there (976 → 989), the 9 DB-backed tests skip without RUN_INTEGRATION. No regressions. Claude-Session: https://claude.ai/code/session_018b3MtMEu2htdGVBGRcrmzX Co-authored-by: Claude <noreply@anthropic.com> * test(backend): migration order pins, encryption round-trip suite, e2e→subcutaneous rename (#398) (#418) Partial delivery of the subcutaneous write-path suite — the pieces provable or low-risk without a running stack: - test_migrations.py (default lane, VERIFIED): pins the runner's apply order. The 0021 pair is load-bearing — 0021_gradebook.sql CREATEs `assignments` and 0021_gradebook_curve.sql ALTERs it to add curve_* columns, so gradebook MUST apply first. sorted(glob()) does exactly that ('.' 0x2E < '_' 0x5F). This also corrects the issue comment, which claimed the sort yields "gradebook_curve before gradebook" — it does not; the pin guards against a rename flipping it. - tests/integration/test_encryption_roundtrip.py: reads every encrypted column from the seeded baseline via the #397 raw-SQL seam and asserts ciphertext at rest + decrypt round-trip across text (decrypt_if_present), numeric (decrypt_numeric, assignments.points_*), and JSON (decrypt_json, sessions.summary_json) — the "silent decrypt regression" sentinel. - tests/integration/test_migrations_ledger.py: the DB-backed half of the migration check (schema_migrations records every file on disk). - Renamed test_e2e_staging.py → test_subcutaneous_staging.py (it drives HTTP routes below the UI; not a browser E2E). Marker `e2e_staging` unchanged. Default lane: +5 verified migration tests (1002 → 1007), no regressions. The integration files are marked `integration` and skip without RUN_INTEGRATION. Remaining #398 scope (test_postgrest_semantics, test_constraints, test_authz_real_rows, and the actual run-to-find-bugs) needs the local stack and is tracked as a follow-up — #398 stays open. Claude-Session: https://claude.ai/code/session_018b3MtMEu2htdGVBGRcrmzX Co-authored-by: Claude <noreply@anthropic.com> * feat(ocr): transcribe text-layer-less pages with Gemini vision Scanned and photographed handwritten coursework carries no text layer, so there are no characters to copy out. Docling's OCR is meant to cover this but crashes on such documents -- `Stage preprocess failed for run 1, pages [13]: std::bad_alloc` -- and the error is swallowed, so the page comes back empty. Docling *does* flag those pages in `fallback_pages`, but the only consumer of that signal was gated behind `OCR_ENGINE=auto` + `GOT_OCR_ENABLED`, and the default engine is `docling`. So in practice the signal was computed and discarded, and a 13-page handwritten practice final extracted to "" -- which then reached the classify prompt as an empty `Content:` block and was answered with an invented summary. Add a Gemini-vision backend that transcribes a rendered page image, and wire it to that existing signal. Deliberately NOT gated on `OCR_ENGINE=auto`, since that gate is precisely why the rescue never fired for real uploads. Chosen over the alternatives for handwritten maths specifically: Tesseract is poor at handwriting, and GOT-OCR needs a ~2GB weight download and is impractical CPU-only. Gemini already backs every other AI path here, and returns LaTeX. Verified end to end on the document that triggered this, with OCR_ENGINE at its default: 0 chars -> 4,507 chars, including row reductions, characteristic polynomials, and \boxed answers. Off by default (`GEMINI_VISION_OCR_ENABLED`): it costs one LLM call per flagged page. Pages with a normal text layer are never flagged, so a text PDF costs nothing. Per-page failures keep whatever Docling produced for that page -- a partial document beats none -- while an unavailability error aborts the loop rather than burning a failed call for every page of a long scan. Also: - extract the OCR cache key into `_ocr_cache_key` and include the new flag, so enabling vision cannot serve the empty string cached from before it was on - correct the comment claiming OCR is deterministic. It no longer is, which matters for content-addressed chunk ids (ADR 0019): two students uploading the same scan only dedup to one embedding if they transcribe identically. Persisting OCR output content-addressed rather than merely caching it is the real fix, and is not attempted here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(observability): activate Logfire ops/error/LLM tracing (#119) (#406) * feat(observability): activate Logfire ops/error/LLM tracing (#119) Turn on Logfire safely and document it. The SDK was already configured (logfire.configure + instrument_pydantic_ai + the scrub_value scrubber), but two gaps kept the success criteria unmet: - instrument_fastapi was never called, so no FastAPI request traces would appear even with a token set. Wire it in main.py. - Enabling FastAPI instrumentation introduces a content-egress path the scrubber cannot reach: OTel records parsed endpoint arguments (request body + params) under `fastapi.arguments.values`, which Logfire does not route through scrub_value (a field named e.g. `body` matches no risky pattern). Drop those arguments at the source via a request_attributes_mapper that returns None, keep headers off (capture_headers=False), and keep the extra argument/endpoint spans off (extra_spans=False). No prompts, completions, chat messages, note bodies, quiz answers, or uploaded document text leave the process on request spans. Also: - Add LOGFIRE_TOKEN to .env.example (optional; dormant when unset via send_to_logfire="if-token-present") and surface it through config.py. - Document Logfire in docs/observability-logging-tracking.md: what it captures vs the owned Supabase events/llm_usage tables (independent, no double-count), how to enable, what is scrubbed, and the in-scope query-string caveat. - Tests: AST guards that fail if the argument-dropping mapper / header / span flags regress, plus an end-to-end test asserting a request body never lands in any exported span. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(flashcards): stop rate-limit retry-after overshooting the window check_rate_limit computed `int(_RATE_WINDOW_SEC - elapsed) + 1`, which returns 61 when the limited calls land in the same clock tick (elapsed == 0) — one second past the 60s window, and it tripped test_sixth_call_returns_retry_after (`assert 61 <= 60`). Use math.ceil of the true remaining time instead: it still rounds a sub-second remainder up to 1 (never 0) but is bounded by the window, so retry-after is always in [1, _RATE_WINDOW_SEC]. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(agents): bound note_chat orchestrator + remaining worker agents with usage limits (#345) * fix(agents): bound note_chat + remaining worker agents with usage limits (#329) Residual from #327/#243: three run-sites still executed without usage_limits, defaulting to library maximums. - note_chat now runs under ORCHESTRATOR_LIMITS; guardrail trips (UsageLimitExceeded / UnexpectedModelBehavior) degrade to an in-band reply with degraded=true instead of an uncaught 500 (no legacy fallback exists for this path per ADR 0017). - note_summary / note_concepts run under WORKER_LIMITS via a shared _run_note_worker helper that converts guardrail trips to 503. - syllabus_extraction in calendar_service now passes WORKER_LIMITS; its caller already degrades gracefully. - Tests pin the usage_limits kwarg at all four run-sites and the new degrade/503 behavior. Closes #329 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015ori1maMbbFjkpCS7jPgjj * fix(notes): use noun form in summarize 503 detail (CodeRabbit nit) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015ori1maMbbFjkpCS7jPgjj * fix(notes,calendar): separate budget trips from model bugs in agent guardrails (#329) Review fixes for the usage-limit guardrails so a deterministic budget trip and a genuine model bug are no longer conflated: - notes worker (_run_note_worker): UsageLimitExceeded -> 413 with an honest "note too long, shortening may help" detail (no transient "try again" wording); UnexpectedModelBehavior -> 500 + logger.exception so a real bug pages us with a traceback instead of hiding behind a 503/WARNING. - note_chat: UsageLimitExceeded keeps the in-band degrade (its budget wording is now accurate); UnexpectedModelBehavior -> 500. Success path now returns degraded: false for schema symmetry with the degrade path. - calendar (extract_assignments_from_file): UsageLimitExceeded degrades with an honest "syllabus too long / split it" warning; model hiccups and bare exceptions keep the generic degrade. _degraded_result gains a `warning=` override. - tests: rewrite the guardrail tests to the new contract and dedup the fake-note fixture into one module-level factory. Note: this revises behavior previously asserted by test_503_when_guardrails_trip and the parametrized note_chat degrade test — UnexpectedModelBehavior is intentionally no longer treated as a budget trip. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * ci: re-trigger frontend-staging Workers build Empty commit to re-run CI after the frontend-staging Workers build-command config fix (npm run cf:build restored; the broken wrangler deploy --env staging skipped the OpenNext build). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: AndresL230 <190146319+AndresL230@users.noreply.github.com> * feat(frontend): test environment profile with same-origin API proxy (#380) (#421) Add build:test / start:test npm scripts that produce a production Next build targeting the local stack with ALL API traffic same-origin through the Next /api/:path* rewrite to the local FastAPI on :5000: - NEXT_PUBLIC_API_URL is set explicitly EMPTY so every client fetch is same-origin and the sapling_session cookie always rides along (the landing page falls back to cross-origin http://localhost:5000 when the var is merely unset). - BACKEND_URL=http://localhost:5000 bakes the rewrite destination and satisfies next.config.ts's production-build guard. - Local Supabase URL + demo anon key are inlined so the lazy lib/supabase.ts client initializes instead of throwing. - start:test supplies the runtime side: BACKEND_URL for the middleware session check and the fixed local SESSION_SECRET for the session route. All values are the committed-safe local defaults from .env.local.example, inlined in the scripts (real process env beats .env* files in Next, so the profile is deterministic regardless of a dev's .env.local). Zero new dependencies; middleware.ts and the production `npm run build` are untouched. Recipe documented in docs/local-supabase.md. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * refactor(ocr): route vision transcription through a Pydantic AI agent The vision OCR call built a raw genai.Client and invoked generate_content directly. Three reasons that is wrong here, the third load-bearing: - CLAUDE.md: new LLM-driven code belongs in backend/agents/ as a Pydantic AI agent, not a fresh client. - ADR-0008 made agents/_providers.py::model_for(task) the one place a model is chosen. GEMINI_VISION_OCR_MODEL was a competing knob that bypassed it; the slot is now SAPLING_MODEL_OCR_VISION like every other agent's. - Cost attribution. Logfire's instrument_pydantic_ai() tags every pydantic-ai span with tokens and USD; a raw client call is invisible to it and to the usage capture #118/PR #375 is building. Vision OCR is one metered call per scanned page — plausibly the largest per-document LLM spend in the app, and it would have been the one call the new cost dashboard could not see. The run is bounded by WORKER_LIMITS: it sits in a per-page loop, where an unbounded run multiplies a single runaway page across the whole document. Also fixes a latent bug this refactor surfaced. _extract_text_or_422 is sync but called from both async handlers (routes/documents.py:640, :771), so a bare asyncio.run raises there — and _apply_gemini_vision_fallback's per-page `except Exception: continue` would have swallowed it, silently turning vision OCR into a no-op on the main upload path. _run_from_anywhere hands the coroutine to a worker thread when a loop is already running, copying the context so agent.override and the active span survive. The module contract is unchanged: same function name and signature, same GeminiVisionUnavailableError semantics, GEMINI_VISION_OCR_ENABLED still the switch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(ocr): cache key, cost ceiling, sequential rescuers, accurate docs Four findings from the review of #420. Cache key omitted the model. _ocr_cache_key claimed to include "every flag that changes the output" but not the vision model, so switching models kept serving the old transcription for the full 30-day TTL. Model and page cap are now in the key, mixed in only when vision is enabled so the vision-off majority keeps its existing entries. GOT_OCR_MODEL_PATH has the same pre-existing gap; the docstring now names it instead of overclaiming. No cost ceiling. Each flagged page is one metered call, and nothing upstream bounds the count: routes/extract.py allows min(max_pages, 50) and the upload path has no rate limit at all. The #182 limit (10 req/60s) was sized when a request meant one bounded local OCR run. GEMINI_VISION_OCR_MAX_PAGES caps it per document, default 10, and logs how many pages it left behind — a silent cap reads downstream as a full transcription. if/elif made the rescuers mutually exclusive. Enabling both meant vision never ran, including on pages GOT-OCR failed to fill, recreating the exact "signal computed then dropped" bug this feature exists to fix. They now run in sequence — GOT-OCR first (local, free), then vision over what it could not fill. Both share one driver; GOT-OCR's gate is byte-for-byte unchanged. Three false claims. .env.example said an unreadable scan "is rejected" — it is not on this base; the upload paths convert only extraction *exceptions* to 422, so "" reaches the classify prompt and the model fabricates. That rejection is PR #419, still open. The module docstring said vision applies to "any engine"; it needs Docling to have run and succeeded. And the cache comment cited ADR 0019 (actually the SAPLING_MODEL_MODE test seam) for content-addressed chunk ids, whose dedup claim is untrue on main and becomes true only under PR #352. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(frontend): NEXT_PUBLIC_TEST_MODE determinism flag (#383) (#422) New src/lib/testMode.ts exports IS_TEST_MODE (build-time inlined), random() (mulberry32-seeded drop-in for Math.random), and now() (frozen 2026-03-11T12:00:00Z clock seam, overridable via globalThis.__SAPLING_TEST_NOW__). With the flag on: - KnowledgeGraph2D seeds its initial node positions and takes the reduced-motion path (synchronous fixed-tick settle) so two loads render identical coordinates. - KnowledgeGraph3D forces cooldownTicks=0 (the reduced-motion seam). - Landing page point cloud + floating cards park their rAF loops on a deterministic static frame; the frame's time read goes through now(). - AtmosphericBackdrop paints one still frame with seeded orbs. - HowItWorks/Study set framer-motion MotionGlobalConfig.skipAnimations. - Dashboard freezes the quote to index 0 and routes greeting, week strip, and relative labels through now(); Calendar (dueLabel, cursor, today) and Notetaker (relTime) do the same. Flag off, every seam passes through to Math.random()/Date.now() and no rAF/motion gate changes: production behavior is unchanged. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * test(infra): one-command local stack boot — make e2e-up / e2e-down (#384) (#423) Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * ci: run the integration lane on every push to main (#402) (#427) The subcutaneous suite (#396–#398) currently runs only on manual workflow_dispatch — a real-DB lane that never runs protects nothing. Per epic #402's open decision 3 (lean: main-only first, promote to a PR gate once #388's stability bar holds), trigger it on every push to main while keeping manual dispatch. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * ci: gate test_extraction_service.py — it needs none of the OCR stack The CI pytest step ignored four files. Three genuinely need what requirements.lock deliberately excludes: transformers (test_extraction_backends), docling (test_docling_integration), live network (test_ocr_pipeline). test_extraction_service.py needs none of them — it stubs every backend it exercises. It was swept into the list with its heavy neighbours, and the consequence is that nothing in it has ever gated a PR: not the OCR engine gating, not the content-addressed cache key (#97), and not the cost ceiling and rescuer sequencing added alongside this change. #420's own fallback and cache-key tests were ungated for the same reason. Verified against the locked (non-OCR) dependency set CI actually installs, using CI's exact command and env: 1069 passed, 23 skipped, 0 failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(e2e): Playwright harness and fixtures (#385) (#428) * test(e2e): Playwright harness and fixtures (#385) Browser-lane foundation for epic #402 — #386/#387/#392–#395 build on this. - frontend/playwright.config.ts: chromium-only, workers=1 (serial to start), retries=2 gated on CI, trace/video/screenshot on failure, JSON reporter (e2e/results/last-run.json) with per-attempt retry indices for #390 flake tracking, timezoneId pinned to America/New_York for the frozen #383 clock. No webServer block: the boot contract belongs to make e2e-up (#384); global-setup fails fast with the exact fix when the stack is down. - e2e/global-setup.ts: health-check the stack, mint a session for rich-user-active via POST /api/auth/test-login (#381) through the same-origin proxy, persist as storageState. - e2e/support/db.ts: the single DB seam — pg over 127.0.0.1:54322 (loopback-exact guard, mirroring #397), TRUNCATE mutable tables RESTART IDENTITY CASCADE with the #397 denylist, re-seed via the canonical db/seed_local_rich.py. - e2e/support/fixtures.ts: auto fixture resets the DB before each test; specs import test/expect from here. - e2e/smoke.spec.ts: one harness proof (authed /dashboard renders app-shell), deliberately not a journey. - build:test now bakes NEXT_PUBLIC_TEST_MODE=1 (the #383 flag; this composition is what it was built for). - ShellFrame: data-testid="app-shell" on both layout variants — the stable authed-shell anchor per the #382 convention. Verified against a cold make e2e-up boot: npx playwright test green twice in a row (truncate/re-seed isolation holds), tsc --noEmit, eslint, vitest (204 passed), and a plain production build all clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(e2e): review fixes — testid process + comment accuracy (#385) - Follow docs/frontend-testids.md 'Adding a surface' for app-shell (missed in the initial commit): App shell row in the owning-files table, an `app` inventory section noting ShellFrame.tsx and the smoke-spec anchor role, and ShellFrame.tsx added to the eslint no-restricted-syntax scope (passes clean — the frame renders no intrinsic button/input/textarea). Doc's 'six files' phrasing generalized now that the list has seven. - global-setup.ts: correct the cookie-flags comment — auth.py only sets Secure under an https FRONTEND_URL (config.py), so the local cookie is HttpOnly/Lax; we mint secure:true and Chromium accepts it on http://localhost. - smoke.spec.ts: correct both redirect comments — unauthed /dashboard goes to ${BACKEND_URL}/api/auth/google via the middleware (BACKEND_URL is always set under start:test), not to the landing page. Verified: npx tsc --noEmit clean; npx eslint . 0 errors with ShellFrame.tsx newly in scope (scoped run at --max-warnings=0 clean); vitest 204/204. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * fix(ocr): send the transcription prompt in the user turn, not as system Caught by the first real Gemini call anyone has made against this feature. Moving the instruction to `system_prompt` during the agent refactor changed what the model produces. Measured on a rasterized syllabus with known ground truth (231 chars of source text, 0-char text layer): prompt as system_prompt -> 743 chars: \documentclass{article}, five \usepackage lines, \begin{document}, a tabular, \end{document} prompt in the user turn -> 359 chars: clean Markdown table Both transcribe the facts correctly — every assignment, date and type matches. The difference is that as a system prompt, "Use LaTeX for mathematics" reads as a document-format directive rather than an instruction about math notation, so the model emits a whole LaTeX file. The preamble is not cosmetic. extracted_text feeds the classify, summary and concept prompts and is chunked into course_chunks for RAG, so "amsmath" and "booktabs" become candidate concepts on a graph shared by every student in the course — the same pollution this feature exists to prevent, arriving by a different door. Restores the wire shape the original raw-client implementation used (contents=[image, prompt]), verified to produce 358 chars of clean Markdown on the same fixture. The agent seam, the ADR-0008 model slot and the cost attribution are all unaffected — only the placement changes. The test now pins placement in the user turn and asserts the instruction is absent from any system prompt. Revert-proof: reintroducing system_prompt fails it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(e2e): journey — study room with two browser contexts (#394) (#431) Two signed-in contexts (rich-user-active + rich-user-second), one seeded room. Both contexts assert receipt of the other's message through the real propagation path — Supabase Realtime postgres_changes signal + decrypting REST re-fetch (#124) — and both users' knowledge graphs render. Zero waitForTimeout: cross-context sends only happen after each context's postgres_changes subscription is server-confirmed ("Subscribed to PostgreSQL" frame). Unblocking migrations (both verified-needed at runtime on the local migrations-only schema): - 0032: add the rooms columns routes/social.py already selects (topic/course/owner_id/updated_at/is_public) — bug #405 made every room listing endpoint 500 (verified: PostgREST 42703); columns stay nullable/unpopulated, the create_room semantics remain open in #405. - 0033: publish room_messages on supabase_realtime (guarded, idempotent) — verified empty publication locally; without it postgres_changes never fire, and the chat has no polling fallback. Harness additions (additive): e2e/support/session.ts mints a second user's storageState (cookie + the sapling_user localStorage identity that UserContext requires) via POST /api/auth/test-login; USER_SECOND joins stack.ts; global-setup.ts takes the #386 branch's localStorage fix verbatim so sibling PRs converge on identical content. Social.tsx joins the #382 data-testid convention (social-* inventory in docs/frontend-testids.md, eslint files array). Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * fix(ocr): per-run provider — every second vision call died on a closed loop Found by the live test added here, which is the only thing that could have found it: every other test in this feature substitutes the model, and a FunctionModel has no client and no event loop. Measured against the live API, calling the seam four times in one process: call 1: OK 302 chars call 2: RuntimeError: Event loop is closed call 3: OK 308 chars call 4: RuntimeError: Event loop is closed `_providers._provider` is a module-level GoogleProvider, so its async httpx client binds to the first loop `asyncio.run` creates and dies when that loop closes. Every `run_agent_sync` caller shares this — it is #354, and the sweep is still open in PR #358. Transcription is the only caller that runs in a LOOP, which turns a latent bug into an unusable feature: a 10-page scan alternates success and failure page by page, and `_apply_gemini_vision_fallback`'s per-page `except Exception: continue` keeps Docling's text without a word. Half a document silently degrades to the mangled OCR this feature exists to replace. So this path does not wait for #358. `fresh_ocr_vision_model()` builds a provider per run and is passed as a per-run `model=` override, leaving the shared `_provider` untouched so it cannot conflict with whatever #358 lands. It returns None outside SAPLING_MODEL_MODE=real, where the FunctionModel has no loop affinity and must not be overridden. Four consecutive live calls now pass. The fixture is an image-only math worksheet. A missing text layer alone is not enough to reach vision — Docling ships RapidOCR and reads rasterized prose fine. This page is reached because `_detect_math_without_latex` flags math-shaped content carrying no LaTeX, the scanned-math case the feature is for. Docling alone drops problem 3 entirely as `<!-- formula-not-decoded -->`; with vision it comes back as `$\sqrt{x^2 + 16} \leq 5$`. Tests live in the `live_llm` lane, not tests/integration/: they need Docling and a real model, not Postgres, and that lane's conftest mandates a running Supabase stack. Opt-in via RUN_LIVE_OCR=1 plus a real key; skipped otherwise, so CI's dummy key is a clean skip. One test guards the premise and fails loudly if Docling ever stops flagging the fixture, since the other two would then pass vacuously. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(e2e): journey — seeded session → dashboard (#386) (#429) * test(e2e): journey — seeded session → dashboard (#386) Co-Authored-By: Claude Fable 5 <norepl…
Description
The integration lane existed but did not test the database: two tests round-tripped through
db.connection.table()(PostgREST) both ways — proving the echo, not the DB — and two asserted only on the app's own JSON. This adds the raw-SQL assertion seam the lane was missing, the autouse truncate isolation, and the seeded-user / auth fixtures that #398 builds on. The defining rule: writes go through the app; assertions read back with raw SQL (psycopg), never through the layer that made the write.Changes Made
db_conn— session-scoped psycopg connection onSUPABASE_DB_URL(dict rows, autocommit so reads see the app's committed writes). The raw-SQL seam; not a PostgREST read._require_local_db_url— the non-negotiable safety gate.SUPABASE_DB_URLis independent of theSUPABASE_URLthat_require_local_stackchecks, and.env.staging/.env.productionboth hold live direct-Postgres strings, so a misconfigured var wouldTRUNCATEa real project. It parses the host (strict —127.0.0.1.evil.comis rejected, unlike a substring check) and raises, never skips on anything non-local._reset_between_tests— autouseTRUNCATE ... RESTART IDENTITY CASCADEof every mutable table + reseed of the rich baseline before each test → order-independence. The denylist preserves the migration-seeded reference layer (terms,roles,achievements,achievement_triggers,cosmetics) + the catalog hierarchy (schools,courses,course_offerings), which I verified against the schema carry no FK tousers, so no CASCADE from a truncated user table can ever reach them.seeded_userfactory (distinct, approved users) andauthed_client/other_user_client, replacing the per-testcookies.set(..., mint_session(...))boilerplate.test_local_stack.pyrefactored onto the fixtures: the flagship test POSTs a note through the real route and asserts the ciphertext at rest via raw SQL; a truncate-isolation pair proves ordering-independence; distinct-users and seeded_user tests cover the new fixtures.tests/test_integration_db_guard.py— proves the safety gate in the DEFAULT hermetic lane (pure URL logic, no DB), so it gates every PR.Related Issues
Closes#397
Testing
ruff check .clean; default CI lane 976 → 989 passed, 10 skipped, no regressions. The safety-gate test (13 cases) passes in the default lane. The 9 DB-backed integration tests collect and skip cleanly withoutRUN_INTEGRATION.Notes for Reviewers
Important
The 9 DB-backed tests could not be executed in the authoring environment — this cloud session's egress proxy returns
403on the Docker image CDNs, sosupabase startcannot pullsupabase/postgres/PostgREST and no local stack can come up. I verified everything provable offline (safety gate + default-lane green + collection), and I'm dispatching theintegration (local supabase)workflow on this branch to exercise the DB-backed lane against a real stack. Merge is gated on that run being green, in addition to the defaultBackend (pytest)check.SUPABASE_DB_URL, bypassing PostgREST. The_require_local_db_urlgate raising (not skipping) on non-local is the load-bearing invariant — a silent skip would read as "safe" while wiping real data. It's proven in the always-on lane._TRUNCATE_DENYLIST, or its rows vanish on the first reset. Called out in the conftest comment.config-touching imports (seed, encryption,main, session tokens) stay function-local, exactly as the pre-existing conftest did, soload_dotenv(override=True)still wins over the root conftest's hermetic keys.Generated by Claude Code
Summary by CodeRabbit