fix(ui): thinking window no longer scrolls — live 320px view pinned to the stream tail
This commit is contained in:
@@ -1,122 +0,0 @@
|
||||
# Phase 20 — Sources Mid-Stream: an in-flight answer is not lost on navigation
|
||||
|
||||
**Source:** `TODO.md` L3 — *"Clicking "sources" while chat is generating
|
||||
clears chat and result will never show up"*
|
||||
**Story:** `.agent/user_stories/sources-midstream.md` (created by task 02)
|
||||
**Context:** `frontend/assets/app.js` — the phase-14 persistence block
|
||||
(`STORAGE_KEY = "bor.chat.v1"`, `conversation`, `saveConversation`,
|
||||
`rememberBrainTurn`), the turn state machine (`UI_STATE.thinking` /
|
||||
`.streaming`), the streaming accumulators (`acc` / `thinkingAcc` /
|
||||
`sawThinking`), and the phase-14 restore path; `frontend/index.html`
|
||||
(`#nav-sources` link); `frontend/assets/header.js` (`clearChatStorage` —
|
||||
the deliberate New-Chat clear, NOT this bug).
|
||||
|
||||
## Objective
|
||||
When the user leaves the chat page (the Sources nav link, the document
|
||||
viewer, any link) while a turn is still in flight, the answer generated so
|
||||
far must not vanish. Today the brain message is persisted only on `done`,
|
||||
so navigating away aborts the stream and the partial answer is lost — the
|
||||
user returns to their own question with no result, ever. After this phase,
|
||||
returning to the chat shows the question **and** the partial answer that
|
||||
had already streamed (rendered like any brain message, thinking block
|
||||
restored if any).
|
||||
|
||||
## Owner-confirmed (2026-08-24, roadmap A1)
|
||||
1. **A partial answer is persisted as a plain brain message** — no
|
||||
"(partial)" marker, no sources/suggestions (the turn is dead; the user
|
||||
can re-ask for the full answer).
|
||||
2. Navigation **before the first answer token** (pure thinking) persists
|
||||
nothing brain-side: the question is restored, no empty/partial bubble.
|
||||
3. The deliberate **New Chat** `clearChatStorage()` (sources/viewer pages)
|
||||
is untouched — that clear is by design (phase 14/19).
|
||||
4. No server-side resume (A10 stays stateless) and no
|
||||
"leave page?" confirmation dialog.
|
||||
|
||||
## Design
|
||||
- **`app.js` — one new `pagehide` handler** (`window.addEventListener(
|
||||
"pagehide", …)` — fires on navigate-away and bfcache store):
|
||||
- Guard: only when a turn is in flight (current `uiState` is
|
||||
`UI_STATE.thinking` or `UI_STATE.streaming`) **and** `acc` is
|
||||
non-empty.
|
||||
- Action: `rememberBrainTurn(acc, { thinking: thinkingAcc ||
|
||||
undefined })` — reuse the existing save-point helper, so the partial
|
||||
text is stored raw (the restore path re-renders through the
|
||||
escape-first markdown renderer; the phase-17 `thinking` field
|
||||
restores the collapsed Thinking block).
|
||||
- **Idempotency guard:** a turn-local `persistedOnLeave` flag so a
|
||||
second `pagehide` (or bfcache store+restore churn) never appends the
|
||||
same partial message twice. The `done` save point is unaffected
|
||||
(navigation means the stream is dead; if the user returns via
|
||||
bfcache the turn is already aborted by the unloading page).
|
||||
- **Restore path:** unchanged — a stored partial message is a well-formed
|
||||
brain message and renders exactly like a completed one (minus
|
||||
sources/deflection, which it simply doesn't carry).
|
||||
- **Non-goals:** no resume of the SSE stream, no API changes, no changes
|
||||
to the New Chat buttons, sign-out, or the document-viewer back link.
|
||||
|
||||
## Dependencies
|
||||
- `14_chat_persistence` (complete) — `bor.chat.v1` shape, save points,
|
||||
restore, and the `rememberBrainTurn` helper this phase reuses.
|
||||
- `17_thinking_display` (complete) — the `thinking` field on persisted
|
||||
brain messages and the `thinkingAcc` accumulator.
|
||||
- `19_shared_header` (complete) — the `#nav-sources` link (admin-only)
|
||||
the bug report clicks.
|
||||
- `18_follow_bottom_scroll` (complete) — no overlap (scroll gating only).
|
||||
|
||||
## Tasks
|
||||
1. `01_persist_inflight_turn.md` — the `pagehide` partial-persistence
|
||||
handler in `app.js` + source-level unit pins.
|
||||
2. `02_e2e_story_suite_commit.md` — `tests/e2e/test_sources_midstream_bug.py`
|
||||
(the story gate, isolated), regression suites, story file, final
|
||||
validation, the single atomic commit, phase move to `complete/`.
|
||||
|
||||
## Locked decisions
|
||||
- **A10 untouched** — API stays stateless; no resume. **A11 untouched** —
|
||||
vanilla JS, no CDN. **A16 honored** — one new story E2E suite +
|
||||
adapted regressions. No anchor changed.
|
||||
|
||||
## Testing & Quality
|
||||
- **Unit (source-level, new `tests/unit/test_sources_midstream.py`,
|
||||
following the repo's source-pin pattern):** `app.js` registers a
|
||||
`pagehide` listener; the guard references the in-flight `uiState` and a
|
||||
non-empty `acc`; the partial path calls `rememberBrainTurn` with
|
||||
`thinking: thinkingAcc || undefined`; a turn-local idempotency flag
|
||||
exists; `STORAGE_KEY`/save-point comments updated to list the new
|
||||
save point.
|
||||
- **Integration:** none (no `app/` changes) — the `uv run pytest
|
||||
--cov=app` number must stay at today's.
|
||||
- **Coverage:** frontend-only; the >90% `app/` gate is unaffected,
|
||||
re-run to prove it.
|
||||
- **E2E:** `tests/e2e/test_sources_midstream_bug.py` (task 02), green
|
||||
**in isolation** (prereq `podman compose up -d db`).
|
||||
- **Lint/types:** `uv run ruff check . && uv run pyright` clean.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] Admin, mid-stream, clicks **Sources** → returns to `/`: the
|
||||
question **and** the already-streamed partial answer are both
|
||||
rendered; no error banner; `bor.chat.v1` holds the partial brain
|
||||
message.
|
||||
- [ ] Navigate away before the first token → back: question restored,
|
||||
no empty/partial brain bubble.
|
||||
- [ ] A completed turn is persisted exactly as before (sources,
|
||||
deflection, suggestions intact).
|
||||
- [ ] New Chat from the sources page still clears the conversation.
|
||||
- [ ] `uv run pytest` green; `uv run pytest --cov=app
|
||||
--cov-report=term-missing` ≥ today's number.
|
||||
- [ ] `uv run pytest tests/e2e/test_sources_midstream_bug.py -v --no-cov`
|
||||
green in isolation; regressions green in isolation (one command
|
||||
each): `test_chat_persistence.py`, `test_thinking_display.py`,
|
||||
`test_shared_header.py`.
|
||||
- [ ] `uv run ruff check . && uv run pyright` clean.
|
||||
- [ ] UI Structure Check (AGENTS.md rule 5): no new UI surface — the
|
||||
restored partial renders through the existing bubble/thinking
|
||||
contract.
|
||||
- [ ] `.agent/user_stories/sources-midstream.md` exists.
|
||||
- [ ] One `--no-gpg-sign` commit (below);
|
||||
`.agent/phases/todo/20_sources_midstream_bug/` moved to
|
||||
`.agent/phases/complete/`.
|
||||
|
||||
## Commit
|
||||
```bash
|
||||
git add -A .agent/ frontend/ tests/ && git commit --no-gpg-sign -m "fix(chat): keep the in-flight answer when navigating away mid-turn — partial answer restored on return"
|
||||
```
|
||||
@@ -1,76 +0,0 @@
|
||||
# Task 02 — E2E story suite, story file, validation, commit
|
||||
|
||||
**Phase:** `20_sources_midstream_bug` · **Source:** `TODO.md` L3
|
||||
|
||||
## Objective
|
||||
The story gate: `tests/e2e/test_sources_midstream_bug.py` proves the bug
|
||||
is fixed end-to-end (navigate away mid-stream, come back, the partial
|
||||
answer is there), plus the regression suites, the story file, final
|
||||
validation, and the single atomic commit.
|
||||
|
||||
## Work
|
||||
1. `tests/e2e/test_sources_midstream_bug.py` (new — mirror
|
||||
`test_chat_persistence.py`'s scaffolding: fixture import via
|
||||
`_import_fixtures`/`_run_in_thread`, mock LLM on a thread, `login`
|
||||
from `e2e.auth_helpers`). The mock LLM must stream **slowly enough**
|
||||
that the turn is still in flight when the test navigates (reuse the
|
||||
streaming pattern from `test_thinking_display.py` / `mock_llm.py`;
|
||||
tune the per-chunk delay until the navigation lands mid-stream).
|
||||
Tests (Playwright Mapping Rule — one per numbered scenario):
|
||||
1. `test_partial_answer_survives_sources_nav_midstream` — admin
|
||||
(`login(page, app_url, next="/")`), send a question, wait for the
|
||||
first streamed chunk to render (expect the first chunk's text in
|
||||
the answer bubble), **click `#nav-sources`** (the actual nav link —
|
||||
admin sees it), land on `/sources.html`, then `page.goto("/")`:
|
||||
expect the question text AND the first-chunk text present, no
|
||||
`role="alert"` banner. Read `localStorage` `bor.chat.v1`: the
|
||||
messages contain a brain message whose text starts with the first
|
||||
chunk.
|
||||
2. `test_no_orphan_brain_message_when_navigated_before_first_token` —
|
||||
mock streams a `thinking` event, then a long pre-token pause;
|
||||
navigate (direct `page.goto("/sources.html")` is fine here) during
|
||||
the pause, return to `/`: the question is present, exactly one
|
||||
user message and **zero** brain messages in both the DOM and
|
||||
`bor.chat.v1`.
|
||||
3. `test_completed_turn_unaffected` — a turn that finishes normally
|
||||
(`done`), then navigate to sources and back: full answer, sources
|
||||
chips, and the done-metadata (sources array) intact in storage.
|
||||
4. `test_new_chat_still_clears_conversation` — regression: completed
|
||||
turn → `/sources.html` → click the sources-page New Chat button →
|
||||
lands on `/` with the empty state and `bor.chat.v1` removed.
|
||||
2. `.agent/user_stories/sources-midstream.md` (new) — the short story
|
||||
file matching the repo's story format (goal, the bug report verbatim
|
||||
from `TODO.md` L3, the owner-confirmed A1 decisions from
|
||||
`00_phase.md`, the E2E mapping table test-name → scenario).
|
||||
3. Run the suite **in isolation** (prereq `podman compose up -d db`):
|
||||
`uv run pytest tests/e2e/test_sources_midstream_bug.py -v --no-cov`.
|
||||
4. Regressions, in isolation, one command each (all must stay green):
|
||||
- `uv run pytest tests/e2e/test_chat_persistence.py -v --no-cov`
|
||||
- `uv run pytest tests/e2e/test_thinking_display.py -v --no-cov`
|
||||
- `uv run pytest tests/e2e/test_shared_header.py -v --no-cov`
|
||||
5. Final validation: `uv run pytest` green; `uv run pytest --cov=app
|
||||
--cov-report=term-missing` ≥ today's number (>90% gate);
|
||||
`uv run ruff check . && uv run pyright` clean.
|
||||
6. **UI Structure Check** (AGENTS.md rule 5): the restored partial
|
||||
renders through the existing bubble/thinking contract — no new
|
||||
surface, no new ids, focus/contrast unchanged.
|
||||
7. Write the phase report
|
||||
(`.agent/reports/20_sources_midstream_bug/` — what changed, E2E
|
||||
results, the manual-smoke note from task 01).
|
||||
8. Commit (one atomic commit) and move the phase:
|
||||
```bash
|
||||
git add -A .agent/ frontend/ tests/
|
||||
git commit --no-gpg-sign -m "fix(chat): keep the in-flight answer when navigating away mid-turn — partial answer restored on return"
|
||||
mv .agent/phases/todo/20_sources_midstream_bug .agent/phases/complete/
|
||||
```
|
||||
|
||||
## Testing & Quality
|
||||
- Story suite green **in isolation**; the three regression suites green
|
||||
in isolation; full unit+integration suite green; `app/` coverage at or
|
||||
above today's number (>90%); ruff + pyright clean.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] `test_sources_midstream_bug.py` 4/4 in isolation.
|
||||
- [ ] Regressions (persistence, thinking display, shared header) green.
|
||||
- [ ] Story file + phase report exist.
|
||||
- [ ] One `--no-gpg-sign` commit; phase directory in `complete/`.
|
||||
@@ -1,57 +0,0 @@
|
||||
# Task 01 — styles.css: `.thinking-text` overflow hidden (no user scroll)
|
||||
|
||||
**Phase:** `21_thinking_no_scroll` · **Source:** `TODO.md` L4 —
|
||||
*"Disable scroll in the thinking window. Users don't need to scroll back
|
||||
through thinking, just see it live."*
|
||||
|
||||
## Objective
|
||||
One CSS property change makes the Thinking window a live-tail-only view:
|
||||
`overflow-y: hidden` instead of `auto`, keeping the 320px clip. The
|
||||
phase-17 JS bottom-pin (which keeps working under `overflow: hidden`) is
|
||||
the sole scroller.
|
||||
|
||||
## Work
|
||||
1. `frontend/assets/styles.css` — in the phase-17 thinking block
|
||||
section (~line 444):
|
||||
```css
|
||||
details.thinking .thinking-text {
|
||||
padding: 0 0.75rem 0.75rem;
|
||||
color: var(--ink-soft); /* 6.9:1 on --surface */
|
||||
font-size: 0.875rem;
|
||||
line-height: 1.55;
|
||||
max-height: 320px;
|
||||
overflow-y: hidden; /* no scroll back (owner choice 2026-08-24):
|
||||
the window is a live tail only — the phase-17
|
||||
JS bottom-pin (scrollTop = scrollHeight per
|
||||
chunk) is the sole scroller */
|
||||
}
|
||||
```
|
||||
(Only the `overflow-y` value + comment change; every other declaration
|
||||
stays byte-identical.)
|
||||
2. `frontend/assets/app.js` — **no change expected.** Verify the pin is
|
||||
intact: the streaming `thinking` branch still does
|
||||
`textEl.scrollTop = textEl.scrollHeight` on every chunk (~line 886).
|
||||
If (and only if) the pin were missing/broken, fix it — do not remove
|
||||
or alter any other scrolling behavior.
|
||||
3. `tests/unit/test_thinking_no_scroll.py` (new — repo source-pin
|
||||
pattern):
|
||||
- `styles.css`: the `details.thinking .thinking-text` rule contains
|
||||
`overflow-y: hidden` and `max-height: 320px` (no `overflow-y: auto`
|
||||
left in that rule).
|
||||
- `app.js`: the bottom-pin line
|
||||
`textEl.scrollTop = textEl.scrollHeight` is still present (the
|
||||
live-tail mechanism).
|
||||
4. Manual smoke (dev server): stream a long thinking turn; try to wheel /
|
||||
drag / Tab+ArrowDown inside the Thinking block — it must not move;
|
||||
the newest chunk is always the one visible at the bottom.
|
||||
|
||||
## Testing & Quality
|
||||
- `uv run pytest tests/unit/test_thinking_no_scroll.py -v` green.
|
||||
- `uv run ruff check . && uv run pyright` clean.
|
||||
|
||||
## Completion Criteria
|
||||
- [ ] `.thinking-text` is `overflow-y: hidden`, `max-height: 320px`, with
|
||||
the owner-choice comment.
|
||||
- [ ] The JS bottom-pin is verified intact (no app.js diff unless the
|
||||
pin was broken).
|
||||
- [ ] Unit pins green; lint/types clean; manual smoke passed.
|
||||
Reference in New Issue
Block a user