Uh oh!
There was an error while loading. Please reload this page.
fix(frontend): DEPLOY_ENV single source of truth + env-mismatch guard (fixes staging session_expired) - #409
Merged
Merged
Conversation
`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>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`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>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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>
/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>Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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>
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>
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>
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>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…#361) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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>
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>
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>
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>
…ack (#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>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`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>
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>
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>
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>
…#140) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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>
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>
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>
…ck (#140) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…399) (#444) * feat(explore): scripts/explore.sh harness + make explore + .explore gitignore (#399) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(explore): explorer mission prompt — persona, break-things mandate, oracle cadence (#399) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(explore): /explore repo skill — interactive exploration mode (#399) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(explore): acquire the lock before touching .explore/, don't clobber GEMINI_API_KEY (#399) A busy lock previously still tripped do_up's trap/wipe path, tearing down another session's live stack via scripts/e2e-down.sh and deleting its .explore/ artifacts before the lock check ever ran. start_lock_holder now runs first, the do_down safety trap arms only after the lock is held, the .explore/ wipe (still selective, preserving lock.pid/lock.ok) happens after that, and a failed acquisition cleans up its own holder/pid files before exiting. GEMINI_API_KEY now defaults only when unset instead of always overwriting an operator's real key. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(explore): stop lock-bookkeeping clobber between concurrent sessions (#399) start_lock_holder previously deleted lock.ok and unconditionally wrote lock.pid before knowing whether the flock would actually succeed. A failed attempt from session B (lock held by session A) would delete A's lock.ok and overwrite A's lock.pid with B's own doomed PID, then B's busy-path cleanup removed the file entirely — leaving A's later `down` unable to find A's holder, leaking both the detached process and the machine-singleton lock. Now the holder subprocess itself is the only writer of lock.ok, and only ever after its own `flock -n 9` succeeds (atomic temp-file + mv, content = the holder's own $$). The parent only polls for that self-identifying marker and touches nothing on disk on the failure path, so a busy lock can never clobber another session's bookkeeping. lock.pid is retired — lock.ok's content is now the sole source of truth stop_lock_holder reads. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(explore): adjustments from first live bounded exploration (#399) The first bounded acceptance run (task-8-report.md) produced a session.log with exactly one line — claude -p's own "Error: Reached max turns" — because the default --output-format=text only prints the FINAL message, never the intermediate tool_use/tool_result turns. That failed the #399 acceptance bar ("session.log shows real Playwright MCP tool calls"). Switch run_explorer to --output-format stream-json --verbose (the CLI requires both together) and reformat the JSONL with jq into readable [assistant]/[tool_use]/[tool_result]/[result] lines, truncated to 500 chars each, so session.log is both grep-able and human-skimmable. fromjson? tolerates stray non-JSON lines instead of aborting the whole transcript; claude -p's stderr now goes to its own session.stderr.log so it can never interleave with (and corrupt) the JSON stream jq parses. Verified: a second bounded run (EXPLORE_MAX_TURNS=25) produced a 281-line session.log with 18 mcp__playwright__* tool calls across /dashboard and /library, plus findings.md with a re-confirmed #430 repro and the oracle final pass re-confirming #355. * fix(explore): --isolated for storageState to actually apply (#399) The follow-up bounded-run diagnosis found a contradiction: run 2's session cookie was accepted (/api/users 200) but the UI acted fully signed-out (Sign In button, dashboard skeleton) — the #430 symptom. Run 1's "Secure cookie dropped over http" diagnosis didn't hold up either, since Chromium does accept Secure cookies on http://localhost. Root-caused by reading @playwright/mcp@0.0.78's bundled source (playwright-core/lib/coreBundle.js): without --isolated, the MCP server launches ONE PERSISTENT Chrome profile keyed only by sha256(cwd) — reused across every explore.sh run from this checkout, never wiped by teardown — and its client factory does `config.browser.isolated ? await browser.newContext(contextOptions) : browser.contexts()[0]`. In the default (non-isolated) branch it just grabs the already-open persistent context and never calls newContext with our --storage-state at all. Verified empirically: wiped the profile dir, booted a clean stack, and drove a 3-turn claude -p probe against the (then-current) mcp.json — navigating to /dashboard redirected straight to a REAL accounts.google.com sign-in page, and sqlite3 on the profile's Cookies db afterward showed zero sapling_session rows (neither cookie nor localStorage from storageState.json was ever applied). The identical probe with --isolated added rendered the real, fully-authenticated dashboard on the first navigation, with localStorage.sapling_user correctly present — this is the same mechanism (ephemeral browser.newContext(contextOptions)) @playwright/test itself uses in Chapter 1's global-setup.ts. Also corrected mint_storage_state's cookie to secure:false, matching what the backend's own SECURE_COOKIES policy actually issues for the http://localhost local stack (config.py derives it from FRONTEND_URL's scheme) — the file should mirror reality regardless of which flag turned out to be the load-bearing one. Verified: EXPLORE_MAX_TURNS=12 make explore reached a signed-in dashboard (nav shows "Rich Active" / "Account", full authenticated menu) using only 2 real Playwright tool calls, zero sign-in recovery turns. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(explore-prompt): stub findings before deep-diving root cause (#399) The definitive acceptance run (harness fixes verified: real auth via --isolated, breadth across 2+ surfaces) hit a genuine new bug — resuming a tutor session 500'd on POST /api/graph/.../concept-description, root-caused via the explorer's own backend-log investigation to a missing SAPLING_FUNCTION_HANDLERS registration for 'concept_describe' (caught independently by the oracle's logscan pass too, so it's not lost, just not explorer-authored). But the explorer spent its remaining turns tailing logs and checking processes to nail the exact LookupError, and ran out of budget before writing an F<N> entry to findings.md — a real gap against #399's "readable transcript AND findings file" bar, distinct from any flag/mechanism defect. Add a "stub it before you dig" ground rule: write a one-line stub finding the instant something looks off, before further root-causing. A written stub survives a turn-budget cutoff; a perfect unwritten diagnosis does not. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(explore): PR-444 review fixes — teardown guards, dummy key, jq preflight, scoped Edit, EXPLORE_USER wiring (#399) - do_down: guard every fallible write with || true so a failed findings.md write can never abort the EXIT trap before e2e-down.sh / stop_lock_holder run (was leaking the stack + machine-singleton lock on write failure). - GEMINI_API_KEY: export unconditionally (matches CI's dummy, #439) instead of deferring to an ambient real key that can bill the below-seam RAG path. - SEED_RICH=1 exported unconditionally so an ambient SEED_RICH=0 can't silently defeat the rich dataset this harness requires. - preflight: add missing `jq` check (hard dependency of the transcript pipeline) so a missing jq fails in seconds, not after a full stack boot. - allowedTools: replace unscoped Write,Edit with Read,Edit(.explore/**) — Write(...) patterns aren't matched by the CLI's file permission check, so only a path-scoped Edit rule actually restricts writes to findings.md. do_up now pre-creates .explore/findings.md so the explorer always has an existing file for the scoped Edit grant. - EXPLORE_USER: derive the sapling_user display name from the user id (case map for the five seeded rich-* users, verified against db/seed_local_rich.py) instead of hardcoding "Rich Active"; pass --user "$EXPLORE_USER" to both oracle invocations in do_down. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
#401) (#445) * docs(e2e): Chapter 2 exploration runbook — kick-off, triage, promotion pipeline (#401) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(e2e): stop citing a gitignored planning file from the runbook Self-review catch: task-8-report.md lives under .superpowers/ (gitignored), so pointing the shipped runbook at it as evidence was a dead reference for anyone without that local planning history. Describe the acceptance-testing evidence inline instead, and fix "earlier round of the same run" to the accurate "separate run of the same acceptance round" (task-8-report.md's run 4 found the tutor-resume 500; run 5 found the wrong-data bug). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(e2e): fix awkward line wrap in runbook Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(e2e): fix runbook per code-review — seam artifact vs real bug, timing, cross-refs (#401) Code-review on PR #445 found the flagship "wrong-data upload" example was a seam artifact (function-mode's E2E_DOC_* fixtures return identical canned output for every upload by design), not a real bug — reframe it as a worked "drop + improve the harness" triage example and promote the genuinely novel tutor-resume 500 (concept_describe unregistered in agents/function_handlers_e2e.py) to the flagship promotable example instead. Also: reconcile the "few minutes" (§3) vs "~10 minutes" (§9) timing claims into one warm-cache-vs-first-run story, and drop two dangling section cross-refs (§6's inline traces/ caveat didn't need a pointer; "forces both (see §3)" pointed at a section that never explained the fact) plus align the --check example with the oracle's own sorted default order. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(e2e): rewrap code span so the module path doesn't split mid-token (#401) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…r enrollment (#447) GET /api/graph/{user_id} returned the subject-root node (subject_root__<course_id>) and its ~5 hub-spoke edges duplicated for any user with two offerings of the same abstract course, because subject-root synthesis in graph_service.get_graph iterated enrollments instead of distinct abstract course ids. Fixes#355. - backend/services/graph_service.py: track seen_course_ids and skip synthesizing a second subject_root/hub-spoke set for a course already processed, so the API never returns two nodes with the same id. - backend/tests/test_graph_service.py: TDD regression test — a user with TWO offerings of the same abstract course now gets exactly one subject_root__<course_id> node and one hub spoke per concept node (failed before the fix: 3 node ids incl. one dup, now 2 unique). - frontend/e2e/graph.spec.ts: un-fixme the #355 acceptance test (promotion 1 of 3) and refresh the header/pre-test/companion comments that described the bug as still open. No assertions relaxed. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…448) * fix(agents): register concept_describe function-mode handler (#446) Tutor-resume 500s because agents/function_handlers_e2e.py registered six tasks but not concept_describe, so agents/_providers.py::_dispatch raised a LookupError that routes/graph.py did not catch, escaping as a 500 on POST /api/graph/{user}/concept-description. - function_handlers_e2e.py: register a concept_describe handler, emitting a fixed ConceptDescription payload (E2E_CONCEPT_DESCRIPTION) through the agent's real structured-output tool, same _structured_output helper as the document-pipeline handlers. Request-path, not a post-response BackgroundTask, so registering is safe; quiz_context stays deliberately unregistered per its existing docstring rationale. - routes/graph.py: catch LookupError alongside AgentRunError/httpx.HTTPError/ ValidationError in describe_concept so a misconfigured function-mode seam degrades to a 502 instead of a bare 500. - tests/test_graph_concept_description.py: cover the LookupError -> 502 degradation path. - tests/test_e2e_function_handlers.py: cover the new handler through the real concept_describe_agent (constants-sync + output-schema contract). - frontend/e2e/tutor.spec.ts: promote a Chapter 1 journey — resuming the seeded "Understanding Recursion" session auto-focuses its topic node in the knowledge-map rail, which has no stored description and so exercises the concept-description function-mode path; asserts the fixed handler constant renders in the rail's focus card. - Learn.tsx / docs/frontend-testids.md: add the tutor-focus-concept-description testid the new spec anchors on. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(agents): narrow #446's route catch to UnregisteredHandlerError PR review (two reviewers, one with an empirical KeyError repro) found the prior fix's except tuple over-broad: `except (..., LookupError)` in routes/graph.py::describe_concept also catches KeyError/IndexError (both LookupError subclasses), silently downgrading any unrelated bug deep in the agent-run path to a 502 "concept-description agent failed" instead of the generic 500 the route's introducing commit (502e324) intended for unexpected exceptions. - agents/_providers.py: add `UnregisteredHandlerError(LookupError)` and raise it (instead of bare LookupError) at _dispatch's no-handler-registered site. Keeping LookupError as the base preserves any existing LookupError callers. - routes/graph.py: catch the specific `UnregisteredHandlerError` instead of the builtin `LookupError`. - tests/test_graph_concept_description.py: renamed the degradation test to raise UnregisteredHandlerError (still asserts 502), and added test_unrelated_key_error_falls_through_to_500 reproducing the reviewers' repro (bare KeyError from run_agent_sync must still 500). Verified test_unrelated_key_error_falls_through_to_500 fails (502 instead of 500) against the prior bare-`except LookupError` code and passes against this fix. Refs #446. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…sessions (#430) (#450) * fix(frontend): UserContext falls back to /api/auth/me on cookie-only 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. * fix(frontend): review round 1 fixes for the #430 UserContext fallback 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.
#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.
…) (#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>
…ed' 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(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>
…#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>
…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>
…ollow-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>
#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>
…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>
…coarse timers (#346) (#351) * fix(limits): retry_after ceiling capped at window — deterministic on coarse timers (#346) Rebased onto current main: main had already adopted math.ceil in the flashcard limiter; this keeps that and adds the min(window, ...) cap to BOTH sliding-window limiters (services/request_limits.py still had the old int(...) + 1 overshoot), plus regression tests for coincident timestamps and sub-second remainders. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(limits): pin the backward-clock branch the min() cap exists for; align twin comments Review follow-ups: request_limits.py's comment now names the negative- elapsed (NTP step) case the cap protects against, matching its twin; a regression test in both limiter test files freezes time backward so a future revert of the cap fails loudly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: AndresL230 <190146319+AndresL230@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…#352) * fix(rag): content-hash chunk ids — dedup identical uploads per course Rebased onto current main (clean apply; no interaction with the #439 model_mode gates or 0030 extracted_text encryption — course_chunks is plaintext by design for retrieval). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(rag): namespace document chunk ids away from the catalog keyspace (review finding) sha256(course::text) is exactly scripts/ingest_catalog.py's id formula for category=catalog rows in the same table + on_conflict=id upsert — a document chunk byte-matching a catalog chunk would silently overwrite it and flip its category. Ids are now sha256(course::document::text); keyspace-isolation regression test added, and the backfill script migrates legacy rows to the namespaced scheme automatically (it derives ids from rag_service.chunk_id). Also corrected the script docstring's 'last-writer-wins' overstatement (winner is first-with-embedding). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: AndresL230 <190146319+AndresL230@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
… summary (#419) * fix(documents): reject empty text extraction instead of fabricating a summary A rasterized PDF has no text layer, so extraction returns "" without raising. `_extract_text_or_422` only caught exceptions, so the empty string flowed straight into the classify/summarize prompt as `Content: ` -- and because that prompt requires a summary plus a concept list with no "insufficient content" escape hatch, the model invented a document instead of failing. Observed on a CS 132 (linear algebra) practice final: the stored summary described the 1964 Berkeley Free Speech Movement and the extracted concepts were CNNs, RNNs, Transformers, and Attention. Those concepts were persisted and bound for the course knowledge graph, which is shared by every enrolled student -- so one unreadable upload would have seeded neural-network topics into a linear algebra course for the whole class. Docling already detects this (it flags low-char pages in `fallback_pages`), but that signal is only acted on when `OCR_ENGINE=auto`, and nothing downstream checked the text at all. Guard both upload paths against near-empty extraction: - `_extract_text_or_422` now raises 422 (covers /upload/sync, and /upload when OCR_ASYNC_ENABLED is off) - the async-OCR branch inside the SSE stream emits the same terminal error+done pair it already uses for extraction failures, so clients need no new case Threshold is 50 stripped chars, matching the floor `extraction_service._extract_text_from_file_uncached` already applies to native PDF text. Emptiness alone would be too weak: a scanned page often yields a few stray characters (a page number, a watermark), which is still enough to trigger fabrication. Happy-path upload fixtures previously returned strings as short as "t", which the guard correctly rejects. They now go through a `_doc_text()` helper so a fixture is no longer indistinguishable from a failed extraction. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(documents): pin the exact 49/50-char guard boundary Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: AndresL230 <190146319+AndresL230@users.noreply.github.com>
#320) (#371) * fix(topnav): close open dropdown instantly when another tab is hovered (#320) Rebase of PR #371 onto current main (the branch had drifted ~147 commits behind; its true diff touches only TopNav.tsx/TopNav.test.tsx, and main had not modified either file since the merge base, so the 3-way apply was conflict-free). Lift the per-trigger dropdown open-state into a new DesktopGroups row component that owns a single openIndex. Previously each NavGroupTrigger kept its own open flag and 140ms close-timer, so hovering from tab A to tab B cancelled only B's timer — A's panel lingered and two panels could show at once. With one owner, entering any tab replaces openIndex synchronously, closing the old panel instantly; the 140ms close-delay now only applies when the cursor leaves the row entirely. NavGroupTrigger becomes presentational (open/onOpen/onScheduleClose/ onClose props); route-change close, click-outside, and Escape handling move to the row level; blur-out of a trigger wrapper still closes immediately. Adds a regression test: opening Community must immediately close Learn (panel gone, aria-expanded flipped). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(topnav): dismiss on dead-space clicks — 'outside' means outside every trigger wrapper, not the flex:1 row (review finding) The lifted click-outside check guarded on rowRef, which stretches across the header's blank strip; a keyboard-opened panel (no hover timer armed) got stuck after a click there. Clicks now dismiss unless inside a [data-nav-group] wrapper. Adds the dead-space regression test plus a fake-timers test for the actual #320 hover-timer race. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: AndresL230 <190146319+AndresL230@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* fix(calendar): animate view switch instead of snapping (#295) Rebase of PR #373 onto current main (~147 commits ahead of the old base). The PR's true diff applied cleanly on top of main's only intervening Calendar change (the #422 test-mode now() seams), so this is the original change re-landed, not a re-implementation: - Wrap the calendar body (skeleton / Month / Week / Day / Table) in AnimatePresence mode="wait" with a motion.div keyed on the load state and active view, so switching views crossfades/slides (0.22s) instead of snapping — mirroring Study.tsx's pattern. - Respect prefers-reduced-motion via useReducedMotion: no initial/exit offset and zero duration when reduced motion is requested (the global CSS rule only covers CSS transitions, not framer's JS animations). - Add Calendar.test.tsx: framer-motion stubbed to a passthrough; asserts skeleton-then-month load, correct body per view toggle, and no leakage between views. Main's now() determinism seams (cursor, today, Today button, dueLabel) are untouched; no testids changed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(calendar): skip framer animations under NEXT_PUBLIC_TEST_MODE (review finding) Calendar is the third framer-motion consumer but lacked the IS_TEST_MODE -> MotionGlobalConfig.skipAnimations gate Study.tsx and HowItWorks.tsx pair with the import (module-side-effect scoped, so a Playwright run landing directly on /calendar would animate for real in the deterministic lane). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: AndresL230 <190146319+AndresL230@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* fix(calendar): restore Google Calendar OAuth connect flow (#61) Rebase of PR #407 onto current main (ea2ab0b, ~127 commits ahead of the original branch point). The true diff applied cleanly with git apply -3; no textual conflicts. Re-verified every reused primitive against main's evolved auth/encryption structure (0024 identity split). The dedicated calendar consent flow — GET /api/calendar/auth-url and /api/calendar/callback — was dropped in the SQLite→Supabase migration, but config.GOOGLE_SCOPES / GOOGLE_REDIRECT_URI and the frontend "Connect Google" button (Calendar.tsx → calendarAuthUrl) still point at it. With the routes gone, clicking "Connect Google" 404'd, so users could never (re)grant calendar access and /sync, /export, /import all failed with 401 "Not connected to Google Calendar." This is the root cause of #61. - Restore both routes, reusing the sign-in flow's OAuth primitives (PKCE + HMAC-signed state cookie) from routes/auth.py as the single source of truth for CSRF handling — no duplication of the security-critical bits. On current main these helpers still live in routes/auth.py (session minting moved to services/session_tokens.py, but the OAuth state/PKCE helpers did not). - Auth scoping (the #61 comment, sibling of the #123 export IDOR): the user_id is sealed into the signed state cookie after require_self, and the callback reads it from that cookie — never from a request parameter — so the minted tokens can only ever bind to the session that initiated connect. - Request access_type=offline + prompt=consent so a refresh_token is always returned; otherwise sync breaks once the access token expires. - Token storage follows main's encryption boundaries: encrypt(access_token), encrypt_if_present(refresh_token), upsert on_conflict=user_id — byte-for- byte the same shape as the sign-in callback's oauth_tokens write. - Fix a latent refresh bug: _get_refreshed_credentials wrote expires_at="" when a refresh yielded no expiry, but expires_at is TIMESTAMPTZ (migration 0024) and "" is not a valid timestamptz (auth.py fixed the same hazard). - The calendar flow uses GOOGLE_REDIRECT_URI (/api/calendar/callback), kept distinct from the sign-in flow's GOOGLE_AUTH_REDIRECT_URI, via _calendar_client_config re-pointing the shared client config. Tests: new test_calendar_oauth_connect.py covers the happy path, the CSRF boundary (nonce mismatch / missing cookie / user-denied), token binding to the cookie user, and the expires_at=None refresh fix. Full suite green on main's tip: 1231 passed, 27 skipped. ruff check clean (zero findings, same as the origin/main baseline). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(db): drop NOT NULL on oauth_tokens.expires_at — the None-expiry write needs schema backing (review finding) The expires_at=None 'fix' traded an invalid-timestamptz cast for a not-null violation (0001 baseline constraint; 0024 only retyped the column) — a refresh PATCH would 500 AND lose the fresh access_token. Readers already treat NULL as 'no known expiry'. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: AndresL230 <190146319+AndresL230@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…' into fix/staging-deploy-env-hardening-rework
AndresL230
marked this pull request as ready for review
July 29, 2026 10:35
AndresL230
commented
Jul 29, 2026
Collaborator
Code reviewFound 1 issue:
Sapling/frontend/next.config.ts Lines 13 to 20 in ed0839e 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
…ving (review finding) The derivation overwrote BACKEND_URL/NEXT_PUBLIC_API_URL/COOKIE_DOMAIN from DEPLOY_ENV before checkFrontendDeployEnv ran, so the explicit-lock branch (the 'all-staging values on a prod deploy' guard) could never fire and a wrong DEPLOY_ENV silently baked wrong URLs. The check now runs against the operator-provided env first; the post-derive copy is removed. Also: wrangler.toml's ADR pointer updated 0020 -> 0022 (sibling of the middleware fix). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
AndresL230
commented
Jul 29, 2026
Collaborator
The ordering issue from the review above is fixed in the latest push: |
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
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.
Why
Login on
staging.saplinglearn.combounces to/?error=session_expired. This is the footgun documented in ADR 0020 — the frontend's environment config (backend origin + session-cookieDomain) can drift from the environment the worker is actually serving, so staging ends up validating a session cookie signed with the wrongSESSION_SECRETand treats every visitor as logged out.The fix was written on
fix/staging-deploy-envbut never landed onmain(that branch also carries unrelated RAG changes). This PR cherry-picks only the frontend/deploy hardening + the ADR — nothing else.What
Make
DEPLOY_ENVthe single knob that drives every environment-specific value, with the explicit vars kept as backward-compatible fallbacks (deployGuard.resolveFrontendEnv):middleware.ts— derivesAPI_URLviaresolveFrontendEnv, and on a protected route callsdetectHostConfigMismatch(host, apiUrl). A worker serving one env's host while wired to another's backend now logs a loud server error and redirects with a distinctenv_misconfigcode instead of the misleadingsession_expired.app/api/auth/session/route.ts— derives the cookieDomainfromresolveFrontendEnv, so a staging build can't mint a.saplinglearn.comcookie that leaks into prod.next.config.ts— derives the build-timeBACKEND_URL(the/apirewrite) and inlinedNEXT_PUBLIC_API_URL/COOKIE_DOMAINfromDEPLOY_ENV.wrangler.toml—DEPLOY_ENV = "production"in[vars],DEPLOY_ENV = "staging"in[env.staging.vars]; documents the Build-command vs Deploy-command distinction.SignInModal.tsx— user-facing copy forenv_misconfig.Per ADR 0020, the running staging worker still needs a real redeploy:
DEPLOY_ENV=stagingas a build variable on thefrontend-stagingWorkers Build (build command staysnpm run cf:build— never a deploy line).wrangler versions upload, which uploads but doesn't promote — promote it, or switch the deploy command towrangler deploy --env staging).curl -sSI https://staging.saplinglearn.com/dashboard→Location: https://api.staging.saplinglearn.com/api/auth/google.Testing
deployGuard.test.tsextended (132 lines) — runs undernpm teston CI (Node 22).tsc --noEmit✅ andeslint✅ on changed files.resolveFrontendEnvderivation for staging/production/unset, anddetectHostConfigMismatchflagging staging-host→prod-backend while leaving previews/localhost alone. All assertions passed.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Documentation
Tests