diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index f90f24dde..5ea7c31ab 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -503,7 +503,7 @@ automatically armed by installing Möbius. ## Chat scroll + steer contract -**Owner-authoritative contract — v1.9 (2026-07-24).** This section is the +**Owner-authoritative contract — v1.10 (2026-07-26).** This section is the canonical source of truth for how a chat scrolls and steers. When implementation, comments, and this contract disagree, the implementation/comments are the bug: fix behavior to match this contract. If a real case is unspecified or the desired @@ -539,8 +539,9 @@ and attaches their rule ids to new diagnostic chats. The Playwright lock-in spec seeing an older user row never qualifies. An otherwise unreachable anchor clamps to real conversation content before visibility is decided. R6's transient question-submit hold is the sole exception: it may reserve only the exact tail - deficit required to keep the answered card at its frozen offset through mobile - viewport growth, and that intent is never persisted. + deficit required for a stable card handoff while the viewport size is unchanged. + It is never persisted and must release to the unanswered card's prior mode before + a keyboard or other viewport resize is laid out. - **R2 — One send rule everywhere.** The first visible user message always pins to the viewport top. Every subsequent direct, queued, promoted, or steered message pins when its submit-time DOM snapshot is at the real-content tail. Geometry is @@ -629,10 +630,12 @@ and attaches their rule ids to new diagnostic chats. The Playwright lock-in spec card enters its pending state or output resumes, the controller snapshots the currently visible message and its exact viewport offset as `ANCHOR_AT`. Resumed output grows without dragging the reader, even when the chat had been following - the tail before Submit. If a mobile viewport grows before that output arrives, - the dynamic spacer temporarily reserves exactly enough room to keep the anchor - target reachable; the reservation disappears as real content replaces it and - the transient reservation intent is stripped before persistence. A failed answer + the tail before Submit. That exact hold is scoped to the viewport where Submit + occurred. If the mobile keyboard changes the viewport, the controller restores + the mode that owned the unanswered card before sizing the new geometry. Answering + therefore adds no movement of its own, while the keyboard still moves the card + exactly as it would have moved unanswered. The transient hold is stripped before + persistence. A failed answer keeps that settled reading anchor for the retryable card rather than manufacturing follow intent again. The source handoff @@ -667,7 +670,8 @@ path means routing it through the same entries rather than inventing another rul | Viewport/keyboard changes | `PIN_USER_MSG` | same `PIN_USER_MSG` | Reapply pin after resize; never infer intent from keyboard-open geometry | | Viewport/keyboard changes | follow or anchor hold | same follow if still at tail, otherwise hold anchor | Never creates follow | | Chat exits/backgrounds/returns | any | `ANCHOR_AT` | Restore exact saved anchor | -| In-process question is answered | any | `ANCHOR_AT` on current visible row; same active assistant row | Hold exact visible anchor through card reflow and resumed output | +| In-process question is answered | any | transient `ANCHOR_AT` over the prior mode; same active assistant row | Hold exact visible anchor through same-viewport card reflow and resumed output | +| Viewport/keyboard changes after question submission | transient question anchor | pre-submit unanswered-card mode | Apply ordinary viewport behavior; answering adds no extra movement | | Live assistant row settles to the durable transcript | any | same mode and row identity | None (except R3's exact spacer handoff) | | Offscreen question or paused-turn nudge tapped | any hold | `ANCHOR_AT` at physical tail | User-requested one-shot move; clears the overlaid composer | diff --git a/backend/app/claude_sdk_runner.py b/backend/app/claude_sdk_runner.py index 4b67df7b9..d4087fd15 100644 --- a/backend/app/claude_sdk_runner.py +++ b/backend/app/claude_sdk_runner.py @@ -62,13 +62,19 @@ import json import logging import os +import signal import shutil import time from collections import deque from typing import Any from uuid import uuid4 -from claude_agent_sdk import ClaudeAgentOptions, ClaudeSDKClient, HookMatcher +from claude_agent_sdk import ( + ClaudeAgentOptions, + ClaudeSDKClient, + HookMatcher, + ProcessError, +) from claude_agent_sdk.types import ( AssistantMessage, PermissionResultAllow, @@ -149,6 +155,26 @@ def _claude_process_group_id(client: ClaudeSDKClient) -> int | None: return pgid +def _claude_process_was_force_stopped(client: ClaudeSDKClient) -> bool: + """Whether the SDK transport recorded one of Möbius's stop signals. + + The SDK's background reader converts ``ProcessError`` into a plain + ``Exception`` before it reaches ``receive_response()``. Keep this predicate + narrow by reading the still-typed transport outcome and accepting only the + TERM/KILL return codes ``terminate_process_group`` can cause. A different + typed process failure that merely races a Stop must remain visible to the + owner. The transport is already an intentional private SDK seam here + (``_claude_process_group_id`` reads its child pid); if the SDK changes shape, + this fails closed and the error remains visible. + """ + transport = getattr(client, "_transport", None) + exit_error = getattr(transport, "_exit_error", None) + return ( + isinstance(exit_error, ProcessError) + and exit_error.exit_code in (-signal.SIGTERM, -signal.SIGKILL) + ) + + def _terminate_claude_process_group(pgid: int | None) -> bool: return terminate_process_group( pgid, @@ -305,6 +331,11 @@ def __init__(self, client: ClaudeSDKClient, chat_id: str): # Never signal a retained PGID twice; the kernel can eventually reuse it # after the first hard stop. self._force_stop_started = False + # Set synchronously before interrupt()'s first await. Claude reports both a + # Möbius-requested Stop and an unexpected provider interruption as an + # error-shaped ResultMessage, so the runner needs this local ownership fact + # to keep a deliberate Stop from overwriting its resumable pause note. + self._interrupt_requested = False # FIFO of mid-turn steer texts: two rapid sends must both reach Claude # (both are already persisted to the transcript), so a single slot would # silently drop the first. The runner drains the whole list on interrupt. @@ -424,6 +455,7 @@ async def interrupt(self) -> None: dropping them here: they were never in the transcript, and Stop's own clear-and-resend path is what preserves them. """ + self._interrupt_requested = True self.pending_steer = [] self._steer_requested = False self._steer_user_msgs = [] @@ -438,6 +470,10 @@ async def interrupt(self) -> None: "runner is wedged", ) + @property + def interrupt_requested(self) -> bool: + return self._interrupt_requested + async def stop(self, timeout: float = 2.0) -> bool: """Interrupts the SDK run and waits up to `timeout` seconds.""" try: @@ -1570,6 +1606,17 @@ def _capture_stderr(line: str) -> None: active_client._interrupt_in_flight = True await client.interrupt() continue + if ( + active_client.interrupt_requested + and isinstance(sdk_msg, ResultMessage) + and sdk_msg.stop_reason == "interrupt" + ): + # The SDK describes a deliberate Stop with the same + # error_during_execution envelope it uses for an unexpected + # interruption. Preserve its usage/cost, but do not let the + # provider-shaped error overwrite chat.py's resumable stop note. + terminal["error"] = None + terminal["terminal_status"] = "interrupted" # Terminal result: the interrupt cycle (if any) is closed, so a # fresh boundary cut may fire on a later turn. active_client._interrupt_in_flight = False @@ -1599,6 +1646,7 @@ def _capture_stderr(line: str) -> None: # retry is also empty the finalize backstop records a retry marker. if ( session_id is not None # a resume (non-first turn) + and not active_client.interrupt_requested # Stop is terminal and not terminal.get("error") # clean terminal (is_error False) and terminal.get("api_error_status") != 429 # not a bare 429/park and not active_client.pending_steer @@ -1646,6 +1694,21 @@ def _capture_stderr(line: str) -> None: # error and finalize() persists a durable error block, instead of the # old silent `error=None` that logged a clean $0 "done" and let the # just-consumed user message go unanswered with nothing to reconcile. + if active_client.interrupt_requested: + # A graceful interrupt may close the response stream without its usual + # ResultMessage. The local ownership flag is enough here: there is no + # provider error to suppress, only the resultless end caused by Stop. + log.warning( + "Claude response stream ended after our own stop chat_id=%s", + chat_id, + ) + return { + "session_id": current_session_id, + "cost_usd": cost_usd, + "usage": None, + "error": None, + "terminal_status": "interrupted", + } return { "session_id": current_session_id, "cost_usd": cost_usd, @@ -1657,6 +1720,28 @@ def _capture_stderr(line: str) -> None: } except Exception as exc: msg = str(exc) + if ( + active_client.interrupt_requested + and _claude_process_was_force_stopped(client) + ): + # force_stop() SIGTERMs the verified private CLI process group when a + # graceful interrupt times out. The SDK's reader converts the typed + # ProcessError into a plain Exception before it reaches us, so consult + # the still-typed transport outcome rather than matching message text. + # WARNING is deliberate: the owner sees a clean interrupted turn, but + # operators retain the only evidence a coincident CLI crash leaves. + log.warning( + "Claude process exited during our own stop chat_id=%s: %s", + chat_id, + exc, + ) + return { + "session_id": current_session_id, + "cost_usd": None, + "usage": None, + "error": None, + "terminal_status": "interrupted", + } # The SDK raises this generic placeholder when the CLI dies before a # structured result (early resume failure, auth, crash, OOM/SIGTERM # kill). Splice in the captured stderr tail ONLY then — gating on the diff --git a/backend/tests/test_claude_sdk_runner.py b/backend/tests/test_claude_sdk_runner.py index d577a8ca8..aa0589177 100644 --- a/backend/tests/test_claude_sdk_runner.py +++ b/backend/tests/test_claude_sdk_runner.py @@ -16,6 +16,7 @@ import pytest +from claude_agent_sdk import ProcessError from claude_agent_sdk.types import ( AssistantMessage, RateLimitEvent, @@ -44,7 +45,7 @@ steer_into_active_turn, ) from app.database import SessionLocal -from app.runner_registry import registry +from app.runner_registry import RunnerKind, registry class _Bus: @@ -276,6 +277,189 @@ def _interrupt_result(session_id: str = "sess-1") -> ResultMessage: ) +async def _run_claude_stop_outcome(monkeypatch, mode: str, *, owned: bool): + """Run one fake response stream with an optional owner Stop in flight.""" + process_error = ProcessError( + f"Command failed with exit code {'1' if mode == 'process_failure' else '-15'}", + exit_code=1 if mode == "process_failure" else -15, + stderr="Check stderr output for details", + ) + + class _Transport: + _process = None + _exit_error = process_error if mode in ("process_error", "process_failure") else None + + class _FakeClient: + def __init__(self, options): + del options + self._transport = _Transport() + + async def connect(self): + return None + + async def query(self, message): + del message + + async def interrupt(self): + return None + + async def disconnect(self): + return None + + async def receive_response(self): + handle = registry.get_handle("claude-stop-shape", RunnerKind.CLAUDE_SDK) + assert handle is not None + handle._interrupt_requested = owned + if mode == "terminal": + yield _interrupt_result() + return + if mode == "resultless": + return + if mode in ("process_error", "process_failure"): + raise Exception(str(process_error)) + if mode == "other_error": + raise ValueError("unexpected notification payload") + raise AssertionError(mode) + + monkeypatch.setattr(claude_sdk_runner, "ClaudeSDKClient", _FakeClient) + return await run_claude_sdk_turn( + "hello", + session_id=None, + base_env={}, + cwd="/tmp", + chat_id="claude-stop-shape", + skill_text="system", + bc=_ChatBus(), + pending_questions={}, + db=None, + ) + + +@pytest.mark.asyncio +async def test_claude_interrupt_marks_owner_request_before_sdk_await(): + observed = [] + + class _Client: + async def interrupt(self): + observed.append(handle.interrupt_requested) + + handle = ActiveClaudeClient(_Client(), chat_id="claude-owned-stop") + task = asyncio.create_task(handle.interrupt()) + while not observed: + await asyncio.sleep(0) + handle.mark_finished() + await task + + assert observed == [True] + + +def test_claude_force_stop_check_uses_the_sdk_type_not_its_name(): + impostor = type("ProcessError", (Exception,), {}) + + class _Client: + _transport = type("Transport", (), {"_exit_error": impostor("boom")})() + + assert claude_sdk_runner._claude_process_was_force_stopped(_Client()) is False + + +def test_claude_force_stop_check_rejects_other_typed_process_failures(): + class _Client: + _transport = type("Transport", (), { + "_exit_error": ProcessError("CLI failed", exit_code=1), + })() + + assert claude_sdk_runner._claude_process_was_force_stopped(_Client()) is False + + +@pytest.mark.asyncio +async def test_owner_stop_turns_claude_interrupt_result_into_clean_terminal( + monkeypatch, +): + result = await _run_claude_stop_outcome(monkeypatch, "terminal", owned=True) + + assert result["error"] is None + assert result["terminal_status"] == "interrupted" + assert result["cost_usd"] == 0.01 + assert result["usage"] == {"input_tokens": 1, "output_tokens": 2} + + +@pytest.mark.asyncio +async def test_unrequested_claude_interrupt_result_stays_an_error(monkeypatch): + result = await _run_claude_stop_outcome(monkeypatch, "terminal", owned=False) + + assert result["error"] == "Execution interrupted." + assert result.get("terminal_status") is None + + +@pytest.mark.asyncio +async def test_owner_stop_accepts_resultless_claude_stream_as_interrupted( + monkeypatch, +): + result = await _run_claude_stop_outcome( + monkeypatch, "resultless", owned=True, + ) + + assert result["error"] is None + assert result["terminal_status"] == "interrupted" + + +@pytest.mark.asyncio +async def test_unrequested_resultless_claude_stream_stays_an_error(monkeypatch): + result = await _run_claude_stop_outcome( + monkeypatch, "resultless", owned=False, + ) + + assert "ended unexpectedly" in result["error"] + assert result.get("terminal_status") is None + + +@pytest.mark.asyncio +async def test_owner_stop_reclassifies_typed_claude_process_exit( + monkeypatch, caplog, +): + result = await _run_claude_stop_outcome( + monkeypatch, "process_error", owned=True, + ) + + assert result["error"] is None + assert result["terminal_status"] == "interrupted" + assert any( + record.levelname == "WARNING" + and "Claude process exited during our own stop" in record.message + for record in caplog.records + ) + + +@pytest.mark.asyncio +async def test_unrequested_claude_process_exit_stays_an_error(monkeypatch): + result = await _run_claude_stop_outcome( + monkeypatch, "process_error", owned=False, + ) + + assert "Command failed with exit code -15" in result["error"] + assert result.get("terminal_status") is None + + +@pytest.mark.asyncio +async def test_owner_stop_does_not_hide_unrelated_claude_failure(monkeypatch): + result = await _run_claude_stop_outcome( + monkeypatch, "other_error", owned=True, + ) + + assert result["error"] == "unexpected notification payload" + assert result.get("terminal_status") is None + + +@pytest.mark.asyncio +async def test_owner_stop_does_not_hide_other_claude_process_failure(monkeypatch): + result = await _run_claude_stop_outcome( + monkeypatch, "process_failure", owned=True, + ) + + assert "exit code 1" in result["error"] + assert result.get("terminal_status") is None + + @pytest.mark.asyncio async def test_steer_fires_at_assistant_boundary_not_on_deltas(monkeypatch): """THE core contract: a steer requested mid-turn does NOT interrupt diff --git a/frontend/src/components/ChatView/__tests__/chatContract.test.js b/frontend/src/components/ChatView/__tests__/chatContract.test.js index 20a4276fd..1a0d17661 100644 --- a/frontend/src/components/ChatView/__tests__/chatContract.test.js +++ b/frontend/src/components/ChatView/__tests__/chatContract.test.js @@ -67,21 +67,26 @@ test('ChatView only consumes methods returned by the scroll controller', () => { `ChatView consumes missing useScrollMode members: ${missing.join(', ')}`) }) -test('owner contract freezes question answers with one transient reachability exception', () => { +test('owner contract freezes question answers without locking keyboard movement', () => { const architecture = readFileSync( new URL('../../../../../ARCHITECTURE.md', import.meta.url), 'utf8', ) - assert.match(architecture, /Owner-authoritative contract — v1\.9 \(2026-07-24\)/) + assert.match(architecture, /Owner-authoritative contract — v1\.10 \(2026-07-26\)/) assert.match( architecture, - /In-process question is answered \| any \| `ANCHOR_AT` on current visible row; same active assistant row/, + /In-process question is answered \| any \| transient `ANCHOR_AT` over the prior mode; same active assistant row/, 'question submission must freeze the reader while preserving the R6 row', ) assert.match( architecture, - /question-submit hold is the sole exception: it may reserve only the exact tail\s+deficit required to keep the answered card at its frozen offset/, - 'question submission may reserve only its documented reachability deficit', + /question-submit hold is the sole exception: it may reserve only the exact tail\s+deficit required for a stable card handoff while the viewport size is unchanged/, + 'question submission may reserve only its same-viewport reachability deficit', + ) + assert.match( + architecture, + /Viewport\/keyboard changes after question submission \| transient question anchor \| pre-submit unanswered-card mode/, + 'keyboard movement must return to the unanswered card baseline', ) }) diff --git a/frontend/src/components/ChatView/__tests__/useScrollMode.test.js b/frontend/src/components/ChatView/__tests__/useScrollMode.test.js index 5d856546c..39205e6be 100644 --- a/frontend/src/components/ChatView/__tests__/useScrollMode.test.js +++ b/frontend/src/components/ChatView/__tests__/useScrollMode.test.js @@ -30,6 +30,7 @@ import { readerInputActivatesDisclosure, readerInputMayScroll, readerInputNeedsFrameRelease, + releaseQuestionSubmissionForViewport, settledPinMode, shouldPinSend, } from '../useScrollMode.js' @@ -337,11 +338,34 @@ test('question submission freezes the visible row before same-turn output resume kind: 'ANCHOR_AT', key: 'assistant-with-question', offset: 60, - reserveTail: true, + questionSubmitViewportH: 600, + questionSubmitBaseMode: { kind: 'FOLLOW_BOTTOM' }, }, ) }) +test('question submission releases to the unanswered mode only after viewport size changes', () => { + const baseMode = { kind: 'PIN_USER_MSG', cid: 'latest' } + const heldMode = { + kind: 'ANCHOR_AT', + key: 'assistant-with-question', + offset: 60, + questionSubmitViewportH: 400, + questionSubmitBaseMode: baseMode, + } + + assert.equal( + releaseQuestionSubmissionForViewport(heldMode, 400), + heldMode, + 'same-size card reflow keeps the submit anchor exact', + ) + assert.equal( + releaseQuestionSubmissionForViewport(heldMode, 700), + baseMode, + 'keyboard growth restores the mode that owned the unanswered card', + ) +}) + test('question submission keeps the current mode when there is no visible row', () => { const current = { kind: 'FOLLOW_BOTTOM' } const scrollEl = { querySelectorAll() { return [] } } @@ -939,7 +963,7 @@ test('a saved partially-visible anchor remains exact', () => { 'an anchor whose row still intersects its restored viewport is preserved') }) -test('question-only tail reservation is never restored as durable reader state', () => { +test('question-only viewport overlay is never restored as durable reader state', () => { const row = { offsetTop: 500, offsetHeight: 220, @@ -949,7 +973,8 @@ test('question-only tail reservation is never restored as durable reader state', kind: 'ANCHOR_AT', key: 'assistant-question', offset: 100, - reserveTail: true, + questionSubmitViewportH: 400, + questionSubmitBaseMode: { kind: 'FOLLOW_BOTTOM' }, } const scrollEl = { clientHeight: 700, @@ -1352,7 +1377,8 @@ test('question submission reserves the exact room that keeps its anchor reachabl kind: 'ANCHOR_AT', key: 'assistant-question', offset: 60, - reserveTail: true, + questionSubmitViewportH: 600, + questionSubmitBaseMode: { kind: 'PIN_USER_MSG', cid: 'c-1' }, } const listEl = { offsetHeight: 1400 } const latestUser = { @@ -1368,7 +1394,51 @@ test('question submission reserves the exact room that keeps its anchor reachabl assert.equal( _computeSpacerH(scrollEl, listEl, latestUser, 700, mode), 440, - 'viewport growth adds exactly the room needed to preserve the anchor', + 'the same-viewport overlay keeps the exact anchor until resize releases it', + ) +}) + +test('answered question uses the unanswered card spacer when the keyboard closes', () => { + const anchor = { offsetTop: 1200, offsetHeight: 220 } + const scrollEl = { + clientHeight: 700, + querySelector(selector) { + return selector === '[data-key="assistant-question"]' ? anchor : null + }, + } + const baseMode = { kind: 'PIN_USER_MSG', cid: 'c-1' } + const heldMode = { + kind: 'ANCHOR_AT', + key: 'assistant-question', + offset: 60, + questionSubmitViewportH: 400, + questionSubmitBaseMode: baseMode, + } + const listEl = { offsetHeight: 1400 } + const latestUser = { + offsetTop: 1100, + offsetHeight: 80, + dataset: { cid: 'c-1' }, + } + + assert.equal( + _computeSpacerH(scrollEl, listEl, latestUser, 700, heldMode), + 440, + 'without release the answered card would remain locked', + ) + const released = releaseQuestionSubmissionForViewport(heldMode, 700) + const answeredSpacer = _computeSpacerH( + scrollEl, listEl, latestUser, 700, released, + ) + const unansweredSpacer = _computeSpacerH( + scrollEl, listEl, latestUser, 700, baseMode, + ) + assert.equal(answeredSpacer, unansweredSpacer) + assert.equal(answeredSpacer, 396) + assert.equal( + listEl.offsetHeight + answeredSpacer - scrollEl.clientHeight, + 1096, + 'ordinary geometry moves the card instead of preserving scrollTop 1140', ) }) diff --git a/frontend/src/components/ChatView/useScrollMode.js b/frontend/src/components/ChatView/useScrollMode.js index 61b139048..7725674ad 100644 --- a/frontend/src/components/ChatView/useScrollMode.js +++ b/frontend/src/components/ChatView/useScrollMode.js @@ -13,10 +13,11 @@ * — user msg at top (post-send), keyed on * the stable client `cid` (data-cid) * { kind: 'FOLLOW_BOTTOM' } — sticky-bottom for streaming - * { kind: 'ANCHOR_AT', key, offset, reserveTail? } + * { kind: 'ANCHOR_AT', key, offset, questionSubmitViewportH?, + * questionSubmitBaseMode? } * — anchored at a specific msg; an in-message - * question may temporarily reserve the - * exact room needed to keep it reachable + * question may temporarily preserve its + * submit-time position at one viewport size * * Send pinning has one rule for direct, queued, and steered messages: the * first visible user message always pins; every later message pins when the @@ -31,8 +32,9 @@ * restores it. PIN_USER_MSG may reserve before its row lands so a fresh send * can pin in one frame; every other ordinary mode reserves only while that * latest row is visible. The one explicit exception is the transient - * question-submit anchor: it reserves exactly enough tail room to keep its - * target reachable while the mobile viewport grows. + * question-submit anchor: it reserves exactly enough tail room for a stable + * same-viewport handoff. A keyboard resize restores the pre-submit mode before + * sizing, so the answered card moves exactly as the unanswered card would. * Gesture-driven bottom detection reads the scroll container's geometry in * the scroll event itself. There is no second sentinel/observer authority * that can lag behind the reader and contradict the current viewport. @@ -372,6 +374,39 @@ export function _anchorModeIntersectsContent(row, mode, viewportHeight) { && offset > -row.offsetHeight } + +function _durableQuestionSubmissionMode(mode) { + if (mode?.kind !== 'ANCHOR_AT') return mode + if (!Object.hasOwn(mode, 'questionSubmitViewportH') + && !Object.hasOwn(mode, 'questionSubmitBaseMode') + && !Object.hasOwn(mode, 'reserveTail')) { + return mode + } + const { + questionSubmitViewportH: _transientViewport, + questionSubmitBaseMode: _transientBaseMode, + reserveTail: _legacyTransientReservation, + ...durable + } = mode + return durable +} + + +/** A question answer temporarily overlays the reader's existing scroll mode + * only while the viewport size is unchanged. Keyboard movement belongs to the + * pre-submit mode: release the overlay before spacer sizing so the answered + * card receives the same resize behavior as the unanswered card. */ +export function releaseQuestionSubmissionForViewport(mode, viewportHeight) { + if (mode?.kind !== 'ANCHOR_AT' + || !Number.isFinite(mode.questionSubmitViewportH) + || !Number.isFinite(viewportHeight) + || Math.abs(mode.questionSubmitViewportH - viewportHeight) <= 1) { + return mode + } + return mode.questionSubmitBaseMode + || _durableQuestionSubmissionMode(mode) +} + /** The ANCHOR_AT twin of `_pinReapplyNeeded` — the SAME two-case repair. A * settled anchor drifts off its reader-chosen position when either the anchor * element's offsetTop SHIFTED (content grew above it) or scrollTop was CLAMPED @@ -422,10 +457,7 @@ export function _validateSavedMode(saved, messages, scrollEl) { // a huge negative offset while the viewport sat wholly in spacer below it. // Enforce the same content-intersection invariant used by spacer sizing, // self-healing every off-content restore to the real tail. - const durable = saved.reserveTail - ? { kind: 'ANCHOR_AT', key: saved.key, offset: saved.offset, - ...(saved.defaultTail ? { defaultTail: true } : {}) } - : saved + const durable = _durableQuestionSubmissionMode(saved) return _anchorModeIntersectsContent(row, durable, scrollEl?.clientHeight) ? durable : holdBottom() @@ -498,7 +530,8 @@ function _latestUserOwnsSpacer(scrollEl, listEl, lastUserMsgEl, mode, viewH) { /** Spacer height needed so the latest visible user message can sit near the * top of the viewport, with the PIN_OFFSET breathing room above it, or so a - * transient question-submit anchor remains reachable through viewport growth. + * transient question-submit anchor remains reachable while the submit-time + * viewport size is unchanged. * * Visibility is the defining invariant. The matching latest user pin may * reserve before placement; every other mode gets room only while its real @@ -521,8 +554,9 @@ function _latestUserOwnsSpacer(scrollEl, listEl, lastUserMsgEl, mode, viewH) { * * Once the latest user row leaves the viewport, ordinary reservation * collapses. An older visible user row never receives it. A question-submit - * anchor instead reserves only its exact reachability deficit; that transient - * intent is stripped before persistence. + * anchor instead reserves only its exact reachability deficit for a + * same-viewport handoff. A keyboard resize restores the mode that owned the + * unanswered card before this function runs again. */ const PIN_OFFSET = 4 const PIN_BOTTOM_ROOM = 0 @@ -535,7 +569,8 @@ export function _computeSpacerH( ) { if (!scrollEl || !listEl) return 0 const viewH = fullViewH || scrollEl.clientHeight - if (mode?.kind === 'ANCHOR_AT' && mode.reserveTail) { + if (mode?.kind === 'ANCHOR_AT' + && Number.isFinite(mode.questionSubmitViewportH)) { const anchorEl = _anchorEl(scrollEl, mode.key) if (!anchorEl) return 0 const anchorTarget = Math.max(0, anchorEl.offsetTop - mode.offset) @@ -794,11 +829,19 @@ export function modeForDisclosureToggle(scrollEl, currentMode) { * assistant row and may replace the card's controls immediately. It is not a * request to follow the live tail. Freeze the exact visible row/offset before * that card-to-stream handoff so neither the control reflow nor resumed output - * moves the reader. */ + * moves the reader. The overlay remembers the mode that owned the unanswered + * card and is scoped to the current viewport height; a keyboard resize returns + * to that base mode before layout is recomputed. */ export function modeForQuestionSubmission(scrollEl, currentMode) { if (!scrollEl) return currentMode const anchor = anchorModeFromScroll(scrollEl) - return anchor ? { ...anchor, reserveTail: true } : currentMode + if (!anchor) return currentMode + return { + ...anchor, + questionSubmitViewportH: scrollEl.clientHeight, + questionSubmitBaseMode: + currentMode?.questionSubmitBaseMode || currentMode, + } } @@ -1497,6 +1540,18 @@ export default function useScrollMode({ // forceApply so the current PIN/FOLLOW/ANCHOR survives the viewport clamp. function syncLayout({ forceApply = false, viewportChange = false } = {}) { const preserveBottom = viewportChange && nearScrollBottomRef.current + // Question submission freezes the card-to-stream handoff, not the + // keyboard. Restore the unanswered card's mode before sizing a changed + // viewport so its ordinary reservation and clamp remain authoritative. + if (viewportChange) { + const released = releaseQuestionSubmissionForViewport( + modeRef.current, + scrollEl.clientHeight, + ) + if (released !== modeRef.current) { + transitionMode(released, 'layout:question-viewport-release') + } + } sizeSpacer() // Input precedes the browser's first `scroll` event. Every layout entry // point shares this gate so streaming, footer reflow, keyboard resize, diff --git a/tests/chat-redesign.spec.mjs b/tests/chat-redesign.spec.mjs index 2c9a4d1fd..904efb3aa 100644 --- a/tests/chat-redesign.spec.mjs +++ b/tests/chat-redesign.spec.mjs @@ -563,7 +563,7 @@ test.describe('Q&A atomic write', () => { expect(questionFreeze.to?.kind).toBe('ANCHOR_AT') }) - test('an Android viewport growth cannot clamp a submitted question anchor', async ({ page }) => { + test('an Android viewport growth releases a submitted question to its unanswered mode', async ({ page }) => { const longLead = 'Context before the question. '.repeat(180) const streamBody = [ `data: ${JSON.stringify({ type: 'text', content: longLead })}\n\n`, @@ -651,12 +651,18 @@ test.describe('Q&A atomic write', () => { requestAnimationFrame(() => requestAnimationFrame(resolve)) ))) const after = await geometry() + const viewportRelease = await page.evaluate(() => ( + window.__mobiusChatScrollTrace?.transitions?.find( + row => row.event === 'layout:question-viewport-release', + ) || null + )) releaseAnswer() await submitClick expect(after.viewport).toBeGreaterThan(before.viewport) - expect(Math.abs(after.scrollTop - before.scrollTop)).toBeLessThanOrEqual(2) - expect(Math.abs(after.cardTop - before.cardTop)).toBeLessThanOrEqual(2) + expect(viewportRelease).toBeTruthy() + expect(viewportRelease.from?.kind).toBe('ANCHOR_AT') + expect(Math.abs(after.cardTop - before.cardTop)).toBeGreaterThan(2) }) })