Uh oh!
There was an error while loading. Please reload this page.
feat(learn): stream tutor replies over SSE with live graph deltas (#70, #74) - #349
Merged
Conversation
Warning Review limit reached
Next review available in:3 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (21)
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 | 1d2a763 | Commit Preview URL Branch Preview URL | Jul 29 2026, 08:41 AM |
This was referenced Jul 17, 2026
Closed
#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>
AndresL230force-pushed
the
feat/streaming-tutor
branch
from
July 29, 2026 08:18
6f37e57 to
02a8b62CompareAndresL230
marked this pull request as ready for review
July 29, 2026 08:19
…ew 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>
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Jul 29, 2026
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>
AndresL230 added a commit
that referenced
this pull request
Jul 29, 2026
… admin analytics (#120) Rebase of PR #375 (which included stacked #376) onto current main, rebuilt as a fresh application of the branch diff plus the adaptations main now requires. Core (unchanged from #375/#376): - agents/usage.py: record_agent_usage(result, feature=, task=, user_id=) — one-line, never-raising usage capture for every Pydantic AI run. - services/events_service.py: bounded-queue fire-and-forget writer draining events/llm_usage rows through db/connection.table() off the request thread. - services/llm_pricing.py: token-field normalization + per-1K price map; unknown real models record cost_usd=NULL with a one-time warning. - gemini_service: call_gemini / call_gemini_multiturn log via _log_gemini_usage (feature= threaded from callers; json delegates). - routes/admin_analytics.py (+ mount): admin usage/cost rollup endpoints. - tests/test_usage_instrumentation_coverage.py: AST guard — a module that runs an agent without referencing record_agent_usage fails CI. Rebase adaptations: - Migration renumbered 0032_observability.sql -> 0035_observability.sql (main grew 0032-0034 in the meantime); content unchanged, header comment updated. - main.py lifespan: events_service.start_worker()/shutdown() coexists with #406's Logfire wiring (configure/instrument_pydantic_ai/instrument_fastapi). - Conflict resolutions keep main's guardrail semantics and add capture on top: notes.py wraps _run_note_worker results (WORKER_LIMITS + 413/500 mapping intact) and the note_chat try/except (ORCHESTRATOR_LIMITS + degraded reply intact); learn.py wraps the _prepare_chat_run-based _chat_via_agent; calendar_service keeps usage_limits=WORKER_LIMITS. - NEW run-sites landed on main since the branch: * SSE streaming tutor (#349): stream_agent_turn grows an optional on_usage(run_result) hook fed by the final AgentRunResultEvent, called once on the success path before on_complete (tokens are spent even if persistence fails); /chat/stream and /start-session/stream pass record_agent_usage(feature="chat_tutor", task="chat_tutor"). Error rungs and the Rung-1 legacy fallback don't fire it — legacy usage is captured inside call_gemini_multiturn(feature=). Covered in test_chat_stream.py. * gemini_vision_backend: per-page ocr_vision_agent run wrapped (feature="document", task="ocr_vision"; no user_id — extraction is content-addressed and user-agnostic). - SAPLING_MODEL_MODE=function: 'function:<task>' model names record with cost_usd=NULL and NO unpriced-model warning (they are the e2e/CI seam, not real spend); real unknown models keep the one-time warning. Tested. Verification: full backend suite 1275 passed / 27 skipped; ruff clean; AST guard green over all current run-sites. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
AndresL230 added a commit
that referenced
this pull request
Jul 29, 2026
Rework of PR #360 (feat/semester-scoped-learning) replayed onto current main as a single commit, resolving the drift accumulated since the merge-base (~40 commits, incl. the #349 Learn.tsx SSE rewrite, the #355 subject-root dedup, the #352 content-hash chunk ids, and the #140 current/archive dashboard split, which the semester tabs supersede per the branch's own reconciliation). Backend semester scoping: - services/academics.py: term_id_for_label (semester label -> term id, with term-id fallback) + user_course_ids_for_term (enrollments -> offerings -> term-filtered course ids). - services/graph_service.py: get_graph/get_recommendations accept an optional semester label; nodes/edges/stats and the synthesized subject roots are restricted to that term's courses (composes with the #355 per-course subject-root dedup — roots are built from the already- filtered enrollment list). - add_course: no-retake rule across ALL terms (returns already_existed with the existing term label) and an optional term label so the hub enrolls into the tab being viewed, falling back to the current term. - routes/graph.py: optional ?semester= on GET /api/graph/{user_id} and /recommendations; AddCourseBody gains term. - routes/gradebook.py: private _term_id_for_semester duplicate removed; call sites now use the shared academics.term_id_for_label (follow-up noted on the original PR). Frontend semester scoping: - lib/useActiveSemester.ts: localStorage-backed active-semester hook (cross-tab sync, hydration flag so first fetches are scoped once). - Dashboard/Tree/Learn/Quiz/Study fetch the graph scoped to the active semester and scope course/concept pickers to it; Learn's scoping was redone by hand against main's SSE-streaming Learn.tsx (pass the active semester into the bootstrap getGraph, term on concepts, scopedCourses for the course select, TopicPicker term filter — main's own suggest/highlight logic kept, not duplicated). - ManageCoursesModal becomes the Courses & Semesters hub (per-term tabs, enroll-into-tab, no-retake feedback); the Dashboard archive rail from #140 is replaced by the active-semester scoping. - vitest.setup.ts: install an in-memory localStorage when the jsdom environment exposes none (jsdom 29 under Vitest leaves window.localStorage undefined), so useActiveSemester/useLayoutPref are testable in DOM tests. Per-type document chunking + tutor integrity: - services/chunker.py: chunk_for_category routes prose-like categories (essays/assignments) to a sentence-aware prose chunker, others to the existing chunker; routes/documents.py passes the doc category through; backfill script follows. Composes with #352's content-addressed chunk ids (sha256 over chunk text — only boundaries change). - prompts/preamble.txt: academic-integrity rule for the tutor (guide, never hand over graded-work answers). Conflict resolutions: test_document_indexing.py keeps both main's #439 relevance-gate test (repointed at the chunk_for_category seam) and the branch's category-passthrough test; Dashboard.tsx keeps main's IS_TEST_MODE/now + #369 learnHrefForNode imports alongside courseTermLabels; eslint-suppressions.json re-baselined (Dashboard no-restricted-syntax count drops with the removed archive UI). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
AndresL230 added a commit
that referenced
this pull request
Jul 29, 2026
* feat(observability): capture LLM token usage + cost per call (#118) + admin analytics (#120) Rebase of PR #375 (which included stacked #376) onto current main, rebuilt as a fresh application of the branch diff plus the adaptations main now requires. Core (unchanged from #375/#376): - agents/usage.py: record_agent_usage(result, feature=, task=, user_id=) — one-line, never-raising usage capture for every Pydantic AI run. - services/events_service.py: bounded-queue fire-and-forget writer draining events/llm_usage rows through db/connection.table() off the request thread. - services/llm_pricing.py: token-field normalization + per-1K price map; unknown real models record cost_usd=NULL with a one-time warning. - gemini_service: call_gemini / call_gemini_multiturn log via _log_gemini_usage (feature= threaded from callers; json delegates). - routes/admin_analytics.py (+ mount): admin usage/cost rollup endpoints. - tests/test_usage_instrumentation_coverage.py: AST guard — a module that runs an agent without referencing record_agent_usage fails CI. Rebase adaptations: - Migration renumbered 0032_observability.sql -> 0035_observability.sql (main grew 0032-0034 in the meantime); content unchanged, header comment updated. - main.py lifespan: events_service.start_worker()/shutdown() coexists with #406's Logfire wiring (configure/instrument_pydantic_ai/instrument_fastapi). - Conflict resolutions keep main's guardrail semantics and add capture on top: notes.py wraps _run_note_worker results (WORKER_LIMITS + 413/500 mapping intact) and the note_chat try/except (ORCHESTRATOR_LIMITS + degraded reply intact); learn.py wraps the _prepare_chat_run-based _chat_via_agent; calendar_service keeps usage_limits=WORKER_LIMITS. - NEW run-sites landed on main since the branch: * SSE streaming tutor (#349): stream_agent_turn grows an optional on_usage(run_result) hook fed by the final AgentRunResultEvent, called once on the success path before on_complete (tokens are spent even if persistence fails); /chat/stream and /start-session/stream pass record_agent_usage(feature="chat_tutor", task="chat_tutor"). Error rungs and the Rung-1 legacy fallback don't fire it — legacy usage is captured inside call_gemini_multiturn(feature=). Covered in test_chat_stream.py. * gemini_vision_backend: per-page ocr_vision_agent run wrapped (feature="document", task="ocr_vision"; no user_id — extraction is content-addressed and user-agnostic). - SAPLING_MODEL_MODE=function: 'function:<task>' model names record with cost_usd=NULL and NO unpriced-model warning (they are the e2e/CI seam, not real spend); real unknown models keep the one-time warning. Tested. Verification: full backend suite 1275 passed / 27 skipped; ruff clean; AST guard green over all current run-sites. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(observability): apply #375 review fixes — range validation, truncation flag, private caching, poison-row salvage, per-call-site usage guard - admin_analytics _resolve_range: validate from/to as ISO 8601 (422 naming the bad param) and reject from > to; validated strings echoed unchanged. - Surface the 100k _SCAN_CAP: _scan_range returns (rows, truncated) and UsageSummary/UsageByUser/LLMCost carry `truncated: bool = False` (the /errors feed paginates server-side, so it has no cap to surface). - All 4 analytics GETs now send `Cache-Control: private` via a Response param. - test_default_range_is_last_30_days freezes the module clock at 2026-07-21 so the fixture window can't rot after 2026-08-09. - events_service._flush_batch: a failed bulk insert now retries rows one at a time, dropping only the rows that individually fail (per-row debug log + one warning with the drop count); never-raise contract kept, unit-tested with a 3-row batch where only the poison row is lost. - test_usage_instrumentation_coverage: upgraded the file-level substring guard to a real per-call-site AST check — every agent-run site needs an enclosing record_agent_usage, with pass-through runner helpers (return await agent.run(...)) checked at their module-local callers instead. The docstring now states the exact granularity and the cross-module blind spot. 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>
AndresL230 added a commit
that referenced
this pull request
Jul 29, 2026
Rework of PR #360 (feat/semester-scoped-learning) replayed onto current main as a single commit, resolving the drift accumulated since the merge-base (~40 commits, incl. the #349 Learn.tsx SSE rewrite, the #355 subject-root dedup, the #352 content-hash chunk ids, and the #140 current/archive dashboard split, which the semester tabs supersede per the branch's own reconciliation). Backend semester scoping: - services/academics.py: term_id_for_label (semester label -> term id, with term-id fallback) + user_course_ids_for_term (enrollments -> offerings -> term-filtered course ids). - services/graph_service.py: get_graph/get_recommendations accept an optional semester label; nodes/edges/stats and the synthesized subject roots are restricted to that term's courses (composes with the #355 per-course subject-root dedup — roots are built from the already- filtered enrollment list). - add_course: no-retake rule across ALL terms (returns already_existed with the existing term label) and an optional term label so the hub enrolls into the tab being viewed, falling back to the current term. - routes/graph.py: optional ?semester= on GET /api/graph/{user_id} and /recommendations; AddCourseBody gains term. - routes/gradebook.py: private _term_id_for_semester duplicate removed; call sites now use the shared academics.term_id_for_label (follow-up noted on the original PR). Frontend semester scoping: - lib/useActiveSemester.ts: localStorage-backed active-semester hook (cross-tab sync, hydration flag so first fetches are scoped once). - Dashboard/Tree/Learn/Quiz/Study fetch the graph scoped to the active semester and scope course/concept pickers to it; Learn's scoping was redone by hand against main's SSE-streaming Learn.tsx (pass the active semester into the bootstrap getGraph, term on concepts, scopedCourses for the course select, TopicPicker term filter — main's own suggest/highlight logic kept, not duplicated). - ManageCoursesModal becomes the Courses & Semesters hub (per-term tabs, enroll-into-tab, no-retake feedback); the Dashboard archive rail from #140 is replaced by the active-semester scoping. - vitest.setup.ts: install an in-memory localStorage when the jsdom environment exposes none (jsdom 29 under Vitest leaves window.localStorage undefined), so useActiveSemester/useLayoutPref are testable in DOM tests. Per-type document chunking + tutor integrity: - services/chunker.py: chunk_for_category routes prose-like categories (essays/assignments) to a sentence-aware prose chunker, others to the existing chunker; routes/documents.py passes the doc category through; backfill script follows. Composes with #352's content-addressed chunk ids (sha256 over chunk text — only boundaries change). - prompts/preamble.txt: academic-integrity rule for the tutor (guide, never hand over graded-work answers). Conflict resolutions: test_document_indexing.py keeps both main's #439 relevance-gate test (repointed at the chunk_for_category seam) and the branch's category-passthrough test; Dashboard.tsx keeps main's IS_TEST_MODE/now + #369 learnHrefForNode imports alongside courseTermLabels; eslint-suppressions.json re-baselined (Dashboard no-restricted-syntax count drops with the removed archive UI). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
AndresL230 added a commit
that referenced
this pull request
Jul 29, 2026
* feat: semester-scoped learning + Courses & Semesters hub (#360, rebased) Rework of PR #360 (feat/semester-scoped-learning) replayed onto current main as a single commit, resolving the drift accumulated since the merge-base (~40 commits, incl. the #349 Learn.tsx SSE rewrite, the #355 subject-root dedup, the #352 content-hash chunk ids, and the #140 current/archive dashboard split, which the semester tabs supersede per the branch's own reconciliation). Backend semester scoping: - services/academics.py: term_id_for_label (semester label -> term id, with term-id fallback) + user_course_ids_for_term (enrollments -> offerings -> term-filtered course ids). - services/graph_service.py: get_graph/get_recommendations accept an optional semester label; nodes/edges/stats and the synthesized subject roots are restricted to that term's courses (composes with the #355 per-course subject-root dedup — roots are built from the already- filtered enrollment list). - add_course: no-retake rule across ALL terms (returns already_existed with the existing term label) and an optional term label so the hub enrolls into the tab being viewed, falling back to the current term. - routes/graph.py: optional ?semester= on GET /api/graph/{user_id} and /recommendations; AddCourseBody gains term. - routes/gradebook.py: private _term_id_for_semester duplicate removed; call sites now use the shared academics.term_id_for_label (follow-up noted on the original PR). Frontend semester scoping: - lib/useActiveSemester.ts: localStorage-backed active-semester hook (cross-tab sync, hydration flag so first fetches are scoped once). - Dashboard/Tree/Learn/Quiz/Study fetch the graph scoped to the active semester and scope course/concept pickers to it; Learn's scoping was redone by hand against main's SSE-streaming Learn.tsx (pass the active semester into the bootstrap getGraph, term on concepts, scopedCourses for the course select, TopicPicker term filter — main's own suggest/highlight logic kept, not duplicated). - ManageCoursesModal becomes the Courses & Semesters hub (per-term tabs, enroll-into-tab, no-retake feedback); the Dashboard archive rail from #140 is replaced by the active-semester scoping. - vitest.setup.ts: install an in-memory localStorage when the jsdom environment exposes none (jsdom 29 under Vitest leaves window.localStorage undefined), so useActiveSemester/useLayoutPref are testable in DOM tests. Per-type document chunking + tutor integrity: - services/chunker.py: chunk_for_category routes prose-like categories (essays/assignments) to a sentence-aware prose chunker, others to the existing chunker; routes/documents.py passes the doc category through; backfill script follows. Composes with #352's content-addressed chunk ids (sha256 over chunk text — only boundaries change). - prompts/preamble.txt: academic-integrity rule for the tutor (guide, never hand over graded-work answers). Conflict resolutions: test_document_indexing.py keeps both main's #439 relevance-gate test (repointed at the chunk_for_category seam) and the branch's category-passthrough test; Dashboard.tsx keeps main's IS_TEST_MODE/now + #369 learnHrefForNode imports alongside courseTermLabels; eslint-suppressions.json re-baselined (Dashboard no-restricted-syntax count drops with the removed archive UI). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(360): review fixes — race-safe add_course, honest add toasts, single scoped dashboard fetch Review follow-ups on the semester-scoped learning rework (#360): - add_course race (A): new partial unique index (migration 0036) closes the NULL-section gap in course_offerings_unique; academics.resolve_offering (create=True) now catches the 409 from a lost create race and re-selects the winner's offering; the ManageCoursesModal Add button disables while an add request is in flight. The now-unreachable per-offering already-enrolled check in add_course is removed and the single return contract documented. - add toast honesty (B): handleAdd reads the response — already_existed shows an informational "Already taken in <term>" toast instead of a false success; success only toasts when a row was created. - CoursesKey empty state (C): "Nothing enrolled this semester." is reachable — the key stays rendered when courses exist but the active semester scopes to none; still null when there are no courses at all. - dashboard first load (D): with no stored semester, resolve + persist the default (courses + semesters fetch) BEFORE the scoped fetch and early-return; the effect re-run performs the single scoped fetch. Zero-courses stays a single unscoped pass. - cross-screen default (E): new useActiveSemester.ensureDefaultActiveSemester (termLabels) persists a default when none is stored, called from Learn/Quiz/Tree/Study once course term labels are in hand. - prune (F): drop orphaned partitionCurrentAndArchive, groupCoursesByTerm, UNKNOWN_TERM_LABEL, CoursePartition (+ TermGroup) and their test blocks. Tests: resolve_offering conflict-retry + non-409 propagation units; add_course duplicate-path contract; ManageCoursesModal component tests for the already-existed/success/in-flight paths; Dashboard single-scoped-fetch, zero-courses and CoursesKey empty-state renders; ensureDefaultActiveSemester units. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * rework(360): default = All semesters — semester scoping is opt-in (e2e veto) The e2e lane vetoed the auto-default: the rich seed spans Fall 2025 / Spring 2026 / Summer 2026 and 7 of 11 journeys failed because the auto-resolved term silently hid cross-term fixtures (dashboard lost MATH210; the graph shrank to 1 of 17 nodes). New semantics: - Default = ALL SEMESTERS (unscoped). An empty stored active-semester value IS the default and means "All semesters": removed ensureDefaultActiveSemester + its Learn/Quiz/Tree/Study call sites, Dashboard's default-resolution early-return pass (deferredToScopedPass) and its getSemesters/courseTermLabels/resolveActiveSemester usage. resolveActiveSemester itself is deleted (orphaned — the hub now reads the stored value raw). Hook, change events, and hydration gating stay. - Hub UI: ManageCoursesModal grows an explicit "All semesters" tab — active when the stored value is empty, clicking it clears the value; term tabs unchanged (both carry aria-pressed). A picked term still persists and scopes every surface. - Kept from the review batch: CoursesKey empty-state reachability, the already_existed info toast, in-flight Add disable, the 0036 partial unique index + resolve_offering conflict retry, semesters.ts pruning. - New journey frontend/e2e/semester-scope.spec.ts: default shows cross-term courses together (MATH210 + BIO110), picking Fall 2025 in the hub hides Spring-only MATH210, "All semesters" restores it. Hub opens via new dashboard-courses-manage testid (documented in docs/frontend-testids.md); tabs selected by role/name. - Tests reworked: Dashboard default test now asserts one unscoped fetch with both terms visible; /api/semesters-failure test dropped (no semesters fetch remains); ensureDefault unit tests replaced by hub tab component tests (All active by default / term persists / All clears). - Design doc amendment records the veto and the opt-in scoping decision. 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>
AndresL230 added a commit
that referenced
this pull request
Jul 29, 2026
…c-ai 2.x broke every main e2e run since #349 The e2e lane freshly resolved pydantic-ai-slim>=0.0.20 to the 2.x major on every run; 2.x's run_stream_events() returns an async context manager, so async-for raises TypeError pre-token, every streamed tutor turn fell to the legacy fallback (orphan user row + dummy-key failure), and the tutor journey failed 7-rows-vs-6 on every push to main — while ci.yml's lock-pinned backend lane (1.107) stayed green. - e2e.yml now installs --require-hashes -r requirements.lock (pip cache keyed on the lock), matching ci.yml. - requirements.txt pins pydantic-ai-slim <2; bump only with a chat_stream.py migration. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
AndresL230 added a commit
that referenced
this pull request
Jul 29, 2026
…c-ai 2.x broke every main e2e run since #349 (#459) The e2e lane freshly resolved pydantic-ai-slim>=0.0.20 to the 2.x major on every run; 2.x's run_stream_events() returns an async context manager, so async-for raises TypeError pre-token, every streamed tutor turn fell to the legacy fallback (orphan user row + dummy-key failure), and the tutor journey failed 7-rows-vs-6 on every push to main — while ci.yml's lock-pinned backend lane (1.107) stayed green. - e2e.yml now installs --require-hashes -r requirements.lock (pip cache keyed on the lock), matching ci.yml. - requirements.txt pins pydantic-ai-slim <2; bump only with a chat_stream.py migration. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 29, 2026
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.
Summary
Streams tutor replies over SSE with live graph deltas (#70, #74). Rebased 2026-07-29 onto current main as a single rework commit (the branch had drifted 148 commits).
What lands:
services/chat_stream.py—stream_agent_turn, the single streaming seam:status:start → token* → (progress:<tool> → graph_update)* → done, with the rung ladder (Rung 1 pre-token failure → route's legacy fallback; Rung 2 mid-stream → terminal error event, no silent re-run;on_completeXORlegacy_fallback— persistence happens exactly once).services/agent_events.py— chat event vocabulary (token/graph_update/done) extending the ADR-0006 document-pipeline shapes.POST /api/learn/chat/stream+POST /api/learn/start-session/stream(EventSourceResponse,X-Request-IDecho); JSON routes unchanged as fallbacks. start-session's agent-path migration closes ADR-0015's TODO on the streamed lane.sse.tsparser,api.tsstream consumers,Learn.tsx/ChatPanellive-token rendering + Stop button (tutor-stoptestid),applyGraphDeltafor in-stream graph updates._function_model_forgained astream_functionthat replays the registered handler'sModelResponseas deltas — E2EE2E_*constants stay byte-identical across JSON and SSE lanes (2 new seam tests).docs/decisions/0020-streaming-tutor-interrupt-retry.md.Dropped from the original branch (superseded by
_LoopSafeGoogleModel, #453):fresh_google_model/fresh_model_for,_fresh_stream_model, all per-callmodel=overrides — the streamed routes now inherit the agent's own model, which also keeps theSAPLING_MODEL_MODEseam in the streaming path.Testing
🤖 Generated with Claude Code