Uh oh!
There was an error while loading. Please reload this page.
fix(frontend): UserContext falls back to /api/auth/me on cookie-only sessions (#430) - #450
Merged
Merged
Conversation
…sessions A browser holding a valid sapling_session cookie but no sapling_user localStorage entry (cleared site data, another profile, a stale-cookie flow) loaded /dashboard forever on its loading skeleton: middleware admits the request on the cookie alone, but UserContext bootstrapped identity ONLY from localStorage, so userId never got set and Dashboard's `userReady && userId` load effect never fired. UserContext.tsx now falls back to the cookie-authenticated GET /api/auth/me (added as api.ts::getMe, same fetchJSON/same-origin convention the OAuth callback already uses against the same endpoint) when bootstrap finds no localStorage identity: on 200 it hydrates the context and write-throughs setActiveUser; on 401 it settles into the existing signed-out state instead of hanging. Local-mode mock bootstrap was already removed from this file in 35c2026, so no local-mode branch needed handling here. e2e/support/session.ts::mintStorageState gains an opt-in `omitLocalStorage` option (default unchanged: every existing journey still mints cookie + localStorage together) to mint a deliberately cookie-only storageState, and a new journey (e2e/auth-session.spec.ts) proves /dashboard hydrates from it instead of spinning, reusing dashboard.spec.ts's existing dashboard-courses-key-toggle / dashboard-course-code testids. Fixes#430.
📝 WalkthroughWalkthroughChangesCookie-only session hydration
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant UserProvider
participant getMe
participant AuthAPI
Browser->>UserProvider: Load dashboard without sapling_user
UserProvider->>getMe: Request /api/auth/me
getMe->>AuthAPI: Send sapling_session cookie
AuthAPI-->>getMe: Return user identity
getMe-->>UserProvider: Provide MeResponse
UserProvider-->>Browser: Render hydrated dashboard
Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deploying with |
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs | frontend-staging | 592d01e | Commit Preview URL Branch Preview URL | Jul 28 2026, 05:55 PM |
Addresses PR-review findings on the #430 cookie-only-session fix: - UserContext.tsx: corrected an inaccurate comment claiming the OAuth callback uses the same fetchJSON convention (it uses a bare fetch); the catch-block comment now also names the 404 case (a cookie naming a deleted user row, the #285-family scenario), not just 401/network. - UserContext.tsx: guard the fallback so it never runs on `/auth/*` routes. The OAuth callback POSTs /api/auth/session then calls GET /api/auth/me itself before its own setActiveUser — racing our own getMe() there was a near-guaranteed 401 before that POST resolves, and on a shared browser with a different user's still-valid session cookie present, could have momentarily hydrated the wrong identity before the callback's setActiveUser overwrote it. - UserContext.tsx: documented, rather than architected away, the one-401-per-anonymous-mount cost (UserProvider wraps the whole app; the HttpOnly cookie has no client-readable signed-in hint to gate the probe on without inventing new architecture). - api.ts: softened getMe's JSDoc — backend get_session_user_id also accepts an auth_token query param, so "cookie alone" overstated it; the real point is no user_id param is needed. - e2e/global-setup.ts: the near-duplicate header comment in support/session.ts was already corrected to past tense in the original #430 commit; this file's copy still asserted the pre-#430 behavior as current fact. Now consistent with support/session.ts. No behavior change to the 200/401 hydration path itself, no new data-testids, no stack run (verification is tsc/eslint/vitest only, per the review's ask). Addresses #430 PR review round 1.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
frontend/e2e/auth-session.spec.ts (1)
55-60: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the required localStorage write-through.
This proves hydration, but a regression that sets only React state would still pass while violating the persistence contract.
Proposed coverage
await expect( page.getByTestId("dashboard-course-code").filter({ hasText: "MATH210" }), ).toBeVisible(); ++ const storedUser = await page.evaluate(() =>+ JSON.parse(localStorage.getItem("sapling_user") ?? "null"),+ );+ expect(storedUser).toMatchObject({+ id: USER_ACTIVE,+ name: "Rich Active",+ avatar: "",+ });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/e2e/auth-session.spec.ts` around lines 55 - 60, Extend the authentication session test around the existing dashboard identity assertion to verify the required localStorage write-through as well. Read the relevant persisted user/session entry after hydration and assert it contains the resolved rich-user-active identity, ensuring React state alone cannot satisfy the test.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@frontend/e2e/auth-session.spec.ts`:
- Around line 55-60: Extend the authentication session test around the existing
dashboard identity assertion to verify the required localStorage write-through
as well. Read the relevant persisted user/session entry after hydration and
assert it contains the resolved rich-user-active identity, ensuring React state
alone cannot satisfy the test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ccee6456-93f0-4055-9475-c28b5a5a2974
📒 Files selected for processing (5)
frontend/e2e/auth-session.spec.tsfrontend/e2e/global-setup.tsfrontend/e2e/support/session.tsfrontend/src/context/UserContext.tsxfrontend/src/lib/api.ts
Uh oh!
There was an error while loading. Please reload this page.
AndresL230 added a commit
that referenced
this pull request
Jul 28, 2026
AndresL230 added a commit
that referenced
this pull request
Jul 28, 2026
AndresL230 added a commit
that referenced
this pull request
Jul 28, 2026
AndresL230 added a commit
that referenced
this pull request
Jul 28, 2026
AndresL230 added a commit
that referenced
this pull request
Jul 29, 2026
…#456) * docs(e2e): refresh known-bugs catalogs after the #402 follow-up batch Fixed and closed: #355, #430, #435, #436 (+root cause #354), #439, #446 (merged via #447/#448/#450/#451/#453/#454). #441's fix (PR #452, PG15 pin) is in final verification, merging imminently. Known-open remaining: #449 (get_courses per-enrollment fan-out produces duplicate course_id rows; Library's instance fixed render-side in #451, Tree/Dashboard/etc. and the DocumentUploadModal picker still exposed). Updates docs/e2e-exploration.md (§6 logscan allowlist note, §7 triage category 3 + worked examples, §8 fixme lifecycle example) and scripts/explore/explorer-prompt.md's known-bugs section to match. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(e2e): #441 landed — move it into the recently-fixed list Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
AndresL230 added a commit
that referenced
this pull request
Jul 29, 2026
…455) * feat(evals): complete extraction-accuracy harness + baselines (#148) Finish the agent eval harness so migrated agents are validated on accuracy, not just smoke-tested — the evidence base the gemini_service cutover (#151) needs. - Record 80 cassettes across the five offline datasets (classification, summary, concepts, syllabus, quiz); replay is now deterministic + keyless. - Gate on regression below a committed baseline (baselines.json) instead of "< 1.0" — the harness measures accuracy, it doesn't assume perfection. - Retry transient 503/429 while recording; force UTF-8 output so non-ASCII cases don't crash rich on a Windows cp1252 console. - Add run_all.py (one combined scored run) and enable evals.yml to run it in replay mode on PRs touching backend/agents/** or the harness. - Document the record/refresh workflow (tests/evals/README.md, README) and the design + baselines (ADR 0020). - Exclude chat_tutor: its retrieval tool reads a live Supabase and can't run offline; folded into the graph-grounded tutor work (#149). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(rag): gate below-seam embedding calls on SAPLING_MODEL_MODE (#439) (#454) services/rag_service.py's _embed_query/_embed_document/_embed_documents_batch and routes/documents.py::_index_document_chunks's catalog-relevance gate construct raw google.genai.Client objects directly, predating the #391 SAPLING_MODEL_MODE seam — so function mode (the hermetic E2E default) still fired live gemini-embedding-001 calls on every document upload, quiz generate, and tutor turn with a course_code, silently billing whenever a real key was present. Add agents/_providers.py::model_mode() as the one sanctioned public read of the seam for call sites outside agents/, and gate every embed call site on it. rag_service.py's client is now built lazily (_get_client()) and only ever reached after a model_mode() == "real" check; in non-real mode the _embed_* helpers raise before touching the client, which the existing broad try/except in retrieve_chunks/index_document_chunks already catches — reusing that path makes the deterministic empty/no-op result the designed behavior instead of an accident of a swallowed exception. Same pattern for the documents.py relevance-gate client, scoped tightly to that block. Interim mitigations (scripts/explore.sh, e2e.yml dummy-key forcing, the e2e_oracles logscan allowlist) are untouched — now defense in depth. * fix(documents): resolve abstract course_id in /api/documents/user (#435) (#451) * fix(documents): resolve abstract course_id in /api/documents/user/{id} Library.tsx filters and labels documents on d.course_id, but the route only ever returned offering_id — every upload silently fell into "Uncategorized" and never matched a course filter. Resolve each row's course_id via services.academics.offering_course_id, batching per unique offering_id (mirrors routes/learn.py::list_sessions) rather than once per row. Adds backend coverage for the enriched response shape (single/batched/ missing offering_id) and extends the #387 upload journey with a library-filter assertion: after upload, filtering by the seeded course (resolved from the persisted row's offering_id) must show the document, and the "Uncategorized" filter must not. Fixes#435. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(documents): PR review round 1 — nullable type, honest tests, safe decrypt Address review findings on the #435 course_id fix before merge: - frontend/src/lib/types.ts: Document.course_id is string | null (the fix's own tests pin course_id: null as a real response shape). Library.tsx already treated it as possibly-falsy; tsc surfaced one spot assuming non-null (courseLookup[d.course_id]) — guarded with `?? ""`. - backend/tests/test_documents_routes.py: relabeled the offering_id=None test as defensive-code coverage (schema-unreachable — 0025 makes documents.offering_id NOT NULL) and added the actually-reachable null branch: a present offering_id that offering_course_id fails to resolve. - backend/routes/documents.py: wrap the concept_notes decrypt_json call in list_documents in try/except, matching the established pattern at _existing_doc_by_request_id and scan_document_concepts — decrypt_json re-raises when both decrypt and plaintext-parse fail, so one corrupted row no longer 500s the whole list. Added a regression test (red-first) proving the corrupted row degrades to concept_notes: [] while sibling rows still return. - frontend/e2e/upload.spec.ts: header now notes the #435 library-course-filter regression coverage the journey carries. Refs #435. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(library): dedupe course filter pills by course_id (#435) Stack verification of the #435 library-filter journey caught a real duplicate-render bug: GET /api/graph/{userId}/courses returns one row per enrollment, so a course with two offerings (e.g. CS101 fall + spring) surfaced as two rows sharing the same course_id. Library.tsx built its filter pills directly off that per-enrollment list, so the same course rendered two identical `library-course-filter-{courseId}` pills — a strict-mode Playwright locator violation, and a real UX bug (duplicate rows in the sidebar). Root cause is #449 (get_courses one-row-per-enrollment), which stays out of scope here — the gradebook depends on the per-enrollment shape. This is the frontend render-side fix: dedupe by course_id via a Map at the two derivation points that render per-course UI (the filter pills and the course label lookup), keeping the first enrollment's row as the stable representative. The raw per-enrollment `courses` list is otherwise untouched (course-scan lookup, upload-button disabled check, and the upload modal's course list still see every enrollment). The e2e library-filter assertion (fronten/e2e/upload.spec.ts, #435) was left as strict-mode (no .first() masking) per review — it should now pass because the pills are unique, not because the locator was weakened. Refs #435, #449. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * fix(agents): loop-safe Gemini provider — kill the 'Event loop is closed' flake (#436, #354) (#453) * fix(agents): make the shared Gemini provider loop-safe, not a sweep (#436, #354) test_ocr_pipeline.py::test_save_to_db errored with "RuntimeError: Event loop is closed" on main — the #354 root cause: agents/_providers.py's module-level GoogleProvider eagerly builds an httpx.AsyncClient whose connection pool binds internal asyncio primitives to whichever event loop is running the first time a request goes out over it. Every agent is built once at import time sharing that one provider, so run_agent_sync's per-call asyncio.run() (and, just as much, any test calling asyncio.run() more than once against the same shared agent in one process, as test_agent_parse then the parsed_assignments fixture do here) trips the stale-loop reuse on every second real call. PR #358 (never merged; reviewed clean, just fell through the cracks) fixed this by sweeping eight run_agent_sync call sites to pass a fresh-provider model= override per call. Adapting it verbatim wouldn't have fixed#436: calendar_service's syllabus_extraction_agent, the actual caller behind this test, isn't on that sweep list — and agents/ocr_vision.py already needed its own ad hoc copy of the same idea for a path #358 didn't cover, proof the sweep needs rediscovering at every new call site. The issue itself named this "the tactical sweep" with "make the shared provider loop-safe" as the strategic fix. Since every agent gets its model from model_for()/google_model() exactly once, fixing those two functions fixes every caller — present and future — with no sweep required. _LoopSafeGoogleModel (a GoogleModel subclass) keeps one GoogleProvider per currently-running event loop (a WeakKeyDictionary keyed by the loop object, self-cleaning once a throwaway loop is GC'd), rebuilding only when a NEW loop calls it and reusing the cached one for as long as that loop lives — identical connection-pooling behavior to the old singleton for FastAPI's one persistent per-process loop, and no stale-loop reuse for run_agent_sync's or a test's disposable ones. This also makes ocr_vision.py's ad hoc fresh_ocr_vision_model() workaround redundant; removed it and its call-site override in gemini_vision_backend.py. Proof: tests/test_loop_safe_google_model.py (new, hermetic) pins the per-loop cache directly. tests/test_ocr_pipeline.py run 3x in one process (pytest.main() loop, since repeated identical CLI paths dedup to a single collection) put 6 asyncio.run() cycles through the same shared agent — 33/33 green, zero "Event loop is closed". Full suite green twice back-to-back (1161 passed, 26 skipped, 0 errors both times). ruff check clean. Fixes#436Fixes#354 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(agents): eliminate the loop-safe provider's shared-pointer race (#436, #354) PR review on the prior fix found a Critical concurrency bug, backed by an empirical repro: 6 threads x 20 sequential asyncio.run calls against one shared _LoopSafeGoogleModel, with an asyncio.sleep standing in for the real await gap, produced 95/120 mismatches between the provider bound for a call and the one actually read back. Root cause: _bind_to_current_loop() resolved the right provider under a lock, but handed it off via `self._provider = provider` — a single shared mutable attribute on a Model instance that is itself a process-wide singleton (every agent is built once at import time). GoogleModel._generate_ content reads self.client (-> self._provider.client) only AFTER an await (self._build_content_and_config(...)); in that window, a different thread running a different event loop — exactly what happens under concurrent requests, since every sync-def route drives run_agent_sync on a fresh thread + throwaway loop, and gemini_vision_backend._run_from_anywhere does the same for OCR — could rebind that same shared attribute. The first call would then resume and read a provider bound to someone else's, possibly already-closed, loop: a deterministic every-second-call flake became a probabilistic, load-dependent one. Fix: remove the hand-off entirely. self._provider (the base GoogleModel/ Model attribute) is now a fixed template, set once and never reassigned, backing only the reads that are genuinely loop-independent (system/base_url, and the bare .name/.base_url pydantic-ai's own count_tokens/usage-metadata code reads directly) — safe because every provider this module constructs uses identical arguments. .client — the one read that IS loop-affine — is now a property that resolves asyncio.get_running_loop() -> the loop-keyed WeakKeyDictionary fresh, at the exact moment of every access, with no instance-attribute write in between "resolve" and "use". Every inherited GoogleModel method reads self.client through ordinary attribute lookup, so this one override covers all of them — request/count_tokens/request_stream no longer need (and no longer have) their own overrides. __aenter__/ __aexit__ still act on the current loop's provider explicitly. Verified standalone: the buggy hand-off shape reproduces 119/120 mismatches; the fixed property-based design shows 0/120, both under the same 6x20 stress. Added TestConcurrentAccessIsRaceFree to tests/test_loop_safe_google_model.py: a minimal stand-in of the original hand-off design proves the test shape itself would have caught the round-0 bug (asserts it DOES mismatch), then the same shape run against the real _LoopSafeGoogleModel asserts zero mismatches. Deterministic and hermetic — no network. agents/ocr_vision.py and gemini_vision_backend.py needed no further changes: they already call through model_for("ocr_vision")'s default model, so the OCR per-page-loop concurrency concern the review raised falls out of this fix directly. Re-verified: threaded test suite 5x back-to-back (9/9 every time), the OCR pipeline module 3x in one process (pytest.main() loop, 33/33 green), full hermetic suite once (1164 passed, 26 skipped, 0 errors). ruff check clean. Fixes#436Fixes#354 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * ci(db): pin local/CI Postgres to 15, matching staging/prod (#441) (#452) * ci(e2e): pin local/CI Postgres to 15, matching staging/prod (#441) supabase/config.toml's major_version pins the local/CI Postgres (via supabase start) independently of hosted staging/prod; PR #440 set it to 17 for local-dev/CI consistency, which drifted the deterministic lane away from what production actually runs. Pin it down to 15 instead — hosted staging/prod stay untouched (bumping them is a separate, outward-facing ops action), and local/CI consistency is preserved since both still follow the same config.toml pin, just at 15. - supabase/config.toml: major_version 17 -> 15 (also the Supabase CLI's own documented default for this key). - .github/workflows/e2e.yml: update the header comment that previously explained "deliberately going against" the PG15 leaning. - backend/tests/integration/test_postgres_version.py: assert the real server's major version via SHOW server_version_num, so drift is loud. Lives in the opt-in integration suite because .github/workflows/integration.yml boots the local stack from empty and runs `RUN_INTEGRATION=1 pytest -m integration` on every push to main — this actually executes in CI, against the same config.toml-pinned Postgres the browser lane (e2e.yml) also boots. - docs/local-supabase.md: update the "Postgres version" troubleshooting entry and add a reset note for stacks provisioned before this pin (the CLI does not swap a running container's image just because config.toml changed). Migration chain audited for PG16+-only constructs (MERGE, JSON_TABLE, REGEXP_*, EXCLUDE, etc.) — none found; UNIQUE NULLS NOT DISTINCT (0023_graph_integrity.sql) is itself a PG15 feature, so it's fine on the target version. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(e2e): fix#441 PR-review findings — history + reset guidance Two documentation-accuracy fixes from PR review, no behavior change: 1. backend/tests/integration/test_postgres_version.py docstring misattributed the PG17 pin's origin to PR #440. Git history shows the pin was born with the local Supabase stack itself (9f54739, config.toml created with major_version = 17) — #440 only added the e2e.yml comment rationalizing the already-existing PG17 against epic #402 decision 2's PG15 leaning, and noted the skew was tracked in #441. Rewrote the causal narrative to match; updated the assert failure message's reset guidance to match fix 2 below. 2. docs/local-supabase.md's "Postgres version" troubleshooting entry was self-contradictory: it said the CLI "does not swap a running container's image just because config.toml changed," then claimed scripts/local-db-reset.sh (supabase db reset) "recreates the Postgres container against the pinned version." Reading local-db-reset.sh: it never calls supabase stop/start, so it can't replace a running container's major version — confirmed against supabase/cli issues #5555 and #4522 (db reset does not reliably pick up a changed major_version and can leave a half-upgraded, broken container). Restructured so the reliable path is primary for a version change: `supabase stop --no-backup` + `supabase start` (fresh container, correct major, no app schema yet), then local-db-reset.sh / make e2e-up for their actual job — migrate + seed. Static-only per the controller's instructions (no stack boot); hermetic backend suite re-run clean (1155 passed, same pre-existing unrelated test_ocr_pipeline.py event-loop flake, #354). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * docs(e2e): refresh known-bugs catalogs after the #402 follow-up batch (#456) * docs(e2e): refresh known-bugs catalogs after the #402 follow-up batch Fixed and closed: #355, #430, #435, #436 (+root cause #354), #439, #446 (merged via #447/#448/#450/#451/#453/#454). #441's fix (PR #452, PG15 pin) is in final verification, merging imminently. Known-open remaining: #449 (get_courses per-enrollment fan-out produces duplicate course_id rows; Library's instance fixed render-side in #451, Tree/Dashboard/etc. and the DocumentUploadModal picker still exposed). Updates docs/e2e-exploration.md (§6 logscan allowlist note, §7 triage category 3 + worked examples, §8 fixme lifecycle example) and scripts/explore/explorer-prompt.md's known-bugs section to match. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(e2e): #441 landed — move it into the recently-fixed list Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * docs: CLAUDE.md — E2E lanes, stack-lock protocol, function-mode seam conventions (#457) Every agent session on this repo now learns the E2E system up front: the deterministic stack commands (e2e-up/down, Playwright lane, oracles, explore), the pre-merge three-way verification expectation, the fix→promoted-journey pairing, the machine-singleton flock protocol, and the function-mode seam rules (fixed constants, handler registration, no raw genai clients below the seam). Follows the #402/#403 epics and the 2026-07-28 bug-queue batch. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * fix(agents): guard run_agent_sync against running event loops (#354 follow-up) (#358) * fix(agents): guard run_agent_sync against running event loops (#354 follow-up) Reworked from the original fresh-client sweep: the cross-loop client problem is now solved by _LoopSafeGoogleModel (#453), so the per-call fresh-client plumbing and the subject_root dedup (landed separately, #355) are dropped. What remains is the piece main still lacks: - run_agent_sync detects a running event loop, closes the handed coroutine (no 'never awaited' warning) and raises a clear error the try/except-guarded sync-from-async callers can degrade on, instead of letting asyncio.run raise opaquely. - HEALTH_PROBE_MODEL constant so probe sites can't drift. - Regression tests for both loop paths. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(agents): name the real async->sync call chain in the loop-guard rationale Review found the cited example chain (build_system_prompt -> get_course_context) never reaches run_agent_sync; the reachable chain is _legacy_chat -> apply_graph_update -> update_course_context -> _generate_summary_with_gemini -> run_agent_sync. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * feat(learn): stream tutor replies over SSE with live graph deltas (#70, #74) (#349) * feat(learn): stream tutor replies over SSE with live graph deltas (#70, #74) — rebased onto main Net rework of feat/streaming-tutor against current main: - Ported: services/chat_stream.py (stream_agent_turn rung ladder), agent_events.py chat-event vocabulary, /chat/stream + /start-session/stream SSE routes, frontend sse.ts + api.ts stream consumers, Learn.tsx/ChatPanel wiring + tests, ADR as 0020. - Dropped the fresh-client plumbing (_fresh_stream_model, fresh_google_model, per-call model= overrides): superseded by _LoopSafeGoogleModel (#453). The streamed routes now inherit the agent's own model so the SAPLING_MODEL_MODE seam applies. - NEW: function-mode seam serves streamed runs — _function_model_for gains a stream_function that replays the registered handler's ModelResponse as deltas (text + DeltaToolCall), keeping E2E_* constants byte-identical across JSON and SSE lanes; covered by two new seam tests. - Learn.tsx edgeKey NUL byte rewritten as \u0000 escape (text-clean). - tutor-stop testid added (e2e-surface lint #382) + docs entry. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(learn): harden stream persistence + concurrent-stream guard (review findings) - stream_agent_turn: on_complete failures after a fully-streamed reply now yield the structured ADR-0020 error event instead of aborting the SSE response uncaught (headers are already flushed at that point). Regression test added. - Learn.tsx: abort any in-flight stream controller before starting a new one — a graph-node click could begin a session while a reply streamed, interleaving two streams into shared state. - Docstrings: the on_complete/legacy_fallback invariant is 'at most one, never both' (error rungs run neither), not 'exactly one'; PENDING_SESSIONS wording updated to match. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * docs(evals): renumber harness ADR to 0021 — 0020 was taken by the streaming-tutor ADR (#349) Also merges current main (clean; #349's frontend/stream files and this harness touch disjoint trees). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(evals): fail closed on missing baselines — an ungated dataset or evaluator must not PASS CI Review finding: the regression gate iterated only committed baseline keys, so a dataset absent from baselines.json (or an evaluator added without refreshing it) reported PASS with zero protection — right as evals.yml becomes a PR gate. Both paths now FAIL with a pointed message; verified empirically (removed dataset + evaluator entries -> exit 1, restored -> exit 0). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Andres Lopez <190146319+AndresL230@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#430.
Root cause
UserContextbootstraps client identity only from thesapling_userlocalStorage entry (written solely by sign-in flows). A browser holding a valid HttpOnlysapling_sessioncookie but no localStorage entry — cleared site data, another profile, stale-cookie flows (#285 family) — gets admitted by middleware but renders the dashboard loading skeleton forever.Fix
frontend/src/context/UserContext.tsx: when bootstrap finds no localStorage identity, fall back to cookie-basedGET /api/auth/me(via the same-originfetchJSONconvention — new typedgetMe()inlib/api.ts); on 200 hydrate and write-throughsetActiveUser(the existing identity write path — no new source), on 401/failure settle into the normal signed-out state. Effect is guarded against post-unmount setState.Promotion (Chapter 2 → Chapter 1, promotion 3 of 3)
frontend/e2e/support/session.ts:mintStorageStategains an opt-inomitLocalStorage— default output byte-identical to before (existing journeys unaffected; sole existing caller verified).frontend/e2e/auth-session.spec.ts: new journey minting cookie-ONLY storage state, loading/dashboard, and proving hydration (courses key toggle + seededMATH210course code — assertions that require the data load to complete, not mere skeleton absence).Verification
tsc --noEmitclean; eslint 0 errors (36 pre-existing warnings, none in touched files); frontend unit suite 204/204.🤖 Generated with Claude Code
Summary by CodeRabbit