diff --git a/.agent/phases/todo/20_sources_midstream_bug/01_persist_inflight_turn.md b/.agent/phases/todo/20_sources_midstream_bug/01_persist_inflight_turn.md deleted file mode 100644 index 0ef26bc..0000000 --- a/.agent/phases/todo/20_sources_midstream_bug/01_persist_inflight_turn.md +++ /dev/null @@ -1,83 +0,0 @@ -# Task 01 — app.js: persist the partial answer on navigate-away (pagehide) - -**Phase:** `20_sources_midstream_bug` · **Source:** `TODO.md` L3 — -*"Clicking "sources" while chat is generating clears chat and result will -never show up"* - -## Objective -`frontend/assets/app.js` gains exactly one new save point: on `pagehide` -(navigate-away / bfcache), if a turn is in flight and answer text has -already streamed, the partial answer is persisted via the existing -`rememberBrainTurn` helper — so returning to the chat restores the -question **and** what had been generated. - -## Work -1. `frontend/assets/app.js` - - Next to the other turn-local state (around the `acc` / - `thinkingAcc` / `sawThinking` / `aborted` declarations, ~line 920+), - declare a turn-local flag: - ```js - let persistedOnLeave = false; // pagehide partial-persist at most once - ``` - and reset it to `false` at the top of `runTurn` (where `acc`, - `thinkingAcc`, `sawThinking`, `wrap` are initialized), so it is - turn-scoped like the rest. - - Register one handler at module boot (next to the other - `window.addEventListener` calls): - ```js - /* Navigate-away save point (phase 20, owner choice 2026-08-24 A1): - * leaving the chat mid-turn would otherwise drop the in-flight - * answer — the brain message persists only on `done`, and - * navigation aborts the stream. On `pagehide`, if a turn is in - * flight and answer text has streamed, persist the partial raw text - * (reusing the save-point helper, so restore re-renders it exactly - * like a completed answer — no "(partial)" marker, no sources). - * Thinking-only (no answer tokens yet) persists nothing brain-side: - * the question is already saved on send and the user can re-ask. - * `persistedOnLeave` makes this idempotent across pagehide/bfcache - * churn. */ - window.addEventListener("pagehide", () => { - if (persistedOnLeave) return; - if (uiState !== UI_STATE.thinking && uiState !== UI_STATE.streaming) - return; - if (!acc) return; // nothing brain-side to save yet - persistedOnLeave = true; - rememberBrainTurn(acc, { thinking: thinkingAcc || undefined }); - }); - ``` - **ASSUMPTION (owner-confirmed A1):** the partial is a plain brain - message (no marker, no sources); the variables `uiState`, `acc`, - `thinkingAcc` are the turn's existing ones — match the names actually - in scope (the file keeps `let acc = ""` / `let thinkingAcc = ""` - turn-locals inside `runTurn`; if the handler needs them outside that - scope, hoist the turn-locals to module scope *without* changing any - behavior — smallest diff wins). - - Update the phase-14 persistence block comment (~line 631): the - "Save points:" sentence now lists three — the user message on send, - the brain message on `done`, and the **partial** brain message on - navigate-away (`pagehide`, phase 20). -2. `tests/unit/test_sources_midstream.py` (new — follow the repo's - source-level pin pattern used by `tests/unit/test_shared_header.py`): - - `app.js` contains `addEventListener("pagehide"` exactly once. - - The pagehide body is guarded by the in-flight states (`thinking` - and `streaming`) and a non-empty `acc` check. - - The pagehide body calls `rememberBrainTurn(acc,` with - `thinking: thinkingAcc || undefined`. - - `persistedOnLeave` is declared and reset in `runTurn`. - - The persistence block comment lists the `pagehide` save point. - - `clearChatStorage` (header.js) is untouched — still the only - deliberate clear. - -## Testing & Quality -- `uv run pytest tests/unit/test_sources_midstream.py -v` green. -- `uv run ruff check . && uv run pyright` clean. -- Manual smoke (dev server, DEBUGPY optional): send a question against - the real LLM (or any slow stream), click Sources mid-stream, return — - the partial answer is visible. - -## Completion Criteria -- [ ] The `pagehide` handler exists, is turn-scoped and idempotent, and - reuses `rememberBrainTurn` (no duplicated storage code). -- [ ] No other save point changed: send + `done` behave byte-identically - to before (existing persistence unit pins still pass). -- [ ] Unit pins green; lint/types clean. diff --git a/frontend/assets/app.js b/frontend/assets/app.js index 699a3e5..7a17dc8 100644 --- a/frontend/assets/app.js +++ b/frontend/assets/app.js @@ -501,6 +501,13 @@ let uiState = UI_STATE.idle; let thinkingClock = 0; // setInterval id — elapsed-seconds hint let thinkingStart = 0; // Date.now() when "thinking" began let turnTimeout = 0; // setTimeout id — 120s pre-token guard +/* Turn accumulators + the navigate-away flag (phase 20, owner choice + * 2026-08-24 A1): module scope because the `pagehide` handler reads them + * while a turn is still in flight; reset per turn at the top of + * handleSend, so they stay turn-scoped like the rest of the turn locals. */ +let acc = ""; // accumulated answer text this turn +let thinkingAcc = ""; // accumulated thinking text (persisted with the turn) +let persistedOnLeave = false; // pagehide partial-persist at most once function stopThinkingClock() { if (thinkingClock) { @@ -640,8 +647,11 @@ function appendMaybeTry(wrap, suggestions) { * * Only RAW TEXT is stored — restore re-renders it through the escape-first * markdown renderer, so no HTML is ever persisted. Save points: the user - * message on send (a failed turn keeps the question) and the brain message - * on `done` (with sources/deflected/suggestions). Every localStorage access + * message on send (a failed turn keeps the question), the brain message on + * `done` (with sources/deflected/suggestions), and the PARTIAL brain + * message on navigate-away (`pagehide`, phase 20 — an in-flight turn keeps + * whatever had already streamed; thinking-only turns persist nothing + * brain-side). Every localStorage access * is try/catch'd — private mode or quota exhaustion degrades silently to * in-memory-only chat. If the serialized state outgrows the budget (~700k * chars, far under the ~5MB quota) the oldest messages are dropped first. @@ -833,11 +843,14 @@ async function handleSend(e) { clearErrorBanner(); let wrap = null; - let acc = ""; let res = null; let aborted = false; // the 120s guard already took the turn to error - // Phase 17 (thinking display): turn-local reasoning state. - let thinkingAcc = ""; // accumulated thinking text (persisted with the turn) + // Phase 20: acc / thinkingAcc / persistedOnLeave live at module scope + // (the pagehide handler reads them) but reset here, so they stay + // turn-scoped exactly like the other turn locals. + acc = ""; + thinkingAcc = ""; + persistedOnLeave = false; let sawThinking = false; // did any `thinking` frame arrive this turn? let sawDone = false; // did the stream end with a `done` event? @@ -972,6 +985,26 @@ input.addEventListener("keydown", (e) => { }); composer.addEventListener("submit", handleSend); +/* Navigate-away save point (phase 20, owner choice 2026-08-24 A1): + * leaving the chat mid-turn would otherwise drop the in-flight + * answer — the brain message persists only on `done`, and + * navigation aborts the stream. On `pagehide`, if a turn is in + * flight and answer text has streamed, persist the partial raw text + * (reusing the save-point helper, so restore re-renders it exactly + * like a completed answer — no "(partial)" marker, no sources). + * Thinking-only (no answer tokens yet) persists nothing brain-side: + * the question is already saved on send and the user can re-ask. + * `persistedOnLeave` makes this idempotent across pagehide/bfcache + * churn. */ +window.addEventListener("pagehide", () => { + if (persistedOnLeave) return; + if (uiState !== UI_STATE.thinking && uiState !== UI_STATE.streaming) + return; + if (!acc) return; // nothing brain-side to save yet + persistedOnLeave = true; + rememberBrainTurn(acc, { thinking: thinkingAcc || undefined }); +}); + /* Boot: auth state FIRST — it decides whether the restored conversation gets Tune buttons and whether the steering UI exists at all (phase 16). Phase 14: the conversation then comes back exactly as left. Phase 19: diff --git a/tests/e2e/mock_llm.py b/tests/e2e/mock_llm.py index 514dddd..056d4ba 100644 --- a/tests/e2e/mock_llm.py +++ b/tests/e2e/mock_llm.py @@ -18,6 +18,10 @@ Implements just enough of the aipi surface: - user message containing ``think out loud`` -> the answer is preceded by ~800 chars of deterministic ``reasoning_content`` chunks (the thinking-display story, phase 17). + - user message containing ``think out loud then hesitate`` -> the + ``think out loud`` stream, then a 4s pause before the first content + frame (the sources-midstream story, phase 20 — a deterministic + "leave during pure thinking" navigation window). - system prompt containing ```` (phase 15, steering notes) -> the composed answer ends with `` (tuning: )`` — makes prompt injection observable in the UI deterministically. @@ -89,6 +93,16 @@ LONG_ANSWER_END = "LONG-ANSWER-END" #: contain the substring, so every other suite is unaffected. THINKING_TRIGGER = "think out loud" +#: Phase 20 (sources-midstream bug): a user message containing this +#: substring (case-insensitive) gets the phase-17 thinking stream followed +#: by a multi-second pause before the FIRST content frame — the +#: navigation window for the "leave during pure thinking" scenario +#: (owner-confirmed A1.2: nothing brain-side may be persisted then). +#: Strictly longer than ``THINKING_TRIGGER``, so the phase-17 suite's +#: questions are unaffected. +SLOW_PRETOKEN_TRIGGER = "think out loud then hesitate" +PRE_CONTENT_PAUSE_S = 4.0 + def long_answer() -> str: """~900-word deterministic walkthrough (phase 11): numbered steps plus @@ -222,7 +236,9 @@ def embeddings(body: dict[str, Any]) -> dict[str, Any]: } -def _sse_stream(answer: str, delay: float, thinking: str = "") -> Any: +def _sse_stream( + answer: str, delay: float, thinking: str = "", pre_content_delay: float = 0.0 +) -> Any: """SSE frames for one chat completion (phase 17: + reasoning). When ``thinking`` is non-empty its 12-char slices go out FIRST as @@ -230,6 +246,11 @@ def _sse_stream(answer: str, delay: float, thinking: str = "") -> Any: as the content frames, the aipi wire convention (reasoning before content). Without ``thinking`` the output is byte-identical to the content-only stream, so the other story suites are unaffected. + + ``pre_content_delay`` (phase 20) inserts a silence gap between the end + of the thinking stream and the first content frame — the client stays + in its pre-token "thinking" state the whole time (0.02s cadence and + frame shapes are unchanged, so 0.0 is byte-identical to before). """ model = "turbo" chunk_id = f"chatcmpl-{uuid.uuid4()}" @@ -247,6 +268,8 @@ def _sse_stream(answer: str, delay: float, thinking: str = "") -> Any: } yield f"data: {json_dumps(payload)}\n\n" time.sleep(0.02) + if pre_content_delay: + time.sleep(pre_content_delay) for piece in re.findall(r".{1,12}", answer, re.S): payload = { "id": chunk_id, @@ -295,7 +318,11 @@ def _apply_max_tokens(answer: str, max_tokens: Any) -> str: def chat_completions(body: dict[str, Any]) -> Any: answer = _apply_max_tokens(compose_answer(body), body.get("max_tokens")) delay = 3.0 if "pretend to think slowly" in _user(body) else 0.0 - thinking = compose_thinking(body) if THINKING_TRIGGER in _user(body).lower() else "" + user_lower = _user(body).lower() + thinking = compose_thinking(body) if THINKING_TRIGGER in user_lower else "" + pre_content = ( + PRE_CONTENT_PAUSE_S if SLOW_PRETOKEN_TRIGGER in user_lower else 0.0 + ) if not body.get("stream"): message: dict[str, Any] = {"role": "assistant", "content": answer} @@ -315,7 +342,7 @@ def chat_completions(body: dict[str, Any]) -> Any: } return StreamingResponse( - _sse_stream(answer, delay, thinking=thinking), + _sse_stream(answer, delay, thinking=thinking, pre_content_delay=pre_content), media_type="text/event-stream", headers={"Cache-Control": "no-cache", "X-Accel-Buffering": "no"}, ) diff --git a/tests/e2e/test_sources_midstream_bug.py b/tests/e2e/test_sources_midstream_bug.py new file mode 100644 index 0000000..16e848b --- /dev/null +++ b/tests/e2e/test_sources_midstream_bug.py @@ -0,0 +1,359 @@ +"""Phase 20 E2E (Playwright): navigating away mid-turn keeps the answer. + +Story: ``.agent/user_stories/sources-midstream.md`` +Bug report (TODO.md L3): *"Clicking "sources" while chat is generating +clears chat and result will never show up."* +Run in isolation (DB must be up: ``podman compose up -d db``): + + uv run pytest tests/e2e/test_sources_midstream_bug.py -v --no-cov + +The bug: the brain message persisted only on ``done``, so leaving the +chat page while a turn was in flight aborted the stream and dropped +whatever had already streamed — the user came back to their own question +with no result, ever. The fix (phase 20, owner-confirmed A1): a single +``pagehide`` save point in app.js persists the partial raw answer (via +the existing ``rememberBrainTurn`` helper) when navigation hits a turn +that is in flight and has already streamed text. + +Timing is deterministic by construction: + +* scenario 1 keys off the mock's ``write a long answer`` trigger — a + ~5400-char / ~450-frame / ~9s content stream, so the navigation lands + mid-stream with a wide margin; +* scenario 2 keys off the mock's ``think out loud then hesitate`` + trigger — the phase-17 thinking stream followed by a 4s silence before + the first content frame, so the navigation lands inside pure thinking; +* scenarios 3 and 4 settle the turn fully (send button re-enabled) + before any navigation. + +Test → story mapping (Playwright Mapping Rule): +1. ``test_partial_answer_survives_sources_nav_midstream`` +2. ``test_no_orphan_brain_message_when_navigated_before_first_token`` +3. ``test_completed_turn_unaffected`` +4. ``test_new_chat_still_clears_conversation`` +""" +from __future__ import annotations + +import asyncio +import json +import re +from collections.abc import Iterator +from pathlib import Path +from threading import Thread +from typing import Any + +import pytest +from playwright.sync_api import Page, expect +from sqlalchemy import text + +from app.config import Settings +from app.db import SessionLocal +from app.rag.importer import ImportSummary, import_sources +from app.rag.llm import LLMClient +from e2e.auth_helpers import login +from e2e.mock_llm import long_answer + +REPO = Path(__file__).resolve().parents[2] +FIXTURES = REPO / "tests" / "fixtures" / "docs" +QUESTION = "How is my Kubernetes cluster set up?" +MOCK_ANSWER_MARKER = "Deterministic mock answer for E2E" +STORAGE_KEY = "bor.chat.v1" + +# --- scenario 1: a turn that is guaranteed to still be in flight -------- +#: The mock's long-answer trigger (~9s content stream at 0.02s/frame). +LONG_QUESTION = "write a long answer about how my kubernetes cluster is set up" +FULL_LONG = long_answer() +#: The mock's first 12-char content slice (the same cut ``_sse_stream`` +#: makes) — the stored partial must START with it (raw text, pre-render). +FIRST_CHUNK_RAW = re.findall(r".{1,12}", FULL_LONG, re.S)[0] +#: The rendered form of those first frames: the shared escape-first +#: markdown renderer converts the "1. " numbered line into a list item, +#: dropping the marker (pinned by test_long_answers). +FIRST_LINE_DOM = "Step 1: configure node-1" + +# --- scenario 2: navigation during pure thinking (no answer tokens) ----- +HESITATE_QUESTION = ( + "think out loud then hesitate — how is my kubernetes cluster set up?" +) +#: Tail of the mock's deterministic scratchpad (mock_llm.compose_thinking) +#: — when it is rendered, the thinking stream has just ended and the 4s +#: pre-content pause (SLOW_PRETOKEN_TRIGGER) is running. +THINKING_TAIL = "nothing is invented" + +#: Phase-10 viewer URL + phase-13 back=/ (byte-identical to the chip the +#: persistence suite pins — grounded-turn sources are unchanged by 20). +CHIP_HREF = "/document.html?source=docs&path=homelab%2Fkubernetes.md&back=%2F" + + +async def _import_fixtures(mock_port: int) -> ImportSummary: + kwargs: dict[str, Any] = {"_env_file": None, "llm_base_url": f"http://127.0.0.1:{mock_port}/v1"} + settings = Settings(**kwargs) # pyright: ignore[reportCallIssue] + return await import_sources([FIXTURES], LLMClient(settings)) + + +def _run_in_thread(coro: Any) -> Any: + """Run a coroutine on a worker thread. + + Playwright's sync API keeps an asyncio loop running on the test thread, + so ``asyncio.run`` cannot be called directly from a test body. + """ + box: dict[str, Any] = {} + + def runner() -> None: + try: + box["value"] = asyncio.run(coro) + except BaseException as e: # noqa: BLE001 — re-raised on the test thread + box["error"] = e + + t = Thread(target=runner) + t.start() + t.join() + if "error" in box: + raise box["error"] + return box["value"] + + +def _reset_db(mock_port: int, seed: bool) -> ImportSummary | None: + """Truncate the KB (and query log), then optionally re-import fixtures.""" + with SessionLocal() as db: + db.execute(text("TRUNCATE chunks, documents, query_log")) + db.commit() + if not seed: + return None + return _run_in_thread(_import_fixtures(mock_port)) + + +def _stored(page: Page) -> str | None: + """Raw localStorage payload for the chat (None when the key is absent).""" + return page.evaluate(f"() => localStorage.getItem('{STORAGE_KEY}')") + + +def _stored_parsed(page: Page) -> dict[str, Any]: + raw = _stored(page) + assert raw is not None, "the conversation key must exist in localStorage" + return json.loads(raw) + + +def _ask(page: Page, question: str) -> None: + """Send one turn and wait until the grounded answer has fully landed.""" + page.fill("#message-input", question) + page.click("#send-btn") + expect(page.locator(".msg.user .bubble").last).to_contain_text(question) + expect(page.locator(".msg.brain .bubble").last).to_contain_text( + MOCK_ANSWER_MARKER, timeout=30_000 + ) + expect(page.locator("#send-btn")).to_be_enabled() + expect(page.locator("#send-label")).to_have_text("Send") + + +def _no_error_banner(page: Page) -> None: + """The never-stale contract: a restored/partial state must never + present an error banner (role=alert) — the turn is simply partial.""" + expect(page.locator('[role="alert"]')).to_have_count(0) + + +@pytest.fixture() +def seeded_kb(mock_llm: int, db_ready: None) -> Iterator[None]: + """A fresh KB seeded from ``tests/fixtures/docs`` (8 docs, A9 formats), + truncated again on teardown. ``db_ready`` (conftest) skips with clear + instructions when Postgres is down.""" + summary = _reset_db(mock_llm, seed=True) + assert summary is not None and summary.added == 8 + yield + _reset_db(mock_llm, seed=False) + + +# --------------------------------------------------------------------------- +# 1. Mid-stream navigation via the Sources nav link: the partial answer +# that had already streamed is persisted and restored +# --------------------------------------------------------------------------- + + +def test_partial_answer_survives_sources_nav_midstream( + page: Page, app_url: str, seeded_kb: None +) -> None: + page.set_default_timeout(30_000) + # Admin (phase 16/19): only the admin sees the #nav-sources link the + # bug report clicks. + login(page, app_url, next="/") + expect(page.locator("#nav-sources")).to_be_visible() + + # Start the ~9s long answer. + page.fill("#message-input", LONG_QUESTION) + page.click("#send-btn") + expect(page.locator(".msg.user .bubble").last).to_contain_text(LONG_QUESTION) + + # Wait until the first streamed frames have rendered (the first line, + # in its list-rendered form) — the turn is now provably mid-stream. + bubble = page.locator(".msg.brain .bubble").last + expect(bubble).to_contain_text(FIRST_LINE_DOM, timeout=30_000) + # The turn is still in flight (the stream runs ~9s; navigation takes + # well under that). + expect(page.locator("#send-btn")).to_be_disabled() + + # THE BUG REPORT, VERBATIM: click "Sources" while chat is generating. + page.click("#nav-sources") + expect(page).to_have_url(app_url + "/sources.html") + # The navigation really landed on the admin catalog (mid-stream state + # of the stream itself does not matter to the page — the fetch is + # aborted by the unload, which is the point). + expect(page.locator("#docs-tbody tr").first).to_be_visible(timeout=15_000) + + # Return to the chat. + page.goto(app_url + "/") + + # The question AND the already-streamed partial answer are both + # rendered — no empty state, no error banner. + expect(page.locator("#empty-state")).to_be_hidden() + expect(page.locator(".msg.user .bubble")).to_have_count(1) + expect(page.locator(".msg.user .bubble").first).to_contain_text(LONG_QUESTION) + restored = page.locator(".msg.brain .bubble") + expect(restored).to_have_count(1) + expect(restored.first).to_contain_text(FIRST_LINE_DOM) + _no_error_banner(page) + + # Storage holds the partial as a plain brain message: raw text starting + # with the first streamed chunk — and SHORTER than the full answer + # (navigation landed mid-stream), with no done metadata (no + # sources/deflected/suggestions/thinking: this turn had none and a + # partial never carries the done fields). + msgs = _stored_parsed(page)["messages"] + assert [m["who"] for m in msgs] == ["user", "brain"] + assert msgs[0]["text"] == LONG_QUESTION + brain = msgs[1] + assert brain["text"].startswith(FIRST_CHUNK_RAW) + assert len(brain["text"]) < len(FULL_LONG), "the stored answer must be partial" + assert brain["text"] != FULL_LONG + assert "sources" not in brain + assert "deflected" not in brain + assert "suggestions" not in brain + assert "thinking" not in brain + + # The partial renders like any brain message (the existing + # bubble contract — no new surface) and is tunable like one. + expect(page.locator("details.thinking")).to_have_count(0) + expect(page.locator(".msg.brain .source-chip")).to_have_count(0) + + +# --------------------------------------------------------------------------- +# 2. Navigation BEFORE the first answer token (pure thinking): nothing +# brain-side is persisted — the question comes back alone +# --------------------------------------------------------------------------- + + +def test_no_orphan_brain_message_when_navigated_before_first_token( + page: Page, app_url: str, seeded_kb: None +) -> None: + page.set_default_timeout(30_000) + page.goto(app_url) + page.fill("#message-input", HESITATE_QUESTION) + page.click("#send-btn") + expect(page.locator(".msg.user .bubble").last).to_contain_text(HESITATE_QUESTION) + + # The mock streamed the ~800-char scratchpad (phase-17 thinking + # frames); wait until its tail is rendered — the 4s pre-content pause + # (SLOW_PRETOKEN_TRIGGER) is now running, so the navigation below + # lands inside pure thinking with a wide margin. + thinking = page.locator(".msg.brain").last.locator("details.thinking") + thinking.wait_for(state="attached", timeout=10_000) + expect(thinking.locator(".thinking-text")).to_contain_text(THINKING_TAIL) + # Still pre-token: the button is busy with the Thinking state. + expect(page.locator("#send-btn")).to_be_disabled() + expect(page.locator("#send-label")).to_have_text("Thinking…") + + # Leave during the pause (no answer token has streamed — acc is empty, + # so the pagehide save point must persist nothing brain-side). + page.goto(app_url + "/sources.html") + + # Return to the chat. + page.goto(app_url + "/") + + # The question is restored — with NO brain message behind it: no empty + # bubble, no partial, no thinking block (owner-confirmed A1.2). + expect(page.locator("#empty-state")).to_be_hidden() + expect(page.locator(".msg.user .bubble")).to_have_count(1) + expect(page.locator(".msg.user .bubble").first).to_contain_text(HESITATE_QUESTION) + expect(page.locator(".msg")).to_have_count(1) + expect(page.locator(".msg.brain")).to_have_count(0) + expect(page.locator("details.thinking")).to_have_count(0) + _no_error_banner(page) + + # Storage agrees: exactly the user message, nothing brain-side. + msgs = _stored_parsed(page)["messages"] + assert len(msgs) == 1 + assert msgs[0] == {"who": "user", "text": HESITATE_QUESTION} + + +# --------------------------------------------------------------------------- +# 3. Completed turn: the done save point is byte-identical to before — +# the new pagehide save point must not duplicate or alter it +# --------------------------------------------------------------------------- + + +def test_completed_turn_unaffected( + page: Page, app_url: str, seeded_kb: None +) -> None: + page.set_default_timeout(30_000) + page.goto(app_url) + _ask(page, QUESTION) + + # The done save point: full answer + metadata, exactly as phase 14. + before = _stored_parsed(page) + assert [m["who"] for m in before["messages"]] == ["user", "brain"] + brain = before["messages"][1] + assert MOCK_ANSWER_MARKER in brain["text"] + assert brain["deflected"] is False + assert any(s["path"] == "homelab/kubernetes.md" for s in brain["sources"]) + + # A trip to Sources and back (the turn finished long ago — uiState is + # idle, so the pagehide save point must be a no-op). + page.goto(app_url + "/sources.html") + page.goto(app_url + "/") + + # Full answer + source chip rendered; no error banner. + expect(page.locator("#empty-state")).to_be_hidden() + expect(page.locator(".msg.user .bubble")).to_contain_text(QUESTION) + bubble = page.locator(".msg.brain .bubble") + expect(bubble).to_have_count(1) + expect(bubble.first).to_contain_text(MOCK_ANSWER_MARKER) + chip = page.locator(".msg.brain .source-chip", has_text="kubernetes.md") + expect(chip).to_have_count(1) + expect(chip.first).to_have_attribute("href", CHIP_HREF) + _no_error_banner(page) + + # Storage is byte-identical to the pre-navigation payload — the + # completed turn persisted exactly as before (one brain message, done + # metadata intact; no duplicate from the pagehide path). + assert _stored_parsed(page) == before + + +# --------------------------------------------------------------------------- +# 4. The DELIBERATE clear is untouched: New Chat from the sources page +# still clears the conversation (phase 14/19 contract) +# --------------------------------------------------------------------------- + + +def test_new_chat_still_clears_conversation( + page: Page, app_url: str, seeded_kb: None +) -> None: + page.set_default_timeout(30_000) + page.goto(app_url) + _ask(page, QUESTION) + assert _stored(page) is not None + + # To the sources page (anonymous is fine — the shared bar carries the + # New Chat control regardless of auth, phase 19). + page.goto(app_url + "/sources.html") + new_chat = page.locator("#new-chat-btn") + expect(new_chat).to_be_visible() + new_chat.click() + + # "New chat" on a non-chat page means "go to the chat, fresh": the + # land page shows the empty state and the conversation key is GONE — + # the deliberate clearChatStorage() is unaffected by the phase-20 + # pagehide save point. + expect(page).to_have_url(app_url + "/") + expect(page.locator("#empty-state")).to_be_visible() + expect(page.locator(".msg")).to_have_count(0) + assert _stored(page) is None, "New Chat must clear the localStorage key" + _no_error_banner(page) diff --git a/tests/unit/test_sources_midstream.py b/tests/unit/test_sources_midstream.py new file mode 100644 index 0000000..4722e08 --- /dev/null +++ b/tests/unit/test_sources_midstream.py @@ -0,0 +1,151 @@ +"""Unit: the navigate-away partial-persistence contract (phase 20). + +The browser behavior is E2E-covered (tests/e2e/test_sources_midstream_bug.py); +here we pin the source-level wiring in app.js — the single `pagehide` +listener, its in-flight guard (uiState thinking/streaming + non-empty acc), +the `rememberBrainTurn(acc, { thinking: thinkingAcc || undefined })` reuse +(no duplicated storage code), the turn-scoped `persistedOnLeave` idempotency +flag (declared at module scope, reset per turn in the turn handler), the +updated persistence-block comment — so a silent regression is caught without +a browser. The deliberate New Chat clear (clearChatStorage in header.js, +phase 14/19) must stay the only other clear: untouched. +""" +from __future__ import annotations + +import re +from pathlib import Path + +FRONTEND = Path(__file__).resolve().parents[2] / "frontend" +ASSETS = FRONTEND / "assets" +APP_JS = ASSETS / "app.js" +HEADER_JS = ASSETS / "header.js" + + +def _js() -> str: + assert APP_JS.is_file() + return APP_JS.read_text(encoding="utf-8") + + +def _pagehide_body(js: str) -> str: + """The body of the single `pagehide` handler in app.js.""" + m = re.search(r'window\.addEventListener\("pagehide", \(\) => \{([\s\S]*?)\n\}\);', js) + assert m, "app.js must register a window `pagehide` handler" + return m.group(1) + + +def test_exactly_one_pagehide_listener_registered() -> None: + """One and only one `pagehide` listener — the navigate-away save + point (phase 20); no duplicated registration (bfcache churn is + handled by the idempotency flag, not a second listener).""" + js = _js() + assert js.count('addEventListener("pagehide"') == 1 + + +def test_pagehide_guard_requires_in_flight_state_and_streamed_text() -> None: + """The handler persists ONLY when a turn is in flight (uiState is + `thinking` or `streaming`) and answer text has already streamed + (non-empty acc). Thinking-only turns persist nothing brain-side: + the question is already saved on send and the user can re-ask.""" + body = _pagehide_body(_js()) + assert "UI_STATE.thinking" in body and "UI_STATE.streaming" in body, ( + "the guard must reference both in-flight states" + ) + idx_state = body.find("uiState !== UI_STATE.thinking && uiState !== UI_STATE.streaming") + assert idx_state != -1, "the in-flight guard is missing" + idx_acc = body.find("if (!acc) return;") + assert idx_acc != -1, "the non-empty-acc guard is missing" + assert idx_state < idx_acc, "state guard must run before the acc guard" + + +def test_pagehide_reuses_remember_brain_turn_with_thinking() -> None: + """The partial is persisted through the EXISTING save-point helper — + raw text, optional thinking field (`undefined` drops the key from the + JSON), no sources/deflection: a plain brain message that restore + re-renders exactly like a completed answer (no '(partial)' marker).""" + js = _js() + body = _pagehide_body(js) + assert "rememberBrainTurn(acc, { thinking: thinkingAcc || undefined });" in body + # No duplicated storage code: the handler pushes nothing itself. + assert "conversation.push" not in body + assert "saveConversation()" not in body + # The partial carries no done metadata. + assert "deflected" not in body and "sources" not in body + + +def test_persisted_on_leave_flag_is_module_scoped_and_turn_reset() -> None: + """`persistedOnLeave` makes the save point idempotent (a second + pagehide / bfcache store+restore never appends the same partial + twice): checked first, set true immediately before the persist + call. It is declared at module scope (the handler reads it) and + reset to false at the top of the turn handler — turn-scoped like + the other turn locals. acc / thinkingAcc were hoisted the same way + (no behavior change: same reset point, same names).""" + js = _js() + # Module-scope declarations (column 0). + assert re.search(r"^let persistedOnLeave = false", js, re.M) + assert re.search(r"^let acc = \"\"", js, re.M) + assert re.search(r"^let thinkingAcc = \"\"", js, re.M) + # Each hoisted local is declared exactly once (module scope only — + # the turn handler assigns, never re-declares). + assert js.count("let acc = \"\"") == 1 + assert js.count("let thinkingAcc = \"\"") == 1 + assert js.count("let persistedOnLeave") == 1 + + body = _pagehide_body(js) + idx_check = body.find("if (persistedOnLeave) return;") + idx_set = body.find("persistedOnLeave = true;") + idx_call = body.find("rememberBrainTurn(acc,") + assert -1 < idx_check < idx_set < idx_call, ( + "flag check first, set immediately before the persist call" + ) + + # Reset at the top of the turn handler (handleSend) — before the + # turn's fetch, where the other turn locals are initialized. + send = js.find("async function handleSend") + assert send != -1 + top = js[send : send + 1500] + assert "persistedOnLeave = false;" in top, ( + "persistedOnLeave must be reset per turn, at the top of the turn handler" + ) + assert "acc = \"\";" in top + assert "thinkingAcc = \"\";" in top + assert "let acc" not in top and "let thinkingAcc" not in top, ( + "the turn handler must assign the hoisted locals, not re-declare them" + ) + assert "let persistedOnLeave" not in top + + +def test_persistence_comment_lists_the_pagehide_save_point() -> None: + """The phase-14 persistence block comment now lists THREE save + points: user message on send, brain message on `done`, and the + PARTIAL brain message on navigate-away (`pagehide`, phase 20).""" + js = _js() + comment_start = js.find("conversation persistence (phase 14)") + key_idx = js.find('const STORAGE_KEY = "bor.chat.v1"') + assert -1 < comment_start < key_idx + comment = js[comment_start:key_idx] + assert re.search(r"Save points:.*?pagehide", comment, re.S), ( + "the Save points sentence must list the pagehide save point" + ) + assert "PARTIAL brain" in comment + + +def test_new_chat_clear_is_untouched_and_still_the_only_deliberate_clear() -> None: + """clearChatStorage (header.js) — the deliberate New Chat clear from + the non-chat pages — is untouched and remains the only place that + removes the literal key string; app.js's own clear goes through + STORAGE_KEY (#new-chat-btn, phase 14/19), and nothing new was added.""" + header = HEADER_JS.read_text(encoding="utf-8") + fn = header.find("function clearChatStorage") + assert fn != -1 + body = header[fn : header.find("\n}", fn)] + assert 'localStorage.removeItem("bor.chat.v1")' in body + assert "try" in body and "catch" in body + # No page script clears the key via the literal string. + for name in ("app.js", "sources.js", "document.js", "login.js"): + text = (ASSETS / name).read_text(encoding="utf-8") + assert 'removeItem("bor.chat.v1")' not in text, ( + f"{name}: clearChatStorage in header.js is the only literal-key clear" + ) + # app.js still has exactly one clear (clearStoredConversation, STORAGE_KEY). + assert _js().count("removeItem(STORAGE_KEY)") == 1