From bc25df33ff6c4f44e94cd8a707d5b9993bd3f256 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Sat, 29 Aug 2026 07:28:10 +0000 Subject: [PATCH] fix(web): keep chat transcript pinned in WebKit (#3135) --- .../task/chat/message-list-native-scroll.ts | 3 +- .../task/chat/message-list-native.test.tsx | 4 +- .../webkit-transcript-auto-scroll/plan.md | 115 ++++++++++++++++++ .../task-01-use-webkit-safe-bottom-offset.md | 102 ++++++++++++++++ .../system-design/transcript-auto-scroll.md | 17 ++- 5 files changed, 233 insertions(+), 8 deletions(-) create mode 100644 docs/plans/webkit-transcript-auto-scroll/plan.md create mode 100644 docs/plans/webkit-transcript-auto-scroll/task-01-use-webkit-safe-bottom-offset.md diff --git a/apps/web/components/task/chat/message-list-native-scroll.ts b/apps/web/components/task/chat/message-list-native-scroll.ts index 382577f762..d57058c61d 100644 --- a/apps/web/components/task/chat/message-list-native-scroll.ts +++ b/apps/web/components/task/chat/message-list-native-scroll.ts @@ -25,7 +25,8 @@ import { createDebugLogger, isDebug } from "@/lib/debug/log"; const paginationDebug = createDebugLogger("messages:pagination"); -const NATIVE_BOTTOM_SCROLL_TOP = Number.MAX_SAFE_INTEGER; +// INT32_MAX: WebKit resolves Number.MAX_SAFE_INTEGER to 0 (not bottom). +const NATIVE_BOTTOM_SCROLL_TOP = 2_147_483_647; /** Writes a clamped maximum so the browser resolves the native bottom without * forcing a synchronous scrollHeight layout read. */ diff --git a/apps/web/components/task/chat/message-list-native.test.tsx b/apps/web/components/task/chat/message-list-native.test.tsx index 55c5f0b32a..a19a98b3be 100644 --- a/apps/web/components/task/chat/message-list-native.test.tsx +++ b/apps/web/components/task/chat/message-list-native.test.tsx @@ -407,7 +407,7 @@ describe("useScrollToDividerOrBottom — anchored-bar offset", () => { expect(scrollContainer.scrollTop).toBe(123); }); - it("pins an appended message without reading content size", () => { + it("uses a WebKit-safe maximum offset for pinned appends", () => { const { rerender } = render( , ); @@ -442,7 +442,7 @@ describe("useScrollToDividerOrBottom — anchored-bar offset", () => { ); }).not.toThrow(); expect(writes).toBe(1); - expect(scrollTop).toBe(Number.MAX_SAFE_INTEGER); + expect(scrollTop).toBe(2_147_483_647); }); it("restores the disabled offset after a transient layout clamp", () => { diff --git a/docs/plans/webkit-transcript-auto-scroll/plan.md b/docs/plans/webkit-transcript-auto-scroll/plan.md new file mode 100644 index 0000000000..edf3995011 --- /dev/null +++ b/docs/plans/webkit-transcript-auto-scroll/plan.md @@ -0,0 +1,115 @@ +--- +created: 2026-08-29 +status: done +requirements: + - REQ-UI-TRANSCRIPT-AUTO-SCROLL-001 +system_design: + - ../../specs/ui/system-design/transcript-auto-scroll.md +legacy_specs: [] +--- + +# Implementation Plan: Keep WebKit Transcripts at the Bottom + +## Overview + +Replace the unsafe bottom-scroll offset with the largest signed 32-bit integer. +Keep the write-only message path and all current scroll-owner guards. + +## Scope + +### In scope + +- Keep enabled transcripts at the bottom in Safari, Orion, Firefox, and + Chromium browsers. +- Preserve the write-only scroll command for live message commits. +- Add a regression test for the WebKit-safe offset. +- Keep the existing desktop and mobile scroll behavior. + +### Out of scope + +- Transcript controls, copy, layout, pagination, or navigation. +- Disabled auto-scroll behavior and saved scroll positions. +- A new Playwright browser project or a change to the CI browser matrix. +- Browser-specific user-agent branches. + +## Confirmed root cause + +[PR #3093](https://github.com/kdlbs/kandev/pull/3093) changed the native bottom +write from `scrollTop = scrollHeight` to `scrollTop = Number.MAX_SAFE_INTEGER`. +The change removed a synchronous layout read. + +A Playwright WebKit 26.5 probe used a container with a 900 px scroll range. +The old `scrollHeight` write produced 900. The new `Number.MAX_SAFE_INTEGER` +write produced 0. A `2_147_483_647` write produced 900. + +The live-message effect calls this helper after each streamed update. WebKit +therefore moves the transcript to the top after each update. Chromium and +Firefox clamp the same unsafe value to the bottom. + +## Technical approach + +### Bottom-scroll helper + +- Change `NATIVE_BOTTOM_SCROLL_TOP` in + `apps/web/components/task/chat/message-list-native-scroll.ts` to + `2_147_483_647`. +- Keep `scrollNativeToBottom` as one write-only operation. +- Keep the work-start, live-append, and catch-up call sites unchanged. + +### Regression test + +- Update `apps/web/components/task/chat/message-list-native.test.tsx`. +- Name the failing regression `uses a WebKit-safe maximum offset for pinned appends`. +- Make the `scrollHeight` getter throw to protect the write-only requirement. +- Record the setter value and require `2_147_483_647`. + +## Tests + +| Acceptance criterion | Test evidence | +| --- | --- | +| `AC-UI-TRANSCRIPT-AUTO-SCROLL-001.5` | The native-scroll regression requires a WebKit-safe bottom offset after an append. | +| `AC-UI-TRANSCRIPT-AUTO-SCROLL-001.7` | The same regression fails on any synchronous `scrollHeight` read. | + +## E2E tests + +- Desktop: run the enabled live-message scenario in + `apps/web/e2e/tests/chat/auto-scroll-toggle.spec.ts` with the `chromium` project. +- Mobile: run the matching scenario in + `apps/web/e2e/tests/chat/mobile-auto-scroll-toggle.spec.ts` with the + `mobile-chrome` project. +- WebKit: the direct browser probe proves the browser-specific offset behavior. + The unit regression applies that safe value to the product helper. + +## Mobile design contract + +- Desktop outcome: live messages keep the transcript at the bottom. +- Mobile entry point: the existing Chat tab in the full-height task layout. +- Nearest exemplar: `apps/web/components/task/task-layout.tsx` and the current + mobile auto-scroll test. +- Hierarchy and surface: the transcript remains the only vertical scroll owner. +- Shared logic: desktop and mobile use the same bottom-scroll helper. +- Mobile proof: the current mobile live-message scenario covers the shared path. + +## Work orders + +- [x] [Task 01: Use a WebKit-safe bottom offset](task-01-use-webkit-safe-bottom-offset.md) + +## Verification results + +- RED: the regression received `9_007_199_254_740_991` and expected + `2_147_483_647`. The other 26 focused tests passed. +- GREEN: all 27 native-scroll tests passed after the one-line correction. +- TypeScript typecheck and targeted ESLint passed. +- All specification files passed lint. The specification linter's 20 tests + passed. +- Desktop Chromium E2E passed one live-message scenario with retries disabled. +- Mobile Chrome E2E passed the matching Pixel 5 scenario with retries disabled. +- Direct probes showed that `2_147_483_647` reaches the bottom in Chromium 151, + Firefox 152, and WebKit 26.5. + +## Risks + +- A smaller sentinel can fail if a transcript exceeds that scroll range. + The signed 32-bit maximum is larger than a practical browser scroll range. +- A direct WebKit E2E project expands the CI browser matrix. + This fix keeps that expansion out of scope. diff --git a/docs/plans/webkit-transcript-auto-scroll/task-01-use-webkit-safe-bottom-offset.md b/docs/plans/webkit-transcript-auto-scroll/task-01-use-webkit-safe-bottom-offset.md new file mode 100644 index 0000000000..465eb959df --- /dev/null +++ b/docs/plans/webkit-transcript-auto-scroll/task-01-use-webkit-safe-bottom-offset.md @@ -0,0 +1,102 @@ +--- +id: "01-use-webkit-safe-bottom-offset" +title: "Use a WebKit-safe bottom offset" +status: done +wave: 1 +depends_on: [] +plan: "plan.md" +requirements: + - REQ-UI-TRANSCRIPT-AUTO-SCROLL-001 +acceptance_criteria: + - AC-UI-TRANSCRIPT-AUTO-SCROLL-001.5 + - AC-UI-TRANSCRIPT-AUTO-SCROLL-001.7 +system_design: + - ../../specs/ui/system-design/transcript-auto-scroll.md +--- + +# Task 01: Use a WebKit-safe Bottom Offset + +## Summary + +Use the largest signed 32-bit integer for write-only bottom placement. Add a +regression that protects WebKit behavior and the no-layout-read contract. + +## Failing regression first + +Add a test named `uses a WebKit-safe maximum offset for pinned appends`. +Require the setter to receive `2_147_483_647`. The current test receives +`Number.MAX_SAFE_INTEGER`, so the regression fails before the correction. + +## In scope + +- Change the native bottom offset constant. +- Keep `scrollNativeToBottom` write-only. +- Update the focused unit regression. +- Run the existing desktop and mobile live-message scenarios. +- Update the transcript auto-scroll design with the WebKit range constraint. + +## Out of scope + +- Other transcript scroll paths or controls. +- Pagination, prepend restoration, and explicit message navigation. +- New user-facing copy or responsive composition. +- Playwright configuration and CI browser installation. + +## Acceptance + +- Live append writes `2_147_483_647` without reading content size. +- Safari-compatible WebKit clamps the request to the transcript bottom. +- Existing desktop and mobile live-message scenarios remain green. + +## Verification + +```bash +cd apps && pnpm --filter @kandev/web test -- components/task/chat/message-list-native.test.tsx +cd apps && pnpm --filter @kandev/web run typecheck +cd apps/web && pnpm exec eslint components/task/chat/message-list-native-scroll.ts components/task/chat/message-list-native.test.tsx +python3 scripts/lint-spec-files.py --all +git diff --check -- docs/specs docs/plans apps/web/components/task/chat +cd apps/web && pnpm e2e:run --host --project chromium tests/chat/auto-scroll-toggle.spec.ts -- --grep "enabled auto-scroll stays at the bottom for live messages" --retries=0 +cd apps/web && pnpm e2e:run --host --no-build --project mobile-chrome tests/chat/mobile-auto-scroll-toggle.spec.ts -- --grep "enabled auto-scroll stays at the bottom for live messages on mobile" --retries=0 +``` + +## Files likely touched + +- `apps/web/components/task/chat/message-list-native-scroll.ts` +- `apps/web/components/task/chat/message-list-native.test.tsx` +- `docs/specs/ui/system-design/transcript-auto-scroll.md` +- `docs/plans/webkit-transcript-auto-scroll/plan.md` +- `docs/plans/webkit-transcript-auto-scroll/task-01-use-webkit-safe-bottom-offset.md` + +## Dependencies + +None. + +## Risks + +- The sentinel must remain positive after WebKit converts the scroll offset. +- A regression that only checks Chromium cannot detect this browser difference. + +## Parallelism + +`sequential` + +## Inputs + +- `REQ-UI-TRANSCRIPT-AUTO-SCROLL-001` acceptance criteria 5 and 7. +- The bottom-placement section in the transcript auto-scroll design. +- The existing native-scroll unit harness and live-message browser scenarios. + +## Results + +- Changed the native bottom sentinel from `Number.MAX_SAFE_INTEGER` to + `2_147_483_647`. +- Added a regression that keeps the append path write-only and requires the + WebKit-safe offset. +- The regression failed on the old sentinel and passed after the correction. +- All 27 focused unit tests passed. +- Typecheck, targeted ESLint, specification lint, and diff checks passed. +- The desktop Chromium and mobile Chrome live-message scenarios passed with + retries disabled. +- Direct Chromium, Firefox, and WebKit probes accepted the new offset and + placed the 900 px test container at its bottom. diff --git a/docs/specs/ui/system-design/transcript-auto-scroll.md b/docs/specs/ui/system-design/transcript-auto-scroll.md index efd2d5a79e..3429039781 100644 --- a/docs/specs/ui/system-design/transcript-auto-scroll.md +++ b/docs/specs/ui/system-design/transcript-auto-scroll.md @@ -4,6 +4,7 @@ system: ui requirements: - REQ-UI-TRANSCRIPT-AUTO-SCROLL-001 created: 2026-08-27 +updated: 2026-08-29 owners: - kandev --- @@ -31,12 +32,16 @@ common message-update and work-start paths call this helper. They do not read `scrollHeight`, `clientHeight`, a bounding rectangle, or computed style before the write. -The browser clamps the requested offset to the current maximum. The existing -near-bottom reference remains the decision input. The helper does not add a -second animation frame, smooth scrolling, or a resize observer. +The helper writes `2_147_483_647`, which is the largest signed 32-bit integer. +Chromium, Gecko, and WebKit clamp this positive offset to the current maximum. +The helper must not write `Number.MAX_SAFE_INTEGER`. WebKit resolves that value +to the top of the scroll container. + +The existing near-bottom reference remains the decision input. The helper does +not add a second animation frame, smooth scrolling, or a resize observer. Tests instrument the common append path. A `scrollHeight` getter fails the test -if that path reads it. The scroll setter records the maximum-offset request. +if that path reads it. The scroll setter records the WebKit-safe offset. ## Interaction boundaries @@ -73,8 +78,10 @@ change. ## Failure modes - If the container is not mounted, the helper performs no work. -- If a browser clamps a large requested offset, the result is the native +- Chromium, Gecko, and WebKit clamp the signed 32-bit maximum to the native maximum scroll position. +- An offset outside WebKit's safe range can resolve to zero and move the + transcript to the top. - If a layout restore or explicit navigation owns the scroll, the existing guards suppress bottom placement until that owner releases control.