Uh oh!
There was an error while loading. Please reload this page.
fix(learn): consume /learn?resume= deep links (#164) + ADR-0020 interrupted-turn Retry + promoted streaming journeys (#356) - #461
Conversation
…rupted-turn Retry, promoted streaming journeys (#356) #164: Learn read only topic/mode/course/suggest off the URL, so Dashboard's 'Where you left off' cards (?resume=) and Tree's session rows (?session=) landed on the session picker — dead buttons. LearnInner now consumes ?resume= (legacy ?session= accepted) once per id via resumeSessionById, which is server-authoritative (topic/mode/course off the resume payload), with a loading state instead of a picker flash. Tree unified on ?resume=. ADR 0020 (#356 item 5): a chat turn that is Stopped or fails mid-stream (Rung 2, or the Rung-3 JSON fallback itself failing) now keeps the partial reply, marks the bubble interrupted, and offers Retry — nothing was persisted, so Retry is a plain re-send (drops the failed pair first). A turn aborted by SWITCHING sessions appends nothing (no stale bubble). #356 items 4/6/7 land as promoted journeys (frontend/e2e/streaming.spec.ts): stream-fails-to-open → transparent JSON fallback; Stop → interrupted + Retry + zero rows persisted; switch-mid-stream → aborted stream, no stale bubble, both sessions' rows untouched. The mid-stream window is real: the function-mode seam now paces streamed replay (set_function_stream_delay_ms, 24-char chunks; the e2e handlers module opts in at 150ms and serves a long deterministic reply on the E2E_SLOW_STREAM trigger). Item 3 (server Rung-1 legacy fallback) cannot run deterministically in the lane by design — its live proof is tests/test_streaming_rung1_live.py (opt-in, billable), its mechanics stay pinned in test_chat_stream.py. #164 gets a dashboard-entry journey in tutor.spec.ts. Verified under both dep universes: local venv (pydantic-ai 1.89) and a lock-pinned scratch venv (1.107.0) — full backend suite 1316 passed; frontend vitest 254 passed, tsc clean, lint 0 errors (suppressions pruned: the new testids removed two suppressed occurrences). Closes#164. Closes#356. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Warning Review limit reached
Next review available in:50 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 (20)
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 | d6ab1f5 | Commit Preview URL Branch Preview URL | Jul 29 2026, 07:28 PM |
AndresL230
commented
Jul 29, 2026
Code reviewFound 2 issues:
Sapling/frontend/e2e/streaming.spec.ts Lines 114 to 124 in 7d76204
Sapling/frontend/src/components/screens/Learn.tsx Lines 357 to 359 in 7d76204 Lower-confidence items (below threshold, noted for the author): the success-path append after the un-abortable Rung-3 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
- Guard the success-path append (and the streamingText clear) with the same sessionIdRef/controller ownership checks as appendInterrupted — the un-abortable Rung-3 sendChat could resolve after a session switch and inject its reply into the other session's transcript. New journey pins it (delayed JSON fallback + switch → reply dropped client-side, persisted server-side to the original session only). - streaming.spec.ts DB readbacks now assert ciphertext-at-rest AND decrypt-equality (support/decrypt.ts: 'Specs must always make both'). - resuming state lazily initialized from the URL so a /learn?resume= deep link's first paint is the loading branch, not a one-frame picker flash. - send()'s three-rung ladder summary comment updated to the ADR-0020 behavior it now implements; beginSession's mirror comment documents the greeting-scope exception. - ADR 0020 'Scope / where it lands' rewritten to match the implemented Status line. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ansform
The e2e lane's two paced mid-stream journeys failed with ZERO tokens for
5s then everything at once: the stack serves the frontend with production
`next start`, whose default compress:true wraps the /api/* rewrite proxy
in the `compression` middleware, and gzip buffers small SSE frames until
end-of-response. Instant function-mode streams passed only because their
single burst IS the whole response — progressive rendering was silently
broken behind every self-hosted `next start` proxy (docker-compose too).
Verified server-side pacing was live (probe: deltas at 0.01..1.37s for a
10-chunk reply) before touching the HTTP layer. Fix: SSE_CACHE_CONTROL
("no-store, no-transform") on every EventSourceResponse — the middleware's
standard filter skips no-transform responses; sse_starlette only
setdefaults Cache-Control so the route value wins. Red-first route tests
pin the header on both learn streams.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
What
Bundle B1 of the backlog clear — the tutor-resume P1 plus the streaming smoke tail, in one PR because they share files and journeys.
#164 — dead resume deep links
Learn.tsxread onlytopic/mode/course/suggestoff the URL; Dashboard's "Where you left off" cards push/learn?resume=<id>and Tree's session rows pushed/learn?session=<id>, so both stranded users on the session picker. Now:readResumeParam(exported, unit-tested) acceptsresume(canonical) andsession(legacy alias); Tree unified on?resume=.resumeSessionById, which is server-authoritative: topic/mode/course come off the resume payload, so deep links work for sessions outside the 10-row recent list.#356 item 5 / ADR 0020 — interrupted turns keep the partial + offer Retry
The merged #349 code still discarded partials on Stop and rendered Rung-2 failures as an
Error:assistant bubble. Per ADR 0020:ChatMsggainsinterrupted/retryText; ChatPanel renders the partial (dimmed, never blanked) + an "Interrupted" marker + Retry.send's ladder: Stop, Rung-2, and a failed Rung-3 JSON fallback all append the interrupted bubble (error detail goes to a toast, not the transcript). Retry drops the failed pair (removeInterruptedTurn, pure + unit-tested) and re-sends — safe because the routes persist only on completion.sessionIdRefguard) — otherwise the fix itself would have introduced the item-7 stale bubble.beginSession) returns to the entry screen with the topic draft intact — there is no transcript turn to mark.#356 items 4/6/7 — promoted journeys (
frontend/e2e/streaming.spec.ts)/api/learn/chat/streamroute-aborted → transparent JSON fallback → deterministic reply + exactly one encrypted user/assistant pair (decrypt-verified).requestfailedobserved), no stale bubble, both sessions' row counts untouched.dashboard-resume-{id}testid) intutor.spec.ts.Mid-stream windows are real: the function-mode seam paces streamed replay (
set_function_stream_delay_ms— 24-char chunks, reset byclear_function_handlers, no new lane env var: the e2e handlers module opts in at import with 150ms) and serves a long deterministic reply on theE2E_SLOW_STREAMmessage trigger. Constants stay byte-identical between the JSON and streamed lanes; handler-constant ↔ spec ↔test_e2e_function_handlers.pysync maintained.#356 item 3 — live evidence instead of a lane journey
The server-side Rung-1 legacy fallback (
call_gemini_multiturn) has no function-mode gate by design (fallback-only, deleted in #151), so it cannot run deterministically in the e2e lane. Newtests/test_streaming_rung1_live.py(opt-inRUN_LIVE_STREAM_RUNGS=1, billable,live_llmlane) proves the real failure shape degrades to a live legacy reply arriving as a single token + done withon_completenever called — run green locally. Mechanics stay pinned hermetically intest_chat_stream.py/test_learn_stream_routes.py.Verification
tsc --noEmitclean, lint 0 errors (suppressions pruned — the two new testids removed suppressed occurrences).🤖 Generated with Claude Code
Closes#164. Closes#356.