diff --git a/backend/agents/_providers.py b/backend/agents/_providers.py index 71c9b61f..23212cd3 100644 --- a/backend/agents/_providers.py +++ b/backend/agents/_providers.py @@ -280,6 +280,31 @@ async def __aexit__(self, exc_type, exc_val, exc_tb): ] _FUNCTION_HANDLERS: dict[str, FunctionModelHandler] = {} +# Streamed-replay pacing (#356). 0 = off: text parts replay as one whole +# delta each, exactly the pre-pacing behavior. When a handlers module opts +# in (set_function_stream_delay_ms), _stream_dispatch re-chunks text into +# _FUNCTION_STREAM_CHUNK_CHARS-sized deltas with this delay between them, so +# browser journeys have a real mid-stream window to act in (Stop a turn, +# switch sessions). Only the streamed lane paces; the JSON lane and the +# joined stream text stay byte-identical to the handler's constant. +_FUNCTION_STREAM_DELAY_MS = 0.0 +_FUNCTION_STREAM_CHUNK_CHARS = 24 + + +def set_function_stream_delay_ms(ms: float) -> None: + """Opt the function-mode STREAMED replay into pacing (see the comment on + _FUNCTION_STREAM_DELAY_MS above). Called by boot-time handler modules + (agents/function_handlers_e2e.py) rather than wired to another lane env + var; `clear_function_handlers()` resets it so in-process tests always + start unpaced.""" + global _FUNCTION_STREAM_DELAY_MS + _FUNCTION_STREAM_DELAY_MS = max(0.0, float(ms)) + + +def function_stream_delay_ms() -> float: + """Public read of the streamed-replay pacing knob (tests pin it).""" + return _FUNCTION_STREAM_DELAY_MS + class UnregisteredHandlerError(LookupError): """Raised by the function-mode dispatch when a task has no registered @@ -326,9 +351,12 @@ def register_function_handler(task: AgentTask, handler: FunctionModelHandler) -> def clear_function_handlers() -> None: - """Drop all registered handlers. Tests call this around each case so - process-global registrations never leak between them.""" + """Drop all registered handlers and reset the streamed-replay pacing + knob. Tests call this around each case so process-global registrations + (and a handlers module's pacing opt-in) never leak between them.""" + global _FUNCTION_STREAM_DELAY_MS _FUNCTION_HANDLERS.clear() + _FUNCTION_STREAM_DELAY_MS = 0.0 # ── Boot-time handler registration for out-of-process runs (#392) ────────── @@ -401,12 +429,32 @@ async def _stream_dispatch(messages, info): # as one delta, each tool call as a DeltaToolCall — so a task's handler # constants stay byte-identical between the JSON and streamed lanes # (spec assertions compare against the same E2E_* constants either way). + # + # Pacing (#356): when set_function_stream_delay_ms opted this process + # in, text is re-chunked into small deltas with a sleep between them — + # a real mid-stream window for browser journeys. The delay is read + # AFTER _resolve_handler(), which is what imports a boot-time handlers + # module that sets the knob, so the very first paced stream paces. response = _resolve_handler()(messages, info) + delay_s = _FUNCTION_STREAM_DELAY_MS / 1000.0 + emitted_text = False for index, part in enumerate(response.parts): kind = getattr(part, "part_kind", "") if kind == "text": - if part.content: - yield part.content + if not part.content: + continue + if delay_s > 0: + chunks = [ + part.content[i : i + _FUNCTION_STREAM_CHUNK_CHARS] + for i in range(0, len(part.content), _FUNCTION_STREAM_CHUNK_CHARS) + ] + else: + chunks = [part.content] + for chunk in chunks: + if emitted_text and delay_s > 0: + await asyncio.sleep(delay_s) + emitted_text = True + yield chunk elif kind == "tool-call": args = part.args json_args = args if isinstance(args, str) else json.dumps(args or {}) diff --git a/backend/agents/function_handlers_e2e.py b/backend/agents/function_handlers_e2e.py index 62251903..49292110 100644 --- a/backend/agents/function_handlers_e2e.py +++ b/backend/agents/function_handlers_e2e.py @@ -31,7 +31,21 @@ from pydantic_ai.messages import ModelResponse, TextPart, ToolCallPart -from agents._providers import FunctionModelHandler, register_function_handler +from agents._providers import ( + FunctionModelHandler, + register_function_handler, + set_function_stream_delay_ms, +) + +# Streamed-replay pacing (#356): re-chunk streamed text into small deltas with +# 150ms between them, giving the mid-stream journeys (Stop a turn, switch +# sessions while streaming — frontend/e2e/streaming.spec.ts) a real window to +# act in. Import-time is the right moment: this module only loads in the E2E +# lane, and the seam reads the knob after resolving the handler, so even the +# first stream paces. Replies are unchanged byte-for-byte — pacing only slices +# HOW the same constant streams. The seam tests' clear_function_handlers() +# resets the knob, so in-process pytest runs stay unpaced. +set_function_stream_delay_ms(150) # Asserted verbatim by frontend/e2e/tutor.spec.ts (rendered reply + decrypted # messages.content readback). Keep the two literals in sync. @@ -40,8 +54,53 @@ "needs a base case so it can stop calling itself." ) +# Slow lane for mid-stream journeys (#356). A tutor message carrying the +# trigger substring streams this LONG reply instead — ~1000 chars ≈ 40+ paced +# chunks ≈ a 6-second window to press Stop or switch sessions inside. +# Asserted verbatim by frontend/e2e/streaming.spec.ts (including the final +# sentence as the completion sentinel). Keep the literals in sync. +E2E_SLOW_STREAM_TRIGGER = "E2E_SLOW_STREAM" +E2E_TUTOR_SLOW_REPLY = ( + "[e2e-function-model] Deterministic SLOW tutor reply for mid-stream " + "journeys. Recursion solves a problem by reducing it to a smaller copy " + "of itself, and every recursive function needs two ingredients: a base " + "case that stops the descent, and a recursive step that makes real " + "progress toward that base case on every call. Picture the call stack " + "as a tower of postponed promises: each frame waits for the smaller " + "problem beneath it to resolve before it can finish its own work. When " + "the base case finally answers, the tower unwinds in reverse order and " + "every waiting frame completes with the value it was promised. If the " + "recursive step ever fails to shrink the problem, the tower grows " + "without bound until the runtime refuses to add another frame and the " + "program crashes with a stack overflow. That is the whole discipline in " + "one sentence: shrink toward a base case you are certain to reach. This " + "is the final sentence of the slow deterministic reply." +) + + +def _last_user_prompt_text(messages) -> str: + """The most recent user-prompt text in a pydantic-ai message history. + + Used only for trigger sniffing, so it is deliberately tolerant: content + may be a plain string or a sequence mixing strings with binary parts + (pydantic-ai allows both); non-string members are ignored.""" + for message in reversed(messages): + for part in reversed(getattr(message, "parts", None) or []): + if getattr(part, "part_kind", "") != "user-prompt": + continue + content = getattr(part, "content", "") + if isinstance(content, str): + return content + try: + return " ".join(c for c in content if isinstance(c, str)) + except TypeError: + return "" + return "" + def _chat_tutor_handler(messages, info) -> ModelResponse: + if E2E_SLOW_STREAM_TRIGGER in _last_user_prompt_text(messages): + return ModelResponse(parts=[TextPart(content=E2E_TUTOR_SLOW_REPLY)]) return ModelResponse(parts=[TextPart(content=E2E_TUTOR_REPLY)]) diff --git a/backend/routes/documents.py b/backend/routes/documents.py index c8f15cb1..c8647e85 100644 --- a/backend/routes/documents.py +++ b/backend/routes/documents.py @@ -39,7 +39,7 @@ from services.graph_service import apply_graph_update from services.course_context_service import update_course_context from services.achievement_service import check_achievements -from services.agent_events import SaplingEvent, sapling_event_to_sse +from services.agent_events import SSE_CACHE_CONTROL, SaplingEvent, sapling_event_to_sse from services.request_context import current_request_id from agents import WORKER_LIMITS from agents._providers import model_mode @@ -1053,7 +1053,9 @@ async def event_stream(): ): yield sse_event - return EventSourceResponse(event_stream()) + return EventSourceResponse( + event_stream(), headers={"Cache-Control": SSE_CACHE_CONTROL} + ) async def _stream_legacy_fallback( diff --git a/backend/routes/learn.py b/backend/routes/learn.py index e382b779..5cb08523 100644 --- a/backend/routes/learn.py +++ b/backend/routes/learn.py @@ -19,7 +19,7 @@ from db.connection import table from services.academics import offering_course_id, resolve_offering from models import StartSessionBody, ChatBody, EndSessionBody, ActionBody, ModeSwitchBody, RenameSessionBody -from services.agent_events import sapling_event_to_sse +from services.agent_events import SSE_CACHE_CONTROL, sapling_event_to_sse from services.auth_guard import require_self, get_session_user_id from services.chat_stream import merge_graph_updates, stream_agent_turn from services.encryption import encrypt_if_present, encrypt_json, decrypt_if_present, decrypt_json @@ -869,7 +869,8 @@ async def event_stream(): yield sapling_event_to_sse(ev) return EventSourceResponse( - event_stream(), headers={"X-Request-ID": request_id} + event_stream(), + headers={"X-Request-ID": request_id, "Cache-Control": SSE_CACHE_CONTROL}, ) @@ -975,7 +976,8 @@ async def event_stream(): yield sapling_event_to_sse(ev) return EventSourceResponse( - event_stream(), headers={"X-Request-ID": request_id} + event_stream(), + headers={"X-Request-ID": request_id, "Cache-Control": SSE_CACHE_CONTROL}, ) diff --git a/backend/services/agent_events.py b/backend/services/agent_events.py index bbb2675b..67e7da8a 100644 --- a/backend/services/agent_events.py +++ b/backend/services/agent_events.py @@ -110,3 +110,17 @@ def sapling_event_to_sse(event: SaplingEvent) -> dict[str, str]: JSON-encoded full payload, so the frontend can switch on type and still read the full structured event.""" return {"event": event.type, "data": event.model_dump_json()} + + +# Every EventSourceResponse must pass this as its Cache-Control (#356). +# `no-transform` opts the stream out of intermediary compression: the +# frontend proxies /api/* through Next's node server (`next start` — the e2e +# stack and any self-hosted deploy), which wraps responses in the +# `compression` middleware unless config.compress is false. gzip BUFFERS +# small SSE frames, so a paced token stream reaches the browser as one +# burst at end-of-response — progressive rendering silently broken. The +# middleware's standard filter skips responses whose Cache-Control contains +# `no-transform`. `no-store` preserves sse_starlette's own default caching +# posture (it only ever `setdefault`s Cache-Control, so a route-supplied +# value replaces it entirely). +SSE_CACHE_CONTROL = "no-store, no-transform" diff --git a/backend/tests/test_e2e_function_handlers.py b/backend/tests/test_e2e_function_handlers.py index 760d55ac..4b9c3d60 100644 --- a/backend/tests/test_e2e_function_handlers.py +++ b/backend/tests/test_e2e_function_handlers.py @@ -325,3 +325,65 @@ def test_concept_describe_handler_passes_real_output_schema(monkeypatch): assert result.output.description == E2E_CONCEPT_DESCRIPTION assert len(E2E_CONCEPT_DESCRIPTION) <= 400 + + +# ── Slow-stream trigger + lane pacing (#356) ────────────────────────────── +# +# frontend/e2e/streaming.spec.ts needs a mid-stream window to press Stop / +# switch sessions inside. The env module (a) sets the streamed-replay pacing +# knob at import, and (b) serves a LONG deterministic reply when the user +# message carries E2E_SLOW_STREAM_TRIGGER — giving those journeys several +# seconds of real streaming. The default (no trigger) reply must stay +# E2E_TUTOR_REPLY byte-for-byte: tutor.spec.ts asserts it verbatim. + + +def test_env_module_slow_trigger_returns_slow_reply(monkeypatch): + """A tutor turn whose message carries the trigger gets the long slow-lane + constant — through the real agent wiring via the env-autoloaded module.""" + monkeypatch.setenv("SAPLING_MODEL_MODE", "function") + monkeypatch.setenv( + "SAPLING_FUNCTION_HANDLERS", "agents.function_handlers_e2e" + ) + + with socratic_agent.override(model=model_for("chat_tutor")): + result = socratic_agent.run_sync( + "Walk me through this E2E_SLOW_STREAM please", deps=_deps() + ) + + from agents.function_handlers_e2e import E2E_TUTOR_SLOW_REPLY + + assert result.output == E2E_TUTOR_SLOW_REPLY + + +def test_env_module_default_reply_unchanged_by_trigger_support(monkeypatch): + """Regression guard: a normal message (no trigger) still gets the fixed + E2E_TUTOR_REPLY — the slow lane must never hijack the default journey.""" + monkeypatch.setenv("SAPLING_MODEL_MODE", "function") + monkeypatch.setenv( + "SAPLING_FUNCTION_HANDLERS", "agents.function_handlers_e2e" + ) + + with socratic_agent.override(model=model_for("chat_tutor")): + result = socratic_agent.run_sync("What is recursion?", deps=_deps()) + + from agents.function_handlers_e2e import E2E_TUTOR_REPLY + + assert result.output == E2E_TUTOR_REPLY + + +def test_env_module_import_sets_stream_pacing(monkeypatch): + """Importing the module opts the lane into streamed-replay pacing (150ms + between chunked deltas) so mid-stream journeys have a window to act in. + In-process tests stay unpaced: the autouse registry reset + (clear_function_handlers) zeroes the knob again after each case.""" + monkeypatch.setenv("SAPLING_MODEL_MODE", "function") + monkeypatch.setenv( + "SAPLING_FUNCTION_HANDLERS", "agents.function_handlers_e2e" + ) + + with socratic_agent.override(model=model_for("chat_tutor")): + socratic_agent.run_sync("What is recursion?", deps=_deps()) + + from agents._providers import function_stream_delay_ms + + assert function_stream_delay_ms() == 150 diff --git a/backend/tests/test_learn_stream_routes.py b/backend/tests/test_learn_stream_routes.py index 01e239b9..0dc2b326 100644 --- a/backend/tests/test_learn_stream_routes.py +++ b/backend/tests/test_learn_stream_routes.py @@ -196,3 +196,53 @@ async def fake_stream(**kwargs): assert stashed["assistant_reply"] == "Legacy greeting" assert stashed["topic"] == "Eigenvalues" assert "Legacy greeting" in r.text, "the legacy reply must reach the client as a token/done" + + +class TestSseCompressionOptOut: + """SSE responses must carry `Cache-Control: no-transform` (#356 journeys). + + The e2e stack (and any self-hosted `next start`) proxies /api/* through + Next's production server, which wraps responses in the `compression` + middleware unless config.compress is false. gzip BUFFERS small SSE + frames: a paced token stream produced nothing client-side for its whole + duration and arrived as one burst at `done` — progressive rendering + silently broken behind that proxy. `no-transform` is the standard + opt-out the middleware honors; sse_starlette only `setdefault`s its + own Cache-Control, so the route's value must win. + """ + + def test_chat_stream_sets_no_transform(self): + async def fake_stream(**kwargs): + from services.agent_events import SaplingEvent + yield SaplingEvent(type="done", step="reply", message="Complete.", + data={"reply": "Hi", "graph_update": {}, "mastery_changes": []}) + + with patch("routes.learn.stream_agent_turn", fake_stream), \ + patch("routes.learn._prepare_chat_run", + return_value=(MagicMock(), "msg", {}, MagicMock())), \ + patch("routes.learn._consume_pending"), \ + patch("routes.learn._get_session_offering_id", return_value="off-1"), \ + patch("routes.learn.offering_course_id", return_value="c1"), \ + patch("routes.learn._load_message_history", return_value=[]): + r = client.post("/api/learn/chat/stream", json={ + "session_id": "s1", "user_id": "u1", "message": "hello", "mode": "socratic", + }) + assert r.status_code == 200 + assert "no-transform" in r.headers.get("cache-control", "") + + def test_start_session_stream_sets_no_transform(self): + async def fake_stream(**kwargs): + from services.agent_events import SaplingEvent + yield SaplingEvent(type="done", step="reply", message="Complete.", + data={"reply": "Hello", "session_id": "s-new"}) + + with patch("routes.learn.stream_agent_turn", fake_stream), \ + patch("routes.learn._prepare_chat_run", + return_value=(MagicMock(), "msg", {}, MagicMock())), \ + patch("routes.learn._get_course_id_for_topic", return_value=""), \ + patch("routes.learn.resolve_offering", return_value=""): + r = client.post("/api/learn/start-session/stream", json={ + "user_id": "u1", "topic": "Recursion", "mode": "socratic", + }) + assert r.status_code == 200 + assert "no-transform" in r.headers.get("cache-control", "") diff --git a/backend/tests/test_model_mode_seam.py b/backend/tests/test_model_mode_seam.py index b6187768..7ef4b6b7 100644 --- a/backend/tests/test_model_mode_seam.py +++ b/backend/tests/test_model_mode_seam.py @@ -402,3 +402,87 @@ async def run() -> tuple[str | None, list[str]]: assert received == {"query": "eigenvalues", "limit": 2} assert final == "used the tool" assert calls["n"] == 2 + + +# ── Streamed-replay pacing (#356) ───────────────────────────────────────── +# +# Browser journeys that act MID-STREAM (Stop a turn, switch sessions while +# streaming) need a real time window between deltas — a FunctionModel that +# replays its whole reply in one instant delta makes those journeys +# unwritable. `set_function_stream_delay_ms` paces ONLY the streamed replay: +# text is re-chunked into small deltas with a sleep between them, while the +# JSON lane (and the joined stream text) stays byte-identical to the +# handler's constant. Default is 0 — no pacing, single whole-part delta — +# so nothing changes for existing tests or the hermetic lane unless a +# handlers module opts in (agents/function_handlers_e2e.py does). + + +async def _collect_stream_deltas(agent, task, prompt) -> tuple[list[str], str | None]: + deltas: list[str] = [] + final: str | None = None + with agent.override(model=model_for(task)): + async for event in agent.run_stream_events(prompt, deps=_deps()): + cls_name = type(event).__name__ + if cls_name == "PartStartEvent": + part = getattr(event, "part", None) + if getattr(part, "part_kind", None) == "text" and part.content: + deltas.append(part.content) + elif cls_name == "PartDeltaEvent": + delta = getattr(event, "delta", None) + if getattr(delta, "part_delta_kind", None) == "text": + deltas.append(delta.content_delta) + elif cls_name == "AgentRunResultEvent": + final = event.result.output + return deltas, final + + +def test_stream_pacing_chunks_text_between_sleeps(monkeypatch): + """With a pacing delay set, streamed text replays as multiple small + deltas (24-char chunks) whose join is byte-identical to the handler's + reply, and the run takes at least (chunks - 1) * delay of wall clock — + the mid-stream window the #356 journeys act inside.""" + import asyncio + import time + + from agents._providers import function_stream_delay_ms, set_function_stream_delay_ms + + monkeypatch.setenv("SAPLING_MODEL_MODE", "function") + reply = "x" * 60 # 3 chunks at 24 chars + register_function_handler( + "note_chat", lambda m, i: ModelResponse(parts=[TextPart(content=reply)]) + ) + set_function_stream_delay_ms(25) + assert function_stream_delay_ms() == 25 + + start = time.monotonic() + deltas, final = asyncio.run(_collect_stream_deltas(note_chat_agent, "note_chat", "hi")) + elapsed = time.monotonic() - start + + assert deltas == ["x" * 24, "x" * 24, "x" * 12] + assert final == reply # joined text identical to the JSON-lane constant + # Two inter-chunk sleeps of 25ms each give a hard lower bound; asserting + # only the lower bound keeps this stable on a loaded CI runner. + assert elapsed >= 0.05 + + +def test_stream_pacing_defaults_off_and_resets_with_clear(monkeypatch): + """Default is a single whole-part delta (no pacing), and + clear_function_handlers() — which every seam test fixture already calls — + also resets a previously-set delay, so pacing can never leak between + tests the way a bare module global would.""" + import asyncio + + from agents._providers import function_stream_delay_ms, set_function_stream_delay_ms + + monkeypatch.setenv("SAPLING_MODEL_MODE", "function") + set_function_stream_delay_ms(150) + clear_function_handlers() + assert function_stream_delay_ms() == 0 + + reply = "y" * 60 + register_function_handler( + "note_chat", lambda m, i: ModelResponse(parts=[TextPart(content=reply)]) + ) + deltas, final = asyncio.run(_collect_stream_deltas(note_chat_agent, "note_chat", "hi")) + assert deltas == [reply] # one delta: the whole part, exactly as before + assert final == reply diff --git a/backend/tests/test_streaming_rung1_live.py b/backend/tests/test_streaming_rung1_live.py new file mode 100644 index 00000000..516cafe5 --- /dev/null +++ b/backend/tests/test_streaming_rung1_live.py @@ -0,0 +1,101 @@ +"""Rung 1 of the streaming fallback ladder against the REAL provider (#356 item 3). + +The hermetic suite pins the ladder's mechanics with scripted agents +(test_chat_stream.py: legacy owns persistence, on_complete never runs on the +fallback branch; test_learn_stream_routes.py: the route wiring). What it +structurally cannot prove is the two provider-shaped facts Rung 1 depends on: + + 1. a real pre-token agent failure (here: a nonexistent model name, which the + Google API rejects before any text) surfaces as an exception + `stream_agent_turn` catches — across whatever exception wrapping the + INSTALLED pydantic-ai does (the 1.x → 2.x drift in exactly this seam broke + every main e2e run once, #459); + 2. the legacy `call_gemini_multiturn` path can then serve the turn live, and + the client-visible shape is a SINGLE token event carrying the whole legacy + reply followed by `done` — "the legacy reply arrives as one message". + +This cannot run in the browser E2E lane: the legacy seam (gemini_service) has +no function-mode gate by design — it is fallback-only and scheduled for +deletion in #151 — so the lane's dummy key would make the fallback itself fail. +A live test is the honest vehicle. Costs one real Gemini call. Skipped unless +RUN_LIVE_STREAM_RUNGS=1 and a real key is set (same opt-in posture as +test_vision_ocr_live.py). +""" +from __future__ import annotations + +import asyncio +import os +from types import SimpleNamespace + +import pytest + +pytestmark = pytest.mark.live_llm + +_PLACEHOLDER_KEYS = {"", "dummy-key-for-import", "dummy-not-used-in-tests", "test-key"} + + +def _requires_live_rungs(): + if os.getenv("RUN_LIVE_STREAM_RUNGS") != "1": + pytest.skip("billable live-model lane is opt-in (RUN_LIVE_STREAM_RUNGS=1)") + if (os.getenv("GEMINI_API_KEY") or "").strip() in _PLACEHOLDER_KEYS: + pytest.skip("needs a real GEMINI_API_KEY — this test makes live model calls") + + +def test_rung1_real_agent_failure_degrades_to_live_legacy_reply(monkeypatch): + _requires_live_rungs() + + from pydantic_ai import Agent + + from agents._providers import model_for + from services.chat_stream import stream_agent_turn + from services.gemini_service import MODEL_LITE, call_gemini_multiturn + + # A model name the API cannot serve → the agent fails before any token. + monkeypatch.setenv("SAPLING_MODEL_CHAT_TUTOR", "gemini-model-that-does-not-exist") + monkeypatch.delenv("SAPLING_MODEL_MODE", raising=False) # real mode + + agent = Agent(model=model_for("chat_tutor")) + deps = SimpleNamespace(graph_updates=[], mastery_changes=[]) + persisted: list[str] = [] + + def on_complete(reply, merged, mastery): + persisted.append(reply) + return {} + + async def legacy(): + reply = await asyncio.to_thread( + call_gemini_multiturn, + "You are a terse test assistant. Answer in one short sentence.", + [], + "Reply with the single word: pineapple", + model=MODEL_LITE, + feature="test_live_rung1", + ) + return {"reply": reply} + + async def collect(): + events = [] + async for ev in stream_agent_turn( + agent=agent, + user_message="hello", + run_kwargs={}, + deps=deps, + on_complete=on_complete, + legacy_fallback=legacy, + request_id="live-rung1", + ): + events.append(ev) + return events + + events = asyncio.run(collect()) + types = [ev.type for ev in events] + + # status:start → ONE token (the whole legacy reply as a single message) → + # done. No error event: the fallback served the turn. + assert types == ["status", "token", "done"], types + token, done = events[1], events[2] + assert token.data["delta"] == done.data["reply"] + assert "pineapple" in done.data["reply"].lower() + # The fallback owns persistence — on_complete must not have run (the + # hermetic twin pins this too; here it holds under the real failure shape). + assert persisted == [] diff --git a/docs/decisions/0020-streaming-tutor-interrupt-retry.md b/docs/decisions/0020-streaming-tutor-interrupt-retry.md index a677983f..627298c4 100644 --- a/docs/decisions/0020-streaming-tutor-interrupt-retry.md +++ b/docs/decisions/0020-streaming-tutor-interrupt-retry.md @@ -1,6 +1,6 @@ # 0020 — Interrupted tutor turns keep the partial reply and offer Retry (#356 item 5) -**Status:** decided · **Issue:** #356 (PR #349 e2e-smoke item 5) · **Implements:** the streaming feature in PR #349 +**Status:** implemented (on `main`, with PR #349 merged — ChatPanel `interrupted`/Retry + Learn re-dispatch; journey: `frontend/e2e/streaming.spec.ts`) · **Issue:** #356 (PR #349 e2e-smoke item 5) · **Implements:** the streaming feature in PR #349 ## The decision @@ -51,19 +51,27 @@ The spec (`docs/superpowers/specs/2026-07-16-streaming-design.md`) is explicit: ## Scope / where it lands -The streaming chat UI (`frontend/src/components/ChatPanel.tsx`, -`frontend/src/components/screens/Learn.tsx`, `frontend/src/lib/api.ts`) lives on -the **PR #349 branch (`feat/streaming-tutor`)**, not on `main`. This ADR records -the product decision; the implementation is a follow-up on that branch (or -immediately after it merges), not part of the `main`-based follow-up branch that -carries #354/#355. Acceptance for item 5 in #356 is: this decision recorded -(done here) and implemented on the streaming branch. +Recorded while the streaming chat UI (`frontend/src/components/ChatPanel.tsx`, +`frontend/src/components/screens/Learn.tsx`, `frontend/src/lib/api.ts`) still +lived on the PR #349 branch (`feat/streaming-tutor`); the decision deliberately +preceded the implementation. #349 has since merged, and the implementation +landed on `main` as the ADR-0020 half of the #164/#356 PR: ChatPanel's +`interrupted`/Retry treatment, Learn's ladder rework (Stop / Rung-2 / failed +Rung-3 all keep the partial and offer Retry, with a session-switch guard so a +late turn never writes into another session's transcript), and the +`frontend/e2e/streaming.spec.ts` journeys pinning it. Acceptance for item 5 in +#356 — decision recorded and implemented — is met. ## Consequences - ChatPanel gains an `interrupted` bubble treatment (keep `streamingText`, style as interrupted) and a Retry action; Learn wires Retry to re-dispatch the turn's original `message`/`mode`. +- Scope note (implementation): this covers chat TURNS (`send`'s ladder — Stop, + Rung-2, and a failed Rung-3 fallback all get the interrupted+Retry bubble). + A stopped/failed session GREETING (`beginSession`) has no transcript turn to + mark: no user message exists yet and nothing was persisted, so it returns to + the entry screen with the topic draft intact — Start is its retry affordance. - No backend change: the persistence contract and fallback ladder already guarantee "nothing persisted on stop/failure," which is what makes Retry a plain re-send. This is a client-rendering decision on top of the existing seam. diff --git a/docs/frontend-testids.md b/docs/frontend-testids.md index 47814974..eef9a091 100644 --- a/docs/frontend-testids.md +++ b/docs/frontend-testids.md @@ -139,6 +139,10 @@ route: | `tutor-action-confused` | "I'm confused" | | `tutor-action-skip` | "Skip" | | `tutor-session-resume-{sessionId}` | a "Recent sessions" row's resume button (`screens/Learn.tsx`), suffixed with the session's own id per the stable-domain-id rule | +| `tutor-interrupted` | the "Interrupted" marker inside a stopped/failed assistant bubble (ADR 0020, #356; the partial text stays in the bubble itself) | +| `tutor-retry` | the Retry button inside that marker — re-dispatches the interrupted turn | +| `tutor-back-to-learn` | the chat header's breadcrumb back to the session picker (`screens/Learn.tsx::BackToLearnLink`) | +| `tutor-resume-loading` | the transient loading state while a `/learn?resume=` deep link hydrates (#164) | | `tutor-focus-concept-description` | the knowledge-map rail's "Focused concept" card description text (`screens/Learn.tsx`) — stored `description` if the node has one, else the AI-fetched blurb (`POST /api/graph/{user}/concept-description`, #446), else the connected-concepts fallback sentence | ### `quiz` @@ -231,6 +235,7 @@ never collide in the DOM. | `dashboard-courses-key-toggle` | expand/collapse toggle of the "My courses" key overlay on the graph panel (default sidebar layout; the key starts collapsed) | | `dashboard-course-code` | a course row's code/name label inside the expanded key — repeated per course with **no suffix** (deliberate deviation from the suffix rule above: journeys select a row by seeded content, `getByTestId(…).filter({ hasText })`, so no per-row identity is exposed) | | `dashboard-courses-manage` | the cog inside the expanded key that opens the Courses & Semesters hub (the hub's own semester tabs are plain text buttons — journeys select them by role/name, e.g. "All semesters" / "Fall 2025") | +| `dashboard-resume-{sessionId}` | a "Where you left off" card — deep-links to `/learn?resume={sessionId}` (#164), suffixed with the session's own id per the stable-domain-id rule | ### `library` diff --git a/frontend/e2e/streaming.spec.ts b/frontend/e2e/streaming.spec.ts new file mode 100644 index 00000000..71988551 --- /dev/null +++ b/frontend/e2e/streaming.spec.ts @@ -0,0 +1,261 @@ +/** + * Streaming fallback-ladder journeys (#356, ADR 0020) — the browser-lane + * promotion of PR #349's remaining manual smoke items: + * + * item 6 — the stream failing to OPEN falls back to the JSON turn + * transparently (client Rung 3); the user sees a normal reply. + * items 4+5 — Stop mid-stream keeps the partial reply, marks the bubble + * interrupted, offers Retry (ADR 0020), and persists NOTHING; + * Retry then completes the turn and persists exactly one pair. + * item 7 — switching sessions mid-stream aborts the stream and leaves no + * stale bubble in the other session's transcript. + * + * (Item 3 — the SERVER-side Rung-1 legacy fallback — deliberately has no + * journey here: the legacy gemini_service seam has no function-mode gate by + * design (fallback-only, slated for deletion in #151), so it cannot run + * deterministically in this lane. Its live proof is + * backend/tests/test_streaming_rung1_live.py; its mechanics are pinned + * hermetically in test_chat_stream.py / test_learn_stream_routes.py.) + * + * Mid-stream windows are real: the function-mode seam paces streamed replay + * (agents/function_handlers_e2e.py sets a 150ms inter-chunk delay at import), + * and a message carrying E2E_SLOW_STREAM_TRIGGER gets the LONG deterministic + * reply — ~40 paced chunks ≈ a 6-second stream to act inside. Stack boot: + * + * SAPLING_MODEL_MODE=function \ + * SAPLING_FUNCTION_HANDLERS=agents.function_handlers_e2e make e2e-up + */ +import { expect, test } from "./support/fixtures"; +import { queryRaw } from "./support/db"; +import { decryptTexts } from "./support/decrypt"; + +/** Seeded by db/seed_local_rich.py for rich-user-active. */ +const SESSION_ID = "rich-sess-cs-recursion"; +const SEEDED_MESSAGE_COUNT = 4; +const MATH_SESSION_ID = "rich-sess-math-vectors"; +const MATH_SEEDED_MESSAGE_COUNT = 2; + +/** Must match backend/agents/function_handlers_e2e.py::E2E_TUTOR_REPLY. */ +const TUTOR_REPLY = + "[e2e-function-model] Deterministic tutor reply: every recursive function " + + "needs a base case so it can stop calling itself."; + +/** Must match backend/agents/function_handlers_e2e.py::E2E_SLOW_STREAM_TRIGGER. */ +const SLOW_TRIGGER = "E2E_SLOW_STREAM"; + +/** Must match backend/agents/function_handlers_e2e.py::E2E_TUTOR_SLOW_REPLY. */ +const SLOW_REPLY = + "[e2e-function-model] Deterministic SLOW tutor reply for mid-stream " + + "journeys. Recursion solves a problem by reducing it to a smaller copy " + + "of itself, and every recursive function needs two ingredients: a base " + + "case that stops the descent, and a recursive step that makes real " + + "progress toward that base case on every call. Picture the call stack " + + "as a tower of postponed promises: each frame waits for the smaller " + + "problem beneath it to resolve before it can finish its own work. When " + + "the base case finally answers, the tower unwinds in reverse order and " + + "every waiting frame completes with the value it was promised. If the " + + "recursive step ever fails to shrink the problem, the tower grows " + + "without bound until the runtime refuses to add another frame and the " + + "program crashes with a stack overflow. That is the whole discipline in " + + "one sentence: shrink toward a base case you are certain to reach. This " + + "is the final sentence of the slow deterministic reply."; + +/** Rendered within the first few paced chunks — the "stream is live" gate. */ +const SLOW_REPLY_PREFIX = "[e2e-function-model] Deterministic SLOW"; +/** Rendered only when the stream ran to completion — the completion sentinel. */ +const SLOW_REPLY_TAIL = "final sentence of the slow deterministic reply"; + +const STUDENT_SLOW_MESSAGE = `Walk me through this ${SLOW_TRIGGER} please (e2e #356)`; +const STUDENT_MESSAGE = "What breaks without a base case? (e2e #356 item 6)"; + +async function messageRows(sessionId: string) { + return (await queryRaw( + `SELECT role, content FROM messages + WHERE session_id = $1 + ORDER BY created_at ASC`, + [sessionId], + )) as { role: string; content: string }[]; +} + +async function openSeededSession(page: import("@playwright/test").Page, sessionId: string) { + await page.getByTestId(`tutor-session-resume-${sessionId}`).click(); + await expect(page.getByTestId("tutor-messages")).toBeVisible(); +} + +test.beforeEach(async ({ page }) => { + // The per-browser AI-disclosure modal would swallow composer clicks; model + // a returning user who has already acknowledged it (same as tutor.spec.ts). + await page.addInitScript(() => { + localStorage.setItem("sapling_disclaimer_ack", "true"); + }); +}); + +test("stream that fails to open falls back to the JSON turn transparently (#356 item 6)", async ({ + page, +}) => { + // Kill the SSE route at the network layer: the client's ladder must degrade + // to the non-streaming JSON turn with no user-visible error (Rung 3). + await page.route("**/api/learn/chat/stream", route => route.abort()); + + await page.goto("/learn"); + await openSeededSession(page, SESSION_ID); + + await page.getByTestId("tutor-input").fill(STUDENT_MESSAGE); + await page.getByTestId("tutor-send").click(); + + const log = page.getByTestId("tutor-messages"); + await expect(log).toContainText(STUDENT_MESSAGE); + await expect(log).toContainText(TUTOR_REPLY); + // Transparent means transparent: no interrupted chrome, no error bubble. + await expect(page.getByTestId("tutor-interrupted")).toHaveCount(0); + + // Exactly one user+assistant pair persisted, by the JSON route. + const rows = await messageRows(SESSION_ID); + expect(rows).toHaveLength(SEEDED_MESSAGE_COUNT + 2); + const [userRow, assistantRow] = rows.slice(-2); + expect(userRow.role).toBe("user"); + expect(assistantRow.role).toBe("assistant"); + // Both encryption checks, per support/decrypt.ts ("Specs must always make + // both"): ciphertext at rest — decrypt_if_present echoes plaintext back, + // so decrypt-equality alone can't prove the column was encrypted… + expect(userRow.content).not.toBe(STUDENT_MESSAGE); + expect(assistantRow.content).not.toBe(TUTOR_REPLY); + // …and decrypting to exactly what was sent and rendered. + const [userPlain, assistantPlain] = await decryptTexts([ + userRow.content, + assistantRow.content, + ]); + expect(userPlain).toBe(STUDENT_MESSAGE); + expect(assistantPlain).toBe(TUTOR_REPLY); +}); + +test("Stop mid-stream keeps the partial, marks it interrupted, persists nothing; Retry completes (#356 items 4+5, ADR 0020)", async ({ + page, +}) => { + await page.goto("/learn"); + await openSeededSession(page, SESSION_ID); + + await page.getByTestId("tutor-input").fill(STUDENT_SLOW_MESSAGE); + await page.getByTestId("tutor-send").click(); + + const log = page.getByTestId("tutor-messages"); + // Wait for live streamed text (the paced window is ~6s), then Stop. + await expect(log).toContainText(SLOW_REPLY_PREFIX); + await page.getByTestId("tutor-stop").click(); + + // ADR 0020: the partial stays visible, marked interrupted, with Retry. + await expect(page.getByTestId("tutor-interrupted")).toBeVisible(); + await expect(page.getByTestId("tutor-retry")).toBeVisible(); + await expect(log).toContainText(SLOW_REPLY_PREFIX); + await expect(log).not.toContainText(SLOW_REPLY_TAIL); + + // Item 4's persistence contract: an interrupted turn writes NOTHING — no + // phantom partial assistant row, and no orphaned user row either. + expect(await messageRows(SESSION_ID)).toHaveLength(SEEDED_MESSAGE_COUNT); + + // Retry re-sends the same turn (safe precisely because nothing persisted) + // and this time it runs to completion. + await page.getByTestId("tutor-retry").click(); + await expect(log).toContainText(SLOW_REPLY_TAIL, { timeout: 30_000 }); + await expect(page.getByTestId("tutor-interrupted")).toHaveCount(0); + + const rows = await messageRows(SESSION_ID); + expect(rows).toHaveLength(SEEDED_MESSAGE_COUNT + 2); + const [userRow, assistantRow] = rows.slice(-2); + expect(userRow.role).toBe("user"); + expect(assistantRow.role).toBe("assistant"); + // Both encryption checks, per support/decrypt.ts ("Specs must always make + // both") — ciphertext at rest, then decrypt-equality. + expect(userRow.content).not.toBe(STUDENT_SLOW_MESSAGE); + expect(assistantRow.content).not.toBe(SLOW_REPLY); + const [userPlain, assistantPlain] = await decryptTexts([ + userRow.content, + assistantRow.content, + ]); + expect(userPlain).toBe(STUDENT_SLOW_MESSAGE); + expect(assistantPlain).toBe(SLOW_REPLY); +}); + +test("switching sessions during the JSON fallback drops the late reply — no stale bubble (#356 item 7, PR #461 review)", async ({ + page, +}) => { + // The Rung-3 leg is the one that structurally CANNOT be aborted: sendChat + // takes no AbortSignal. Kill the stream so the turn degrades to Rung 3, + // and delay the JSON route long enough to switch sessions while it is in + // flight. The late reply must be dropped client-side (the sessionIdRef + // guard on the success append) — it still persists server-side to the + // ORIGINAL session, which is exactly why dropping the append loses nothing. + await page.route("**/api/learn/chat/stream", route => route.abort()); + await page.route("**/api/learn/chat", async route => { + await new Promise(resolve => setTimeout(resolve, 3_000)); + await route.continue(); + }); + + const jsonDone = page.waitForResponse( + response => response.url().endsWith("/api/learn/chat") && response.request().method() === "POST", + ); + + await page.goto("/learn"); + await openSeededSession(page, SESSION_ID); + await page.getByTestId("tutor-input").fill(STUDENT_MESSAGE); + await page.getByTestId("tutor-send").click(); + + // While the delayed JSON call is in flight: leave and open the other session. + await page.getByTestId("tutor-back-to-learn").click(); + await page.getByTestId(`tutor-session-resume-${MATH_SESSION_ID}`).click(); + const log = page.getByTestId("tutor-messages"); + await expect(log).toContainText("What is a dot product?"); + + // Let the late reply land. Server-side, the JSON route persisted the pair + // to the ORIGINAL session (these DB round-trips also give the client time + // to mis-append if the guard were broken — the negative asserts below + // would then catch it, rather than racing the response's microtask). + await jsonDone; + const rows = await messageRows(SESSION_ID); + expect(rows).toHaveLength(SEEDED_MESSAGE_COUNT + 2); + expect(await messageRows(MATH_SESSION_ID)).toHaveLength(MATH_SEEDED_MESSAGE_COUNT); + + // …and the late reply never reached the math transcript. + await expect(log).toContainText("What is a dot product?"); + await expect(log).not.toContainText(TUTOR_REPLY); + await expect(log).not.toContainText(STUDENT_MESSAGE); +}); + +test("switching sessions mid-stream aborts the stream and leaves no stale bubble (#356 item 7)", async ({ + page, +}) => { + // Collect stream-request failures as they happen; asserted eventually via + // poll below (the abort races the navigation, so a one-shot waitForEvent + // registered after the click could miss it). + const failedStreams: string[] = []; + page.on("requestfailed", request => { + if (request.url().includes("/api/learn/chat/stream")) { + failedStreams.push(request.failure()?.errorText ?? "failed"); + } + }); + + await page.goto("/learn"); + await openSeededSession(page, SESSION_ID); + + await page.getByTestId("tutor-input").fill(STUDENT_SLOW_MESSAGE); + await page.getByTestId("tutor-send").click(); + await expect(page.getByTestId("tutor-messages")).toContainText(SLOW_REPLY_PREFIX); + + // Mid-stream: leave the chat and open the OTHER seeded session. + await page.getByTestId("tutor-back-to-learn").click(); + await page.getByTestId(`tutor-session-resume-${MATH_SESSION_ID}`).click(); + + // The math transcript renders; the recursion session's streamed text must + // NOT bleed into it (no stale bubble, interrupted or otherwise). + const log = page.getByTestId("tutor-messages"); + await expect(log).toContainText("What is a dot product?"); + await expect(log).not.toContainText(SLOW_REPLY_PREFIX); + await expect(page.getByTestId("tutor-interrupted")).toHaveCount(0); + + // The SSE request was actually torn down (no leaked stream)… + await expect.poll(() => failedStreams.length, { timeout: 5_000 }).toBeGreaterThan(0); + + // …and the interrupted turn persisted nothing to either session. + expect(await messageRows(SESSION_ID)).toHaveLength(SEEDED_MESSAGE_COUNT); + expect(await messageRows(MATH_SESSION_ID)).toHaveLength(MATH_SEEDED_MESSAGE_COUNT); +}); diff --git a/frontend/e2e/tutor.spec.ts b/frontend/e2e/tutor.spec.ts index d38abe6a..51cfd859 100644 --- a/frontend/e2e/tutor.spec.ts +++ b/frontend/e2e/tutor.spec.ts @@ -13,14 +13,11 @@ * * so `model_for("chat_tutor")` builds a FunctionModel (#391) and the backend * self-registers the fixed-reply handler at first dispatch (#392). The spec - * enters the chat by RESUMING a seeded session — deliberately not via - * "Start learning": `POST /api/learn/start-session` still runs on the legacy - * `call_gemini_multiturn` path (routes/learn.py), which the seam does not - * cover, and this journey must never depend on live Gemini. - * - * No token streaming: `sendChat` (src/lib/api.ts) is a plain fetch returning - * the full `{ reply, ... }` object, so the assertions wait on the rendered - * reply locator — no SSE handling, no waitForTimeout. + * enters the chat by RESUMING a seeded session — keeping the journey off the + * greeting turn entirely, so its assertions are exactly one send → one reply. + * (Since #349 the composer streams over SSE with a JSON fallback ladder; the + * seam replays the same constant on both lanes, so the assertions just wait + * on the rendered reply locator either way — no SSE handling here.) */ import { expect, test } from "./support/fixtures"; import { queryRaw } from "./support/db"; @@ -104,6 +101,39 @@ test("tutor turn renders a reply and persists encrypted to messages", async ({ expect(assistantPlain).toBe(TUTOR_REPLY); }); +/** + * Journey (#164): the Dashboard "Where you left off" card deep-links into the + * exact session with its history hydrated. The bug being pinned: Learn read + * only `topic`/`mode`/`course`/`suggest` off the URL, so the `?resume=` the + * card pushes (and Tree's session rows, now unified on the same param) was + * silently dropped — the card landed on the "Start a session" picker, a dead + * button in effect. Entry is via the REAL dashboard card, not a direct URL, + * so the whole caller → param → resume wiring is on the hook. + */ +test("dashboard 'Where you left off' card resumes the exact session (#164)", async ({ + page, +}) => { + await page.addInitScript(() => { + localStorage.setItem("sapling_disclaimer_ack", "true"); + }); + + await page.goto("/dashboard"); + await page.getByTestId(`dashboard-resume-${SESSION_ID}`).click(); + await expect(page).toHaveURL(/\/learn\?/); + + // The seeded conversation hydrates — both an early and a late seeded turn, + // proving the full history loaded rather than a fresh chat on the topic. + const log = page.getByTestId("tutor-messages"); + await expect(log).toContainText("Can you explain recursion?"); + await expect(log).toContainText( + "The base case is the condition where the function stops calling itself.", + ); + + // And it's the chat view, not the session picker the bug used to strand + // users on (the picker's resume row only exists on the entry screen). + await expect(page.getByTestId(`tutor-session-resume-${SESSION_ID}`)).toHaveCount(0); +}); + /** * Journey (#446): resuming a session surfaces the knowledge-map rail's * "Focused concept" card for that session's topic node. The seeded diff --git a/frontend/eslint-suppressions.json b/frontend/eslint-suppressions.json index f9145d12..8e41b55e 100644 --- a/frontend/eslint-suppressions.json +++ b/frontend/eslint-suppressions.json @@ -88,7 +88,7 @@ }, "src/components/screens/Dashboard.tsx": { "no-restricted-syntax": { - "count": 21 + "count": 20 }, "react/no-unescaped-entities": { "count": 2 @@ -99,7 +99,7 @@ "count": 2 }, "no-restricted-syntax": { - "count": 21 + "count": 20 } }, "src/components/screens/Library.tsx": { diff --git a/frontend/src/components/ChatPanel.test.tsx b/frontend/src/components/ChatPanel.test.tsx new file mode 100644 index 00000000..ecef45de --- /dev/null +++ b/frontend/src/components/ChatPanel.test.tsx @@ -0,0 +1,101 @@ +// @vitest-environment jsdom +/** + * ADR 0020 (#356 item 5): an interrupted tutor turn KEEPS its partial text, + * renders an explicit "Interrupted" marker, and offers Retry. Nothing is + * persisted server-side for such a turn (routes/learn.py persists only on + * completion), so Retry re-dispatching the same turn is safe — that wiring + * lives in Learn.tsx; this test pins the ChatPanel rendering contract the + * streaming journey (frontend/e2e/streaming.spec.ts) anchors on: + * - tutor-interrupted marker on interrupted bubbles only + * - tutor-retry only when a retryText + onRetry are present + * - the partial content stays visible (never blanked) + */ + +import React from "react"; +import { describe, it, expect, vi, afterEach } from "vitest"; +import { render, screen, cleanup, fireEvent } from "@testing-library/react"; + +import { ChatPanel, type ChatMsg } from "./ChatPanel"; + +vi.mock("./Icon", () => ({ Icon: () => null })); +// MarkdownChat is lazy-loaded via next/dynamic (heavy markdown stack); render +// plain text in its place so assertions see the message content synchronously. +vi.mock("next/dynamic", () => ({ + default: () => + function MarkdownStub({ children }: { children: React.ReactNode }) { + return
{children}
; + }, +})); + +afterEach(cleanup); + +const noop = () => {}; + +function interruptedMsg(over: Partial = {}): ChatMsg { + return { + id: "m-1", + role: "assistant", + content: "partial reply text that must stay visible", + interrupted: true, + retryText: "original user question", + ...over, + }; +} + +describe("ChatPanel interrupted-turn treatment (ADR 0020)", () => { + it("renders the marker, keeps the partial text, and fires onRetry with the message", () => { + const onRetry = vi.fn(); + render( + , + ); + + expect(screen.getByTestId("tutor-interrupted")).toBeTruthy(); + expect( + screen.getByText("partial reply text that must stay visible"), + ).toBeTruthy(); + + fireEvent.click(screen.getByTestId("tutor-retry")); + expect(onRetry).toHaveBeenCalledTimes(1); + expect(onRetry.mock.calls[0][0].retryText).toBe("original user question"); + }); + + it("shows the marker even when the turn was stopped before any token", () => { + // Stop before the first token: no partial text, but the turn still needs + // the interrupted affordance — otherwise the user's message sits with no + // reply and no recourse (the exact outcome ADR 0020 rejects). + render( + , + ); + expect(screen.getByTestId("tutor-interrupted")).toBeTruthy(); + expect(screen.getByTestId("tutor-retry")).toBeTruthy(); + }); + + it("renders no interrupted chrome on settled messages", () => { + render( + , + ); + expect(screen.queryByTestId("tutor-interrupted")).toBeNull(); + expect(screen.queryByTestId("tutor-retry")).toBeNull(); + }); + + it("omits Retry when no handler is wired (marker still shows)", () => { + render(); + expect(screen.getByTestId("tutor-interrupted")).toBeTruthy(); + expect(screen.queryByTestId("tutor-retry")).toBeNull(); + }); +}); diff --git a/frontend/src/components/ChatPanel.tsx b/frontend/src/components/ChatPanel.tsx index e830956a..a5112d1c 100644 --- a/frontend/src/components/ChatPanel.tsx +++ b/frontend/src/components/ChatPanel.tsx @@ -20,6 +20,14 @@ export interface ChatMsg { role: ChatRole; content: string; loading?: boolean; + /** ADR 0020: the turn was stopped or failed mid-stream. The partial text + * stays in `content` (possibly empty when stopped pre-token); the bubble + * renders an "Interrupted" marker and — when `retryText` is set — a Retry + * action. Nothing was persisted server-side for such a turn (routes + * persist only on completion), so Retry is a plain re-send. */ + interrupted?: boolean; + /** The user text that produced this (interrupted) turn — what Retry re-sends. */ + retryText?: string; } interface ChatPanelProps { @@ -39,6 +47,9 @@ interface ChatPanelProps { streamingText?: string | null; /** Abort the in-flight turn. Shown only while streaming. */ onStop?: () => void; + /** Re-dispatch an interrupted turn (ADR 0020). Receives the interrupted + * message; Learn re-sends its `retryText` after dropping the failed pair. */ + onRetry?: (m: ChatMsg) => void; } export function ChatPanel({ @@ -52,6 +63,7 @@ export function ChatPanel({ draftSeedKey, streamingText, onStop, + onRetry, }: ChatPanelProps) { const scrollRef = useRef(null); const isStreaming = streamingText !== null && streamingText !== undefined; @@ -82,7 +94,7 @@ export function ChatPanel({ gap: 16, }} > - {messages.map(m => )} + {messages.map(m => )} {streamingText !== null && streamingText !== undefined && ( // Reuse Message/MarkdownChat exactly as settled assistant messages // do, so a streaming reply never styles differently from a @@ -237,7 +249,13 @@ const ChatInputBar = React.memo(function ChatInputBar({ ); }); -const Message = React.memo(function Message({ m }: { m: ChatMsg }) { +const Message = React.memo(function Message({ + m, + onRetry, +}: { + m: ChatMsg; + onRetry?: (m: ChatMsg) => void; +}) { const isUser = m.role === "user"; return (
{m.content} + // ADR 0020: an interrupted turn keeps its partial text visible, + // just dimmed — never blanked. +
+ {m.content ? {m.content} : null} +
+ )} + {m.interrupted && !isUser && ( +
+ Interrupted + {onRetry && m.retryText && ( + + )} +
)}
diff --git a/frontend/src/components/screens/Dashboard.tsx b/frontend/src/components/screens/Dashboard.tsx index 506734c8..c233a477 100644 --- a/frontend/src/components/screens/Dashboard.tsx +++ b/frontend/src/components/screens/Dashboard.tsx @@ -564,6 +564,7 @@ export function Dashboard() { sessions.slice(0, 3).map((s) => (