From aba8615177dfc077b9806583c5536c4573e5973d Mon Sep 17 00:00:00 2001 From: ducoterra Date: Sun, 30 Aug 2026 16:14:18 -0400 Subject: [PATCH] =?UTF-8?q?fix(chat):=20rest=20the=20composer=20at=20the?= =?UTF-8?q?=20viewport=20bottom=20=E2=80=94=20sticky=20alone=20left=20it?= =?UTF-8?q?=20mid-screen?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 52's first pass shipped `position: sticky; bottom` on `.composer` and called the phase done, but the owner's requirement — "the chat message-input textarea should be at the bottom of the screen" — still failed in the browser: on an empty/short chat the input rested just under the empty state (~57% of the viewport) with a dead band down to the footer. `position: sticky` can only pull a box UP toward the scrollport's bottom edge; it can never push a box DOWN to meet it, so on a page that does not overflow it is a no-op. The old story suite only exercised an overflowing conversation (one test even asserted the buggy resting position as expected), which is why the half-fix passed. - `.messages { flex: 1 1 auto }` — absorbs a short page's free space so the composer's resting in-flow position is the bottom of the full-height column (body min-height:100dvh -> .app-main flex:1 -> .chat-shell flex:1); basis stays `auto`, no height cap, no overflow — the document stays the scroller - `.composer { bottom: env(safe-area-inset-bottom, 0) }` — the explicit 0 fallback replaces the env()-only offset, which degraded to `auto` (no pin) wherever env() is unsupported - E2E: `test_empty_chat_composer_sits_in_normal_flow` -> `..._at_the_screen_bottom` (chrome-only band below the resting composer); the phone suite now checks the resting position as well as the pinned one - Unit pins: the flex-grow half and the full-height column are pinned, so the fix cannot silently regress to sticky-only Still CSS-only — no DOM change, no JS, no new scroll call site (phase 42 never-auto-scroll contract intact), no z-index. Verified: 1019 unit/integration tests pass (app/ coverage 99%), ruff and pyright clean; tests/e2e/test_pinned_composer.py green in isolation (4), plus the stop/autoscroll/persistence/mobile-nav suites and 14 layout/scroll neighbours green in isolation. --- .../complete/52_pinned_composer/00_phase.md | 74 ++++++++++++ .../52_pinned_composer/01_sticky_composer.md | 33 +++++- .../02_e2e_pinned_composer.md | 20 +++- .../todo/52_pinned_composer/00_phase.md | 41 ------- frontend/assets/styles.css | 39 ++++++- tests/e2e/test_pinned_composer.py | 105 +++++++++++++----- tests/unit/test_pinned_composer.py | 89 ++++++++++++--- 7 files changed, 300 insertions(+), 101 deletions(-) create mode 100644 .agent/phases/complete/52_pinned_composer/00_phase.md rename .agent/phases/{todo => complete}/52_pinned_composer/02_e2e_pinned_composer.md (67%) delete mode 100644 .agent/phases/todo/52_pinned_composer/00_phase.md diff --git a/.agent/phases/complete/52_pinned_composer/00_phase.md b/.agent/phases/complete/52_pinned_composer/00_phase.md new file mode 100644 index 0000000..d8b2274 --- /dev/null +++ b/.agent/phases/complete/52_pinned_composer/00_phase.md @@ -0,0 +1,74 @@ +# Phase 52 — Pinned Message Composer + +**Source:** `TODO.md` L3 — "The message input text box needs to be pinned to the bottom of the screen so it doesn't \"run away\" from the user as they try to click \"stop\"" +**Story:** n/a (TODO-derived — owner instruction 2026-08-30: convert without confirmation) +**Context:** The chat page (`frontend/index.html`) scrolls at the document level: `.chat-shell` (the centered 46rem column, PLAN §7) is a flex column — kb-banner, steering panel, New chat, Save/Share, `.messages`, and finally the `.composer` form (`#message-input` + `#send-btn`). The composer is NOT sticky — in a long conversation it sits below the fold, and since the page never auto-scrolls while a turn streams (phase 42), the Stop button (phase 48: `#send-btn` morphs into the enabled Stop control in flight) can be off-screen exactly when the user wants to click it. The sticky app header is the only sticky chrome (z-index 20, 2px hairline below); the document modal is the topmost layer (z-index 1000). House frontend testing: source pins (`tests/unit/test_frontend_feedback.py` style — `test_frontend_scroll.py` / `test_history_page.py` are the closest precedents) plus one isolated Playwright suite per story (A16). + +## Objective +The composer (input + Send/Stop button) sits at the bottom of the screen — on an EMPTY/short chat as its resting position and at every scroll position of an over-viewport conversation — so the Stop control is always reachable mid-turn without scrolling, and no new auto-scroll behaviour is introduced (the phase-42 contract stays intact). + +## Revision (owner, 2026-08-30) — the first pass did NOT complete this phase +The first pass shipped `position: sticky; bottom` on `.composer` only and +called the phase done. The owner rejected it: *"The chat message-input +textarea should be at the bottom of the screen. It's not right now."* +Verified in the browser: on an empty chat the input rested just under the +empty state (~57% of the viewport) with a dead band down to the footer. + +Why sticky alone cannot satisfy the objective: **`position: sticky` can +only pull a box UP toward the scrollport's bottom edge — it never pushes a +box DOWN to meet it.** So it works only while the document overflows +(which the old E2E suite tested, and which is why the suite went green on +a half-fixed feature); on a page that does not scroll it is a no-op. +The recorded assumption "the pin is CSS-only sticky, no other rule" was +the wrong assumption — flagged and revised here, not silently deviated. + +The pin is now TWO rules: +1. `.messages { flex: 1 1 auto }` — absorbs the free space of a short page + so the composer's resting (in-flow) position IS the bottom of the + full-height column (`body{min-height:100dvh}` → `.app-main{flex:1}` → + `.chat-shell{flex:1}` → grown message list). +2. `.composer { position: sticky; bottom: env(safe-area-inset-bottom, 0) }` + — takes over as soon as the conversation overflows, gluing the box (and + Stop) to the viewport's bottom edge at every scroll position; the `0` + fallback replaces the old `env()`-only offset, which degraded to + `auto` (no pin at all) where `env()` is unsupported. + +Both halves stay CSS-only: no DOM change, no JS, no new scroll call site, +no z-index — so the phase-42 never-auto-scroll contract still holds. The +E2E contract was corrected the same way: the empty-chat test is now +`test_empty_chat_composer_sits_at_the_screen_bottom` (the old +`sits_in_normal_flow` test asserted the buggy geometry as expected +behaviour), and the phone suite checks the resting position too. + +## Dependencies +- `48_stop_generation` (complete) — the Send↔Stop morph; Stop is clicked FROM the pinned composer (the original "run away" scenario). +- `42_no_reply_autoscroll` (complete) — the no-autoscroll-while-streaming contract the pin must not revise. +- `07_story_responsive_polish` (complete) — the 46rem column / responsive rules the pinned composer must sit within. + +## Tasks +1. `01_sticky_composer.md` — the `position: sticky; bottom` pin on `.composer` + safe-area inset + the frontend source pins. +2. `02_e2e_pinned_composer.md` — the story Playwright suite + regressions + commit. + +## Testing & Quality +- Unit: `tests/unit/test_pinned_composer.py` — source pins: `.composer` carries `position: sticky` with a `bottom` offset (safe-area inset) in `styles.css`; `app.js` gains NO new page-scroll call site (the phase-42 invariant — the one page scroll is still `scrollReveal`). +- Coverage: **>90%** on `app/` (validate.sh gate — this phase makes no `app/` changes; the gate must stay green). +- E2E (mandatory, A16): `tests/e2e/test_pinned_composer.py`, run in isolation. + +## Completion Criteria +- [x] On an EMPTY/short chat the composer's resting position is at the bottom of the screen — the only band below it is chrome (`.app-footer`, in flow, never overlapped); no dead wasted space. +- [x] With an over-viewport conversation, scrolled to the top: the composer is fully visible (bounding box inside the viewport) at the bottom edge. +- [x] In flight, scrolled up to read earlier content: the Stop button is visible and clickable; clicking it (no scrolling) stops the turn — partial kept + persisted with `stopped: true` (the phase-48 contract, unchanged), no error banner, no window scroll (phase 42). +- [x] `uv run pytest` green (1019 passed); coverage TOTAL 99% (>90%). +- [x] `uv run pytest tests/e2e/test_pinned_composer.py -v --no-cov` green in isolation (4 passed, DB up). +- [x] Regression E2E suites green in isolation: `test_stop_generation.py` (3), `test_no_reply_autoscroll.py` (6), `test_chat_persistence.py` (4), `test_mobile_hamburger_nav.py` (7) — plus the layout/scroll neighbours `test_smoke.py` (3), `test_chat_rag.py` (3), `test_honest_deflection.py` (3), `test_suggestion_chips.py` (4), `test_loading_feedback.py` (5), `test_long_answers.py` (2), `test_markdown_tables.py` (6), `test_retry_answer.py` (4), `test_thinking_scroll.py` (8), `test_responsive_polish.py` (7), `test_share_chat.py` (4), `test_chat_history.py` (5), `test_dark_tech_theme.py` (6), `test_background_no_motion.py` (8). +- [x] `uv run ruff check . && uv run pyright` clean. + +## Locked decisions +- **REVISION of assumption (1) (owner, 2026-08-30):** sticky alone was wrong — see **## Revision** above. The pin is `position: sticky; bottom: env(safe-area-inset-bottom, 0)` on `.composer` **plus** `flex: 1 1 auto` on `.messages`, so the resting position also lands at the bottom of the screen. Still CSS-only: no `index.html` DOM change, no JS. +- **Recorded assumptions (TODO conversion, 2026-08-30 — owner asked for no confirmation):** (1) ~~the pin is CSS-only — `position: sticky; bottom: env(safe-area-inset-bottom)` on the existing `.composer` inside the existing `.chat-shell` column~~ **revised, see above**; no `index.html` DOM change, no JS; (2) the composer keeps its current solid `--surface` background + border + shadow (no glass/transparency), so scrolled messages never show through it; (3) NO z-index change — the composer already paints above `.messages` by DOM order, never overlaps the sticky header, and stays under the z-1000 document modal; (4) the phase-42 never-auto-scroll contract is strictly upheld — the pin adds zero scroll call sites. +- **A16/A17 honoured** — one story E2E suite, one atomic commit. + +## Commit +```bash +git add -A .agent/ frontend/ tests/ && git commit --no-gpg-sign -m "feat(chat): pin the composer to the viewport bottom — Stop is always reachable while reading" +``` diff --git a/.agent/phases/complete/52_pinned_composer/01_sticky_composer.md b/.agent/phases/complete/52_pinned_composer/01_sticky_composer.md index 8dfcec5..e332f65 100644 --- a/.agent/phases/complete/52_pinned_composer/01_sticky_composer.md +++ b/.agent/phases/complete/52_pinned_composer/01_sticky_composer.md @@ -4,14 +4,37 @@ **Story:** n/a (TODO-derived) ## Objective -The composer stays pinned to the bottom of the viewport at every scroll position — a CSS-only change inside the existing chat column. +The composer stays at the bottom of the screen — resting there on a short page and pinned there at every scroll position of an over-viewport page — with CSS-only changes inside the existing chat column. + +## Revision (owner, 2026-08-30) — this task was NOT done the first time +The first pass applied only `position: sticky; bottom: env(safe-area-inset-` +`bottom)` to `.composer`, and the browser disproved it: on an empty/short +chat the input rested just under the empty state (~57% of the viewport) +with a dead band all the way down to the footer. `position: sticky` can +only pull a box UP to the scrollport's bottom edge; it never pushes a box +DOWN to meet it, so on a page that does not overflow it does nothing — the +old E2E contract only ever exercised an overflowing conversation, which is +why the half-fix passed. Both rules below are now in place and both are +source-pinned in `tests/unit/test_pinned_composer.py`. ## Work -1. `frontend/assets/styles.css` — in the `/* ---------- Composer ---------- */` block (`.composer`, ~L1133): add `position: sticky;` and `bottom: env(safe-area-inset-bottom);` to `.composer`. The page scrolls at the document level and `.chat-shell` is the composer's containing column, so the box sticks to the viewport's bottom edge (offset by the mobile safe-area inset) while `.messages` scrolls behind it; at the document bottom it settles back into its normal flow position above the footer. Keep the existing solid `background: var(--surface)`, border, radius and `box-shadow: var(--shadow)` — messages must never show through the pinned box. +1. `frontend/assets/styles.css` — TWO rules, both required: + a. `.messages { flex: 1 1 auto; }` (Main-frame block) — the message list + absorbs the free space of a short page, so the composer's resting + in-flow position is the bottom of the full-height column + (`body{min-height:100dvh}` → `.app-main{flex:1}` → `.chat-shell{flex:1}`). + `flex-basis` stays `auto` (a `0` basis would size the list below its + content once the conversation overflows and let bubbles overlap the + box); no `height` cap, no `overflow` — the document stays the scroller. + b. `.composer { position: sticky; bottom: env(safe-area-inset-bottom, 0); }` + (`/* ---------- Composer ---------- */` block, ~L1133) — takes over the + moment the conversation overflows. The page scrolls at the document level and `.chat-shell` is the composer's containing column, so the box sticks to the viewport's bottom edge (offset by the mobile safe-area inset) while `.messages` scrolls behind it; at the document bottom it settles back into its normal flow position above the footer. Keep the existing solid `background: var(--surface)`, border, radius and `box-shadow: var(--shadow)` — messages must never show through the pinned box. 2. `frontend/index.html` — verify NO change needed: the composer is already the LAST child of `.chat-shell` (the sticky context), and the `#message-input` / `#send-btn` / `#send-status` markup is untouched. 3. Do NOT touch `frontend/assets/app.js` — the pin must not add any scroll call site (phase-42 invariant; the one page scroll in the file stays `scrollReveal`). 4. `tests/unit/test_pinned_composer.py` (new, house pin style — see `tests/unit/test_frontend_scroll.py`): assert `styles.css` declares `position: sticky` AND a `bottom:` offset on `.composer` (the sticky-bottom pair, matched inside the `.composer` rule); assert `app.js` is unchanged in its scroll surface (the phase-42 single-`scrollReveal` pin still holds — reuse the same assertion approach `test_no_reply_autoscroll.py`'s companion pins use). -- ASSUMPTION: `bottom: env(safe-area-inset-bottom)` (not `bottom: 0` + extra padding) — the standard notch-aware inset; on desktop `env()` resolves to 0, so the box sits flush with the viewport bottom. +- ASSUMPTION (revised): `bottom: env(safe-area-inset-bottom, 0)` — the + notch-aware inset with an explicit `0` fallback; `env()`-only (the first + pass) degrades to `auto`, i.e. no pin, where the function is unsupported. - ASSUMPTION: no `z-index` added — DOM order already stacks the composer above `.messages`; the sticky header (z 20) and doc modal (z 1000) are unaffected. ## Testing & Quality @@ -19,5 +42,5 @@ The composer stays pinned to the bottom of the viewport at every scroll position - Coverage: **>90%** — no `app/` change; the gate stays green. ## Completion Criteria -- [ ] `.composer` in `styles.css` carries `position: sticky` + the `bottom` safe-area offset; the `frontend/` diff contains no JS change. -- [ ] `uv run pytest` green; `uv run ruff check . && uv run pyright` clean. +- [x] `.messages` carries the `flex-grow` and `.composer` carries `position: sticky` + the `bottom` safe-area offset with the `0` fallback; the `frontend/` diff contains no JS and no DOM change. +- [x] `uv run pytest` green (1019 passed, TOTAL 99%); `uv run ruff check . && uv run pyright` clean. diff --git a/.agent/phases/todo/52_pinned_composer/02_e2e_pinned_composer.md b/.agent/phases/complete/52_pinned_composer/02_e2e_pinned_composer.md similarity index 67% rename from .agent/phases/todo/52_pinned_composer/02_e2e_pinned_composer.md rename to .agent/phases/complete/52_pinned_composer/02_e2e_pinned_composer.md index a7f9242..3aeef1c 100644 --- a/.agent/phases/todo/52_pinned_composer/02_e2e_pinned_composer.md +++ b/.agent/phases/complete/52_pinned_composer/02_e2e_pinned_composer.md @@ -6,11 +6,23 @@ ## Objective One isolated Playwright story suite proving the composer never runs away from the user — including the original scenario: clicking **Stop** while reading a streaming answer from a scrolled-up position. +## Revision (owner, 2026-08-30) — the suite tested the half-fix +The first pass wrote `test_empty_chat_composer_sits_in_normal_flow`, which +asserted that on an empty chat the composer "renders in its normal flow +position" — i.e. it pinned the BUG as the expected result, so the suite +went green while the objective failed. It is now +`test_empty_chat_composer_sits_at_the_screen_bottom` (helper +`assert_rests_at_the_screen_bottom`): the band below the resting composer +may be chrome only (the footer's measured height + the settled slot's flow +padding, `BOTTOM_SLACK_PX`), and the composer must sit in the lower part of +the viewport (`LOWER_PART`). The phone suite checks the resting position as +well as the pinned one. + ## Work 1. `tests/e2e/test_pinned_composer.py` (new) — the story suite (isolated run; `mock_llm` deterministic; DB up per the e2e prerequisite): - **Pinned while reading:** build an over-viewport conversation — ask ~8 short questions through the UI (each turn adds user + brain bubbles with meta rows; at the house 1280×720 viewport this exceeds the fold; see the ASSUMPTION below for the fallback if it proves insufficient). `page.evaluate("window.scrollTo(0, 0)")` (a test scroll — the app never scrolls itself, phase 42). Assert `page.locator("#composer").bounding_box()` is fully inside the viewport (`y >= 0`, `y + height <= viewport height`) with its bottom edge at the viewport bottom (± a few px for the safe-area inset). - **The run-away scenario — Stop from scrolled-up, in flight:** submit one question; let the turn enter streaming (the mock LLM streams deltas; wait for the Send label to read "Stop" per the phase-48 contract); scroll the window to the top (the user reads earlier content — phase 42 leaves them there; record `window.scrollY`); assert the Stop button (`#send-btn`, `.is-stop`) is visible WITHOUT scrolling; click it; assert: the turn settled (no in-flight state), the partial answer is on screen with the `.stopped-note` rendered, no error banner, `window.scrollY` UNCHANGED by the click (the pin adds no scroll), and a fresh page load restores the `stopped` record (the phase-48 persistence contract through the normal `bor.chat.v1` path). - - **Natural bottom:** a fresh empty chat — `#composer`'s bounding box bottom is at or above the viewport bottom and the `.app-footer` is present in normal flow (the pin must not float the composer over the footer on a short page). + - **Natural bottom:** a fresh empty chat — the composer RESTS at the bottom of the screen (no dead band under it) while the `.app-footer` stays in normal flow below it, un-overlapped; one short turn still rests there. 2. Regressions, each in isolation (`uv run pytest tests/e2e/ -v --no-cov`): `test_stop_generation.py`, `test_no_reply_autoscroll.py`, `test_chat_persistence.py`, `test_mobile_hamburger_nav.py` (the mobile nav sits in the sticky header — the pin must not break the header/dropdown stacking at ≤640px). 3. One `--no-gpg-sign` commit staging `.agent/ frontend/ tests/` (message per the phase overview); move `.agent/phases/todo/52_pinned_composer/` to `.agent/phases/complete/`. - ASSUMPTION: over-viewport overflow is produced by ~8 UI questions against the mock LLM (short deterministic answers, but each turn adds two bubbles + meta rows). If the suite shows that is not enough to exceed 720px, fall back to a saved long conversation via the phase-50 path (Save a multi-turn chat, reload with `/?chat=`); no new fixture or API surface. @@ -20,6 +32,6 @@ One isolated Playwright story suite proving the composer never runs away from th - The four regression suites green in isolation (no assertion edits outside the scope the phase-48 revised contract already owns — if `test_stop_generation.py` needs a revision it must be the pinned-composer contract, nothing else). ## Completion Criteria -- [ ] `uv run pytest tests/e2e/test_pinned_composer.py -v --no-cov` green in isolation (DB up). -- [ ] `test_stop_generation.py`, `test_no_reply_autoscroll.py`, `test_chat_persistence.py`, `test_mobile_hamburger_nav.py` green in isolation. -- [ ] One `--no-gpg-sign` commit; phase dir moved to `.agent/phases/complete/`. +- [x] `uv run pytest tests/e2e/test_pinned_composer.py -v --no-cov` green in isolation (4 passed, DB up). +- [x] `test_stop_generation.py` (3), `test_no_reply_autoscroll.py` (6), `test_chat_persistence.py` (4), `test_mobile_hamburger_nav.py` (7) green in isolation — plus the layout/scroll neighbours (smoke 3, chat_rag 3, honest_deflection 3, suggestion_chips 4, loading_feedback 5, long_answers 2, markdown_tables 6, retry_answer 4, thinking_scroll 8, responsive_polish 7, share_chat 4, chat_history 5, dark_tech_theme 6, background_no_motion 8). +- [x] One `--no-gpg-sign` commit; phase dir moved to `.agent/phases/complete/`. diff --git a/.agent/phases/todo/52_pinned_composer/00_phase.md b/.agent/phases/todo/52_pinned_composer/00_phase.md deleted file mode 100644 index 55c389d..0000000 --- a/.agent/phases/todo/52_pinned_composer/00_phase.md +++ /dev/null @@ -1,41 +0,0 @@ -# Phase 52 — Pinned Message Composer - -**Source:** `TODO.md` L3 — "The message input text box needs to be pinned to the bottom of the screen so it doesn't \"run away\" from the user as they try to click \"stop\"" -**Story:** n/a (TODO-derived — owner instruction 2026-08-30: convert without confirmation) -**Context:** The chat page (`frontend/index.html`) scrolls at the document level: `.chat-shell` (the centered 46rem column, PLAN §7) is a flex column — kb-banner, steering panel, New chat, Save/Share, `.messages`, and finally the `.composer` form (`#message-input` + `#send-btn`). The composer is NOT sticky — in a long conversation it sits below the fold, and since the page never auto-scrolls while a turn streams (phase 42), the Stop button (phase 48: `#send-btn` morphs into the enabled Stop control in flight) can be off-screen exactly when the user wants to click it. The sticky app header is the only sticky chrome (z-index 20, 2px hairline below); the document modal is the topmost layer (z-index 1000). House frontend testing: source pins (`tests/unit/test_frontend_feedback.py` style — `test_frontend_scroll.py` / `test_history_page.py` are the closest precedents) plus one isolated Playwright suite per story (A16). - -## Objective -The composer (input + Send/Stop button) is pinned to the bottom of the viewport at every scroll position — the Stop control is always reachable mid-turn without scrolling, and no new auto-scroll behaviour is introduced (the phase-42 contract stays intact). - -## Dependencies -- `48_stop_generation` (complete) — the Send↔Stop morph; Stop is clicked FROM the pinned composer (the original "run away" scenario). -- `42_no_reply_autoscroll` (complete) — the no-autoscroll-while-streaming contract the pin must not revise. -- `07_story_responsive_polish` (complete) — the 46rem column / responsive rules the pinned composer must sit within. - -## Tasks -1. `01_sticky_composer.md` — the `position: sticky; bottom` pin on `.composer` + safe-area inset + the frontend source pins. -2. `02_e2e_pinned_composer.md` — the story Playwright suite + regressions + commit. - -## Testing & Quality -- Unit: `tests/unit/test_pinned_composer.py` — source pins: `.composer` carries `position: sticky` with a `bottom` offset (safe-area inset) in `styles.css`; `app.js` gains NO new page-scroll call site (the phase-42 invariant — the one page scroll is still `scrollReveal`). -- Coverage: **>90%** on `app/` (validate.sh gate — this phase makes no `app/` changes; the gate must stay green). -- E2E (mandatory, A16): `tests/e2e/test_pinned_composer.py`, run in isolation. - -## Completion Criteria -- [ ] With an over-viewport conversation, scrolled to the top: the composer is fully visible (bounding box inside the viewport) at the bottom edge. -- [ ] In flight, scrolled up to read earlier content: the Stop button is visible and clickable; clicking it (no scrolling) stops the turn — partial kept + persisted with `stopped: true` (the phase-48 contract, unchanged), no error banner, no window scroll (phase 42). -- [ ] On an empty/short chat the composer renders in its normal flow position (the pin does not float it over the footer or shift the layout). -- [ ] `uv run pytest` green; coverage TOTAL >90%. -- [ ] `uv run pytest tests/e2e/test_pinned_composer.py -v --no-cov` green in isolation (DB up). -- [ ] Regression E2E suites green in isolation: `test_stop_generation.py`, `test_no_reply_autoscroll.py`, `test_chat_persistence.py`, `test_mobile_hamburger_nav.py`. -- [ ] `uv run ruff check . && uv run pyright` clean. -- [ ] One `--no-gpg-sign` commit; phase dir moved to `.agent/phases/complete/`. - -## Locked decisions -- **Recorded assumptions (TODO conversion, 2026-08-30 — owner asked for no confirmation):** (1) the pin is CSS-only — `position: sticky; bottom: env(safe-area-inset-bottom)` on the existing `.composer` inside the existing `.chat-shell` column; no `index.html` DOM change, no JS; (2) the composer keeps its current solid `--surface` background + border + shadow (no glass/transparency), so scrolled messages never show through it; (3) NO z-index change — the composer already paints above `.messages` by DOM order, never overlaps the sticky header, and stays under the z-1000 document modal; (4) the phase-42 never-auto-scroll contract is strictly upheld — the pin adds zero scroll call sites. -- **A16/A17 honoured** — one story E2E suite, one atomic commit. - -## Commit -```bash -git add -A .agent/ frontend/ tests/ && git commit --no-gpg-sign -m "feat(chat): pin the composer to the viewport bottom — Stop is always reachable while reading" -``` diff --git a/frontend/assets/styles.css b/frontend/assets/styles.css index 98987aa..1feaf65 100644 --- a/frontend/assets/styles.css +++ b/frontend/assets/styles.css @@ -404,6 +404,13 @@ html::after { } /* ---------- Main frame ---------- */ +/* Phase 52 (owner revision 2026-08-30): `flex: 1` is load-bearing, not + cosmetic. Body is a `min-height: 100dvh` column flex box, so `main` + grows to the full viewport height and `.chat-shell` grows to fill + `main`. That full-height column is what makes the composer's sticky + shift range tall enough to hold the box on screen — the pin works in + BOTH directions only because of it (see `.composer` and `.messages`). + No `overflow` here: the document must stay the scroll container. */ .app-main { flex: 1; display: flex; @@ -423,7 +430,20 @@ html::after { flex: 1; } +/* Phase 52 (owner revision 2026-08-30): `flex-grow` is the other half of + the pin. `position: sticky` only ever pulls a box UP toward the + viewport bottom — it can never push a box DOWN to meet it — so on an + empty/short chat the composer used to sit just under the empty state, + mid-screen, with a dead band between it and the footer. Growing the + message list absorbs that free space instead, so the composer's + resting (in-flow) position already IS the bottom of the screen. Once + the conversation is taller than the viewport there is no free space + left to absorb, `flex-grow` does nothing, and the `.composer` sticky + rule takes over. `flex-basis: auto` (not `1`) so the list keeps its + content height when the page overflows; the phase-01 `min-height` + floor stays; still no inner scroller (the document scrolls). */ .messages { + flex: 1 1 auto; display: flex; flex-direction: column; gap: 0.9rem; @@ -1140,12 +1160,21 @@ details.thinking .thinking-text ul { margin: 0 0 0.5rem; } fold while phase 42 forbids auto-scrolling a streaming reply — the Stop control could literally not be reached ("it runs away from the user as they try to click stop"). + The pin is TWO rules working together — sticky alone is not enough + (owner revision 2026-08-30: "the chat message-input textarea should be + at the bottom of the screen"). `position: sticky` can only pull the + box UP to the viewport's bottom edge while the document overflows; it + never pushes it DOWN, so on an empty/short chat the box still sat + mid-screen. `.messages { flex: 1 1 auto }` supplies the missing half: + it puts the box's resting position at the bottom of the full-height + column, and sticky takes over the moment the conversation overflows. + The offset is the notch-aware safe-area inset (`env()` resolves to 0, + or falls back to 0 where unsupported, so the box sits flush with the + viewport bottom; ≤640px keeps its own + `main { padding-bottom: env(safe-area-inset-bottom) }`). CSS-only by design: no DOM change, no JS, and NO new scroll call site (the phase-42 never-auto-scroll contract stays intact — - `scrollReveal` is still the one page scroll in app.js). The offset - is the notch-aware safe-area inset (`env()` resolves to 0 on - desktop, so the box sits flush with the viewport bottom; ≤640px keeps - its own `main { padding-bottom: env(safe-area-inset-bottom) }`). + `scrollReveal` is still the one page scroll in app.js). The solid `--surface` background, border, radius and shadow are kept so messages scrolling behind the pinned box never show through it, and no z-index is added: DOM order already paints the composer over @@ -1156,7 +1185,7 @@ details.thinking .thinking-text ul { margin: 0 0 0.5rem; } align-items: flex-end; gap: 0.6rem; position: sticky; - bottom: env(safe-area-inset-bottom); + bottom: env(safe-area-inset-bottom, 0); background: var(--surface); border: 1px solid var(--line); border-radius: var(--radius); diff --git a/tests/e2e/test_pinned_composer.py b/tests/e2e/test_pinned_composer.py index 046a850..93c2f30 100644 --- a/tests/e2e/test_pinned_composer.py +++ b/tests/e2e/test_pinned_composer.py @@ -22,13 +22,16 @@ observed in a real browser: at all (the phase-42 never-auto-scroll contract — the pin adds no scroll call site), and a reload restores the ``stopped`` record through the normal ``bor.chat.v1`` path; -* on an empty/short chat the composer renders in its normal flow position - — the pin does not float it over the footer (``.app-footer`` stays in - flow below it), and at the document bottom the box has settled back into - flow above the footer instead of hovering on the viewport edge; -* ≤640px the pin holds too (the composer is reachable, ≥44px Send target) - and stays UNDER the sticky header — no z-index was added, so the pinned - box can never cover the header or the hamburger dropdown (phase 46). +* on an empty/short chat the composer renders in its RESTING position, + which the grow puts at the BOTTOM of the screen — the only band below it + is chrome (``.app-footer``, in normal flow, never overlapped), and at the + document bottom the box has settled back into that same flow slot above + the footer instead of hovering on the viewport edge; + +* ≤640px the same two rules hold: the empty phone chat puts the input at + the bottom of the screen and an over-viewport chat keeps it there, still + UNDER the sticky header (no z-index was added, so the pinned box can + never cover the bar or the hamburger dropdown — phase 46 stacking). Determinism: the mock's grounded answers quote the question and end in the ``Deterministic mock answer for E2E`` marker; the long answer (``write a @@ -41,7 +44,7 @@ of conversation into the 800px viewport. Test → story mapping (Playwright Mapping Rule): 1. ``test_composer_pinned_at_every_scroll_position`` 2. ``test_stop_is_reachable_from_scrolled_up`` — the TODO.md L3 scenario -3. ``test_empty_chat_composer_sits_in_normal_flow`` +3. ``test_empty_chat_composer_sits_at_the_screen_bottom`` 4. ``test_pin_holds_on_mobile_under_the_header`` — ≤640px stacking """ from __future__ import annotations @@ -100,6 +103,15 @@ FLUSH_PX = 4 #: The sticky header's band (phase 12 pins it at 64px) — the pinned #: composer must never enter it. HEADER_PX = 64 +#: How much of the viewport may sit BELOW a resting composer as chrome: +#: the footer's own measured height plus the flow padding the settled slot +#: keeps (`main { padding-block: 1.25rem }` + the column's 1rem gap, with +#: headroom for the safe-area inset). Anything more is wasted dead space — +#: the empty chat must NOT leave the input hanging mid-screen. +BOTTOM_SLACK_PX = 56 +#: The resting composer also has to be visibly in the lower part of the +#: screen — the pre-revision geometry rested at ~57% of the viewport here. +LOWER_PART = 0.8 # The typing indicator is itself a .msg.brain — exclude its bubble. ANSWER = ".msg.brain .bubble:not(.typing)" @@ -459,43 +471,74 @@ def test_stop_is_reachable_from_scrolled_up( # --------------------------------------------------------------------------- -# 3. Natural bottom: an empty/short chat keeps the composer in its flow -# position — the pin does not float it over the footer or shift the page +# 3. Natural bottom: an empty/short chat RESTS the composer at the bottom +# of the screen — no dead band under it, no float over the footer # --------------------------------------------------------------------------- -def test_empty_chat_composer_sits_in_normal_flow( +def assert_rests_at_the_screen_bottom(page: Page) -> dict[str, float]: + """The resting (in-flow) composer sits at the bottom of the screen. + + This is the half `position: sticky` cannot deliver — sticky only pulls + a box UP toward the viewport bottom while the document overflows, so on + a page that does not scroll the box rested where the empty state ended + (mid-screen) until `.messages` gained `flex: 1 1 auto`. The band below + the box must be chrome only: the footer (measured live, so phones with + the stacked footer are covered) plus the flow padding of the settled + slot. + """ + ch = scroll_state(page)["ch"] + composer = composer_box(page) + bottom = composer["y"] + composer["height"] + footer = box_of(page, ".app-footer") + + assert bottom <= ch + FLUSH_PX, ( + f"the resting composer hangs below the viewport bottom " + f"(bottom={bottom:.1f}, viewport={ch:.0f})" + ) + assert ch - bottom <= footer["height"] + BOTTOM_SLACK_PX, ( + f"dead band under the composer: {ch - bottom:.0f}px below it with a " + f"{footer['height']:.0f}px footer — the input is not at the bottom " + "of the screen" + ) + assert bottom >= ch * LOWER_PART, ( + f"the composer rests at {bottom / ch:.0%} of the viewport — it has " + "to sit at the bottom, not under the empty state" + ) + assert_no_overlap(composer, footer) # chrome, but never overlapped + return composer + + +def test_empty_chat_composer_sits_at_the_screen_bottom( page: Page, app_url: str, seeded_kb: None ) -> None: page.set_default_timeout(30_000) page.goto(app_url) - # A fresh visitor: the empty state, nothing to scroll. + # A fresh visitor: the empty state, nothing to scroll — and yet the + # input is already at the bottom of the screen. expect(page.locator("#empty-state")).to_be_visible() state = scroll_state(page) assert state["sh"] <= state["ch"] + 1, ( - "an empty chat must not be scrollable — there is nothing to pin against" + "an empty chat must not be scrollable — the grow must not invent " + "scrollable space, it only redistributes the space the page has" ) - composer = composer_box(page) - assert composer["y"] + composer["height"] <= state["ch"] + FLUSH_PX, ( - "the composer may never hang below the viewport bottom" - ) + assert_rests_at_the_screen_bottom(page) - # The footer is present, in normal flow, BELOW the composer: the sticky - # box settles in its own flow slot (last child of .chat-shell) instead - # of floating over the footer on a short page. - footer = page.locator(".app-footer") - expect(footer).to_be_visible() - assert_no_overlap(composer, box_of(page, ".app-footer")) + # The footer is present, in flow, BELOW the composer: the grow pushes + # the composer down to its own flow slot, it does not float it over the + # footer. + expect(page.locator(".app-footer")).to_be_visible() - # A single short turn: still in flow, still nothing to scroll past. + # A single short turn: still nothing to scroll past, still resting at + # the bottom (the conversation has not overflowed the viewport yet). submit(page, SHORT_QUESTIONS[0]) wait_settled(page) - state = scroll_state(page) - composer = composer_box(page) - assert composer["y"] + composer["height"] <= state["ch"] + FLUSH_PX - assert_no_overlap(composer, box_of(page, ".app-footer")) + assert scroll_state(page)["sh"] <= scroll_state(page)["ch"] + 1, ( + "one short turn must not overflow the 800px test viewport" + ) + assert_rests_at_the_screen_bottom(page) expect(page.locator(ANSWER).last).to_contain_text(MOCK_ANSWER_MARKER) @@ -513,6 +556,11 @@ def test_pin_holds_on_mobile_under_the_header( try: page.set_default_timeout(30_000) page.goto(app_url) + + # An empty phone chat rests the input at the bottom of the screen. + expect(page.locator("#empty-state")).to_be_visible() + assert_rests_at_the_screen_bottom(page) + build_conversation(page, n=4) state = scroll_state(page) @@ -521,6 +569,7 @@ def test_pin_holds_on_mobile_under_the_header( f"ch={state['ch']:.0f})" ) + # …and an over-viewport one keeps it pinned there while reading. page.evaluate("() => window.scrollTo(0, 0)") assert page.evaluate( "() => document.querySelector('.chat-shell').getBoundingClientRect().bottom" diff --git a/tests/unit/test_pinned_composer.py b/tests/unit/test_pinned_composer.py index 3b823a1..d79b277 100644 --- a/tests/unit/test_pinned_composer.py +++ b/tests/unit/test_pinned_composer.py @@ -1,19 +1,32 @@ -"""Unit: the pinned (sticky-bottom) composer in the static frontend -(phase 52, owner direction 2026-08-30, TODO.md L3 — "The message input -text box needs to be pinned to the bottom of the screen so it doesn't -'run away' from the user as they try to click 'stop'"). +"""Unit: the pinned composer in the static frontend (phase 52, owner +direction 2026-08-30, TODO.md L3 — "The message input text box needs to be +pinned to the bottom of the screen so it doesn't 'run away' from the user +as they try to click 'stop'"; owner revision the same day — the box had to +be at the bottom of the screen on an EMPTY chat too, which sticky alone +cannot do). The chat page scrolls at the DOCUMENT level and `.chat-shell` (the centered 46rem column, PLAN §7.1) is the composer's sticky containing -block, so `position: sticky; bottom: env(safe-area-inset-bottom)` on -`.composer` pins the box to the viewport's bottom edge at every scroll -position and lets it settle back into its normal flow position (above -the footer) once the document bottom is reached. The pin is CSS-only: +block. The pin is therefore TWO rules, and both are pinned here: + +* `.messages { flex: 1 1 auto }` absorbs the free space of a short page, + so the composer's RESTING (in-flow) position is already the bottom of + the full-height column — `position: sticky` can only pull a box UP + toward the viewport's bottom edge, it can never push one DOWN to meet + it, so without the grow the empty/short chat left the input mid-screen + with a dead band under it (the reported bug); +* `.composer { position: sticky; bottom: env(safe-area-inset-bottom, 0) }` + takes over the moment the conversation overflows the viewport and keeps + the box — and the Stop control inside it — glued to the viewport's + bottom edge at every scroll position, settling back into flow (above + the footer) once the document bottom is reached; + +The pin is CSS-only: * `.composer` carries the sticky pair (``position: sticky`` + the - notch-aware ``bottom`` inset) and keeps its solid ``--surface`` - background, border, radius and shadow — a reply scrolling behind the - pinned box must never show through it; + notch-aware ``bottom`` inset, with the explicit ``0`` fallback) and + keeps its solid ``--surface`` background, border, radius and shadow — a + reply scrolling behind the pinned box must never show through it; * NO ``z-index`` is added to the composer (it already paints above ``.messages`` by DOM order, never overlaps the sticky header (z 20) and stays under the z-1000 document modal), and the pin is not undone @@ -24,6 +37,12 @@ the footer) once the document bottom is reached. The pin is CSS-only: * ``index.html`` needs no change: the composer is already the LAST child of `.chat-shell` (the sticky shift range is that column's box) and the `#message-input` / `#send-btn` / `#send-status` markup is untouched; +* the sticky context survives: `.chat-shell` / `.app-main` / `.messages` + gain no ``overflow`` and `.messages` gains no inner scroller — the page + keeps scrolling at the document level (the phase-42 model); +* ``index.html`` needs no change: the composer is already the LAST child + of `.chat-shell` (the sticky shift range is that column's box) and the + `#message-input` / `#send-btn` / `#send-status` markup is untouched; * ``app.js`` gains NO page-scroll call site — the phase-42 invariant (``scrollReveal`` is still the single ``window.scrollTo``; no ``scrollIntoView``, no ``window.scrollY``) is re-pinned here so the @@ -150,22 +169,55 @@ def _tree() -> _Tree: def test_composer_is_sticky_bottom() -> None: """`.composer` carries the sticky pair — `position: sticky` AND a `bottom` offset — inside its own rule. `bottom` is the notch-aware - safe-area inset (on desktop `env()` resolves to 0, so the box sits - flush with the viewport bottom; on a notched phone the composer - clears the home indicator instead of hiding under it).""" + safe-area inset with an explicit `0` fallback (it resolves to 0 on a + desktop viewport, so the box sits flush with the viewport bottom; on a + notched phone the composer clears the home indicator instead of hiding + under it — and a browser without `env()` support still gets 0).""" body = _rule(_css(), ".composer") assert "position: sticky;" in body, ( "the composer must be sticky — phase 52 pins it to the viewport " "bottom so Stop is reachable without scrolling (TODO.md L3)" ) - assert "bottom: env(safe-area-inset-bottom);" in body, ( - "the sticky offset must be the safe-area inset (the phase-07 " - "mobile contract: the composer stays reachable around the notch)" + assert "bottom: env(safe-area-inset-bottom, 0);" in body, ( + "the sticky offset must be the safe-area inset with a 0 fallback " + "(the phase-07 mobile contract: the composer stays reachable " + "around the notch, and never falls back to `auto` = no pin)" ) # `top` would pin it to the wrong edge (and fight the sticky header). assert "top:" not in body, "the composer pins the BOTTOM edge only" +def test_message_list_absorbs_the_short_page_space() -> None: + """The other half of the pin — and the half phase 52 originally missed. + + `position: sticky` never pushes a box DOWN to the viewport's bottom + edge, so on an empty/short chat (no free space absorbed) the composer + sat right under the empty state, mid-screen. `.messages` must GROW to + take up that space, which puts the composer's resting position at the + bottom of the full-height column the page already builds + (`body{min-height:100dvh}` → `.app-main{flex:1}` → `.chat-shell{flex:1}`). + """ + messages = _rule(_css(), ".messages") + assert re.search(r"flex:\s*1(\s+1\s+auto)?;", messages), ( + "the message list must grow to fill the short page — otherwise the " + "pinned composer only works once the conversation overflows and the " + "empty chat leaves the input mid-screen (owner revision 2026-08-30)" + ) + # flex-basis must stay `auto`: a `0` basis would size the list BELOW its + # content when the conversation overflows and let bubbles overlap the box. + assert not re.search(r"flex:\s*1\s+1\s+0", messages), ( + "flex-basis must stay auto — the list keeps its content height when " + "the page overflows (no free space to absorb there anyway)" + ) + # The column the composer sits in must keep stretching to the viewport. + for selector in (".app-main", ".chat-shell"): + frame = _rule(_css(), selector) + assert re.search(r"flex:\s*1[;\s]", frame), ( + f"{selector} must keep growing to the viewport height — the " + "composer can only rest at the bottom of a full-height column" + ) + + def test_composer_stays_opaque_behind_scrolled_messages() -> None: """The pinned box overlaps the message list while the page is scrolled: it keeps its SOLID `--surface` background plus border, @@ -243,7 +295,8 @@ def test_document_level_scroll_is_untouched() -> None: messages = _rule(css, ".messages") assert not re.search(r"(?