You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maka.apache.org by "Astro-Han (via GitHub)" <gi...@apache.org> on 2026/08/28 17:31:23 UTC
[PR] perf(ui): stop the transcript from moving after a session switch [maka]
Astro-Han opened a new pull request, #4105:
URL: https://github.com/apache/maka/pull/4105
## Summary
Switching sessions moved the transcript under the reader, repeatedly, after it had already settled.
The cause was a single speculative prefetch. On arrival, `useChatScroll` scheduled a `requestIdleCallback` that fired about 150ms later and called `requestEarlier()` unconditionally, inserting earlier-history turns above the reader's scroll position. Those turns then inflated in waves at roughly 358ms, 590ms and 784ms as their content resolved. Nothing had asked for that history: the upward-scroll and wheel handlers already load it when the reader actually approaches the top.
Removing the prefetch left two hand-rolled mechanisms with nothing to do, and both are second implementations of an authority that already exists:
- `chat-scroll-anchor` captured a turn id and offset before content landed and restored it a frame later. Browser scroll anchoring already does this, and compensates during layout rather than a frame after it. Measured directly: inserting 1500px above the viewport mid-scroll moves the visible content 0px.
- `arrival-bottom-pin` re-implemented following the tail, which `chat-surface-layout` states Astryx owns. It was added in #2239 because `ChatLayoutContextValue` exposed only `scrollContainerRef` and `contentRef`, so Astryx's controller could not be reached. #2923 opened that seam for `unlockAutoFollow`, and the pin was never revisited.
Reading Astryx's controller, it already covers what six review rounds put into the pin: resize-synthetic scroll events are excluded by comparing `scrollHeight` and `offsetHeight`, a horizontal wheel is excluded by requiring `deltaY < 0`, and gestures are scoped by binding to the scroller itself rather than by testing where the pointer was. Its initial fill positions in one frame instead of springing from the top, which is what the pin's clamp existed to produce. The one gap was reachability again: on a conversation change the patch called `lock()`, which re-enters through the spring because the hook's initial-fill flag was consumed at mount. Asking for the instant jump directly closes it, in the patch that was already there.
`data-turn-window` went with the pin it gated: its `ready` state existed to release the pin, and the `fonts.ready` wait plus fifty markdown polls plus double rAF existed to time that release.
Net: 636 lines removed, two DOM state attributes gone, one gating chain gone, one cross-effect mutable-callback coupling gone. Added in their place: `overflow-anchor` stated on the transcript column so the dependency is legible rather than inherited from the default, a one-pixel nudge for the `scrollTop === 0` case where anchoring is suppressed, and one additive context field.
Refs #4099
## Verification
Measured with alternating within-instance A/B across four sessions, three repetitions each (within-instance sigma about 0.02; note that the same build across restarts varies far more, so only same-instance comparisons are used):
| configuration | mean CLS |
| --- | --- |
| before | 0.481–0.521 |
| prefetch removed | 0.085 |
| prefetch and hand-rolled anchoring removed | 0.090 |
| all three, verified on this branch | 0.089 |
The hand-rolled anchoring measured as contributing nothing once the prefetch was gone (byte-identical on three of four sessions), which is why it could only be removed after it. Handing the pin to Astryx did not change the result: 0.089 against 0.090. Every switch on this branch settles at `distanceFromBottom = 5px`, so Astryx does land the transcript at the bottom.
Not fixed and not claimed: sustained renderer CPU, which this does not explain and which is being investigated separately. First visit to a session is consistently worse than later visits — a cold-load shift source out of scope here.
Ran: `npm run format`, `tsc --noEmit -p packages/ui`, and the affected `@maka/ui` suites (14 tests). Not run: the full repository suite, and the `prompt-rail` E2E whose two waits on `data-turn-window` were retargeted at a mounted turn.
### Review focus
Two capabilities are given up, both in `arrival-bottom-pin`'s discrimination:
- A wheel or touch over the dock while the transcript is animating now releases following. The pin discriminated by gesture origin; Astryx binds to the scroller and does not.
- Returning to the bottom re-locks following. The pin's release was permanent for that arrival.
The patch additions (`scrollToBottom` on the context, and the instant jump on conversation change) are additive and intended to go upstream.
## AI use
Select exactly one:
- [ ] No generative tool made a substantive contribution
- [x] Generative tooling made a substantive contribution
Tool(s) and scope: Claude Code — root-cause investigation, the CDP measurement harness, the code changes, and this description. The contributor of record reviewed the final diff and owns the merge decision.
## Checklist
- [x] Tests cover the change and fail without it
- [x] Lint, format, typecheck and the affected suites pass locally
Does this PR entail a change in behavior?
- [x] Yes — described under Summary above
- [ ] No
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] perf(ui): stop the transcript from moving after a session switch [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han commented on code in PR #4105:
URL: https://github.com/apache/maka/pull/4105#discussion_r3882841124
##########
patches/@astryxdesign+core+0.5.0.patch:
##########
@@ -68,13 +74,18 @@ index ff9b9fa..346e01c 100644
+ return;
+ }
+ conversationKeyRef.current = conversationKey;
-+ scroll.lock();
++ // maka: a new conversation is a new initial fill, not content growth in
++ // the current one. `lock` alone re-enters through the spring, so the
++ // incoming transcript flies to the bottom instead of arriving there.
++ scroll.scrollToBottom({
Review Comment:
Confirmed and fixed in 96d274f63.
You are right about the sequence: `setActiveId` calls `setMessages([])` and `setMessageLoadPending(true)` in the same synchronous update that changes the key (`use-app-shell-session-workspace.ts:181`), so the scroller the effect sees holds only the loading placeholder.
My own measurements corroborate it and I misread them at the time: cold first visits to a session settled 677px from the bottom while warm switches settled at 5px. I attributed that to load cost. It was this path.
The fix arms the pending first fill again after the jump rather than deferring the jump, so both shapes are covered: an already-mounted transcript is positioned by the jump, and a late-arriving one by the first `scrollIfLocked` that sees scrollable content. That needed `resetInitialFill` on the hook, so the patch now touches `useChatStreamScroll` as well — it goes upstream with the other two additions.
Covered by `chat-conversation-swap-scroll.test.tsx`, which drives the empty-on-key-change then delayed-content sequence and asserts both that the arrival lands at the bottom and that it does so without scheduling a frame. Without `resetInitialFill` it fails with `scrollTop` 0 against an expected 4000.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] refactor(ui): make the transcript the only writer of its scroll position [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han commented on code in PR #4105:
URL: https://github.com/apache/maka/pull/4105#discussion_r3885932373
##########
apps/desktop/e2e/transcript-scroll.spec.ts:
##########
@@ -0,0 +1,408 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { expect, test, COMPOSER_INPUT } from './fixtures';
+import type { Page } from '@playwright/test';
+
+/**
+ * Where the transcript is looking, in a real Chromium with a real scroller.
+ *
+ * Two rounds of this work shipped green and wrong, both times because the
+ * instrument could not see the property being claimed: a CLS measurement is
+ * blind to scroll position, and a linkedom harness decides the effect ordering
+ * its own assertions then confirm. Nothing below reads a ref or a flag — each
+ * test states where an element or the viewport ended up, and the app has to put
+ * it there.
+ *
+ * Positions are asserted against an element or against the scroller's own end,
+ * never as a pixel delta: a delta is satisfiable by two wrongs (the content
+ * grew by as much as the view moved), which is the bug class that produced the
+ * `scrollHeight`-difference compensation this replaces.
+ */
+
+const SCROLLER = '[data-chat-scroll-container="true"]';
+const REGENERATE = /^重新生成回答/;
+/** Astryx's dock affordance, relabelled by `ChatSurfaceLayout`. */
+const SCROLL_TO_BOTTOM = /^滚动主对话到底部$/;
+
+/** Sixty lines: more than one viewport once the fake backend echoes it back. */
+const LONG_PROMPT = Array.from(
+ { length: 60 },
+ (_, index) => `第 ${index} 行:这一段用来把转录推过滚动视口的高度。`,
+).join('\n');
+
+function distanceToTail(page: Page): Promise<number> {
+ return scrollMetrics(page).then((metrics) => metrics.distance);
+}
+
+/**
+ * The distance plus the three numbers it came from. A failure that reports only
+ * the distance cannot say whether the transcript grew past the reader or the
+ * viewport shrank under them, and those have different causes.
+ */
+function scrollMetrics(page: Page): Promise<{
+ distance: number;
+ scrollTop: number;
+ scrollHeight: number;
+ clientHeight: number;
+}> {
+ return page.evaluate((selector) => {
+ const root = document.querySelector(selector);
+ if (!root) throw new Error('the chat scroll container is missing');
+ return {
+ distance: Math.round(root.scrollHeight - root.scrollTop - root.clientHeight),
+ scrollTop: Math.round(root.scrollTop),
+ scrollHeight: root.scrollHeight,
+ clientHeight: root.clientHeight,
+ };
+ }, SCROLLER);
+}
+
+/**
+ * Whether the dock affordance is actually offered. It is always in the DOM —
+ * Astryx toggles opacity and pointer-events — so presence proves nothing and
+ * `toBeVisible` passes on the transparent one.
+ */
+function scrollButtonOffered(page: Page): Promise<boolean> {
+ return page.evaluate((name) => {
+ const button = [...document.querySelectorAll('button')].find(
+ (candidate) => candidate.getAttribute('aria-label') === name
+ || candidate.textContent?.trim() === name,
+ );
+ if (!button) throw new Error(`the "${name}" affordance is missing`);
+ const style = getComputedStyle(button);
+ return style.pointerEvents !== 'none' && Number(style.opacity) > 0.5;
+ }, '滚动主对话到底部');
+}
+
+function turnTop(page: Page, turnId: string): Promise<number> {
+ return page.evaluate((id) => {
+ const turn = document.querySelector(`[data-turn-id="${CSS.escape(id)}"]`);
+ if (!turn) throw new Error(`turn ${id} is not mounted`);
+ return Math.round(turn.getBoundingClientRect().top);
+ }, turnId);
+}
+
+/**
+ * Sample the tail through the frames a growing transcript produces.
+ *
+ * Read at the start of each frame, which is one frame behind the pin: the
+ * content commits, the next frame's layout delivers the resize, and the write
+ * lands before that frame paints. So the view can only ever be behind by what
+ * arrived since the last delivery — never more, and never cumulatively. That is
+ * what `worstLag` against `worstFrameGrowth` states, and it is a property no
+ * fixed pixel budget can express: a transcript that stopped following instead
+ * falls behind by the whole of `grewBy`.
+ */
+function measureTailLag(page: Page, frames: number): Promise<{
+ worstLag: number;
+ worstFrameGrowth: number;
+ grewBy: number;
+ viewportHeight: number;
+}> {
+ return page.evaluate(([selector, frameCount]) => new Promise<{
+ worstLag: number;
+ worstFrameGrowth: number;
+ grewBy: number;
+ viewportHeight: number;
+ }>((resolve) => {
+ const root = document.querySelector(selector as string);
+ if (!root) throw new Error('the chat scroll container is missing');
+ const startedAt = root.scrollHeight;
+ let previousScrollHeight = startedAt;
+ let worstLag = 0;
+ let worstFrameGrowth = 0;
+ let left = frameCount as number;
+ const tick = (): void => {
+ const settledTail = previousScrollHeight - root.clientHeight;
+ worstLag = Math.max(worstLag, Math.abs(root.scrollTop - settledTail));
+ worstFrameGrowth = Math.max(worstFrameGrowth, root.scrollHeight - previousScrollHeight);
+ previousScrollHeight = root.scrollHeight;
+ // Stops on the content, not on a frame count: when the answer starts
+ // arriving is the backend's business, and a fixed window can expire
+ // before it does.
+ const enough = root.scrollHeight - startedAt > root.clientHeight;
+ if (enough || --left <= 0) {
+ resolve({
+ worstLag: Math.round(worstLag),
+ worstFrameGrowth: Math.round(worstFrameGrowth),
+ grewBy: Math.round(root.scrollHeight - startedAt),
+ viewportHeight: root.clientHeight,
+ });
+ } else requestAnimationFrame(tick);
+ };
+ requestAnimationFrame(tick);
+ }), [SCROLLER, frames] as const);
+}
+
+async function sendPrompt(page: Page, text: string): Promise<void> {
+ const composer = page.locator(COMPOSER_INPUT);
+ await composer.fill(text);
+ await composer.press('Enter');
+}
+
+/** Answered turns, so a second send can be waited for without a stale match. */
+function answeredTurns(page: Page) {
+ return page.getByRole('button', { name: REGENERATE });
+}
+
+async function scrollTranscriptTo(page: Page, top: number): Promise<void> {
+ await page.evaluate(([selector, position]) => {
+ const root = document.querySelector(selector as string);
+ if (!root) throw new Error('the chat scroll container is missing');
+ root.scrollTop = position as number;
+ }, [SCROLLER, top] as const);
+ await waitForPaintedFrames(page);
+}
+
+async function waitForPaintedFrames(page: Page, count = 3): Promise<void> {
+ await page.evaluate((frames) => new Promise<void>((resolve) => {
+ const tick = (left: number) => {
+ if (left <= 0) {
+ resolve();
+ return;
+ }
+ requestAnimationFrame(() => tick(left - 1));
+ };
+ tick(frames);
+ }), count);
+}
+
+test('a streaming answer keeps the viewport at the tail', async ({ window: page }) => {
+ // A full fake-backend turn, streamed nine characters at a time.
+ test.slow();
+ await page.setViewportSize({ width: 900, height: 700 });
+ await sendPrompt(page, LONG_PROMPT);
+
+ // Measured through the stream, not only at the end: the failure this guards
+ // against is the tail slipping away *while* content arrives, which a single
+ // reading afterwards cannot tell apart from a view dragged back at the last
+ // delta.
+ const lag = await measureTailLag(page, 1_200);
+ await expect(answeredTurns(page)).toHaveCount(1, { timeout: 30_000 });
+
+ // The samples have to have covered more than a viewport of real growth, or
+ // every reading above is a stationary transcript and proves nothing.
+ expect(lag.grewBy).toBeGreaterThan(lag.viewportHeight);
+ expect(lag.worstLag).toBeLessThanOrEqual(lag.worstFrameGrowth + 8);
+ const settled = await scrollMetrics(page);
+ expect(settled.distance, JSON.stringify(settled)).toBeLessThanOrEqual(4);
+ expect(await scrollButtonOffered(page)).toBe(false);
+});
+
+test('content that arrives after the reader scrolls up does not pull them back', async ({
+ window: page,
+}) => {
+ test.slow();
+ await page.setViewportSize({ width: 900, height: 700 });
+ await sendPrompt(page, LONG_PROMPT);
+ await expect(answeredTurns(page)).toHaveCount(1, { timeout: 30_000 });
+
+ const transcript = page.locator('.maka-chat-message-list');
+ await transcript.hover();
+ await page.mouse.wheel(0, -500);
+ await waitForPaintedFrames(page);
+ const before = await distanceToTail(page);
+ expect(before).toBeGreaterThan(100);
+ expect(await scrollButtonOffered(page)).toBe(true);
+
+ const anchorTurnId = await page.evaluate(() => {
+ const turn = document.querySelector<HTMLElement>('[data-turn-id]');
+ const turnId = turn?.dataset.turnId;
+ if (!turnId) throw new Error('the transcript has no mounted turn');
+ return turnId;
+ });
+ const anchorTop = await turnTop(page, anchorTurnId);
+
+ await sendPrompt(page, LONG_PROMPT);
+ await expect(answeredTurns(page)).toHaveCount(2, { timeout: 30_000 });
+ await waitForPaintedFrames(page);
+
+ // The turn the reader was on is still where it was. Everything that arrived,
+ // arrived below them.
+ expect(Math.abs((await turnTop(page, anchorTurnId)) - anchorTop)).toBeLessThanOrEqual(4);
+ expect(await distanceToTail(page)).toBeGreaterThan(before);
+});
+
+test('a gesture a nested scroller consumed does not release the tail', async ({
Review Comment:
CI is green on `94f1c031c` (`test` passed, 23m54s), so the 302 is closed: it was the defect described above, not the environment.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] refactor(ui): make the transcript the only writer of its scroll position [maka]
Posted by "zhiiw (via GitHub)" <gi...@apache.org>.
zhiiw commented on code in PR #4105:
URL: https://github.com/apache/maka/pull/4105#discussion_r3885811898
##########
apps/desktop/e2e/transcript-scroll.spec.ts:
##########
@@ -0,0 +1,408 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { expect, test, COMPOSER_INPUT } from './fixtures';
+import type { Page } from '@playwright/test';
+
+/**
+ * Where the transcript is looking, in a real Chromium with a real scroller.
+ *
+ * Two rounds of this work shipped green and wrong, both times because the
+ * instrument could not see the property being claimed: a CLS measurement is
+ * blind to scroll position, and a linkedom harness decides the effect ordering
+ * its own assertions then confirm. Nothing below reads a ref or a flag — each
+ * test states where an element or the viewport ended up, and the app has to put
+ * it there.
+ *
+ * Positions are asserted against an element or against the scroller's own end,
+ * never as a pixel delta: a delta is satisfiable by two wrongs (the content
+ * grew by as much as the view moved), which is the bug class that produced the
+ * `scrollHeight`-difference compensation this replaces.
+ */
+
+const SCROLLER = '[data-chat-scroll-container="true"]';
+const REGENERATE = /^重新生成回答/;
+/** Astryx's dock affordance, relabelled by `ChatSurfaceLayout`. */
+const SCROLL_TO_BOTTOM = /^滚动主对话到底部$/;
+
+/** Sixty lines: more than one viewport once the fake backend echoes it back. */
+const LONG_PROMPT = Array.from(
+ { length: 60 },
+ (_, index) => `第 ${index} 行:这一段用来把转录推过滚动视口的高度。`,
+).join('\n');
+
+function distanceToTail(page: Page): Promise<number> {
+ return scrollMetrics(page).then((metrics) => metrics.distance);
+}
+
+/**
+ * The distance plus the three numbers it came from. A failure that reports only
+ * the distance cannot say whether the transcript grew past the reader or the
+ * viewport shrank under them, and those have different causes.
+ */
+function scrollMetrics(page: Page): Promise<{
+ distance: number;
+ scrollTop: number;
+ scrollHeight: number;
+ clientHeight: number;
+}> {
+ return page.evaluate((selector) => {
+ const root = document.querySelector(selector);
+ if (!root) throw new Error('the chat scroll container is missing');
+ return {
+ distance: Math.round(root.scrollHeight - root.scrollTop - root.clientHeight),
+ scrollTop: Math.round(root.scrollTop),
+ scrollHeight: root.scrollHeight,
+ clientHeight: root.clientHeight,
+ };
+ }, SCROLLER);
+}
+
+/**
+ * Whether the dock affordance is actually offered. It is always in the DOM —
+ * Astryx toggles opacity and pointer-events — so presence proves nothing and
+ * `toBeVisible` passes on the transparent one.
+ */
+function scrollButtonOffered(page: Page): Promise<boolean> {
+ return page.evaluate((name) => {
+ const button = [...document.querySelectorAll('button')].find(
+ (candidate) => candidate.getAttribute('aria-label') === name
+ || candidate.textContent?.trim() === name,
+ );
+ if (!button) throw new Error(`the "${name}" affordance is missing`);
+ const style = getComputedStyle(button);
+ return style.pointerEvents !== 'none' && Number(style.opacity) > 0.5;
+ }, '滚动主对话到底部');
+}
+
+function turnTop(page: Page, turnId: string): Promise<number> {
+ return page.evaluate((id) => {
+ const turn = document.querySelector(`[data-turn-id="${CSS.escape(id)}"]`);
+ if (!turn) throw new Error(`turn ${id} is not mounted`);
+ return Math.round(turn.getBoundingClientRect().top);
+ }, turnId);
+}
+
+/**
+ * Sample the tail through the frames a growing transcript produces.
+ *
+ * Read at the start of each frame, which is one frame behind the pin: the
+ * content commits, the next frame's layout delivers the resize, and the write
+ * lands before that frame paints. So the view can only ever be behind by what
+ * arrived since the last delivery — never more, and never cumulatively. That is
+ * what `worstLag` against `worstFrameGrowth` states, and it is a property no
+ * fixed pixel budget can express: a transcript that stopped following instead
+ * falls behind by the whole of `grewBy`.
+ */
+function measureTailLag(page: Page, frames: number): Promise<{
+ worstLag: number;
+ worstFrameGrowth: number;
+ grewBy: number;
+ viewportHeight: number;
+}> {
+ return page.evaluate(([selector, frameCount]) => new Promise<{
+ worstLag: number;
+ worstFrameGrowth: number;
+ grewBy: number;
+ viewportHeight: number;
+ }>((resolve) => {
+ const root = document.querySelector(selector as string);
+ if (!root) throw new Error('the chat scroll container is missing');
+ const startedAt = root.scrollHeight;
+ let previousScrollHeight = startedAt;
+ let worstLag = 0;
+ let worstFrameGrowth = 0;
+ let left = frameCount as number;
+ const tick = (): void => {
+ const settledTail = previousScrollHeight - root.clientHeight;
+ worstLag = Math.max(worstLag, Math.abs(root.scrollTop - settledTail));
+ worstFrameGrowth = Math.max(worstFrameGrowth, root.scrollHeight - previousScrollHeight);
+ previousScrollHeight = root.scrollHeight;
+ // Stops on the content, not on a frame count: when the answer starts
+ // arriving is the backend's business, and a fixed window can expire
+ // before it does.
+ const enough = root.scrollHeight - startedAt > root.clientHeight;
+ if (enough || --left <= 0) {
+ resolve({
+ worstLag: Math.round(worstLag),
+ worstFrameGrowth: Math.round(worstFrameGrowth),
+ grewBy: Math.round(root.scrollHeight - startedAt),
+ viewportHeight: root.clientHeight,
+ });
+ } else requestAnimationFrame(tick);
+ };
+ requestAnimationFrame(tick);
+ }), [SCROLLER, frames] as const);
+}
+
+async function sendPrompt(page: Page, text: string): Promise<void> {
+ const composer = page.locator(COMPOSER_INPUT);
+ await composer.fill(text);
+ await composer.press('Enter');
+}
+
+/** Answered turns, so a second send can be waited for without a stale match. */
+function answeredTurns(page: Page) {
+ return page.getByRole('button', { name: REGENERATE });
+}
+
+async function scrollTranscriptTo(page: Page, top: number): Promise<void> {
+ await page.evaluate(([selector, position]) => {
+ const root = document.querySelector(selector as string);
+ if (!root) throw new Error('the chat scroll container is missing');
+ root.scrollTop = position as number;
+ }, [SCROLLER, top] as const);
+ await waitForPaintedFrames(page);
+}
+
+async function waitForPaintedFrames(page: Page, count = 3): Promise<void> {
+ await page.evaluate((frames) => new Promise<void>((resolve) => {
+ const tick = (left: number) => {
+ if (left <= 0) {
+ resolve();
+ return;
+ }
+ requestAnimationFrame(() => tick(left - 1));
+ };
+ tick(frames);
+ }), count);
+}
+
+test('a streaming answer keeps the viewport at the tail', async ({ window: page }) => {
Review Comment:
[P1] This spec fails in CI on the exact head (`test` run 33237483941, Linux/xvfb): `expect(settled.distance).toBeLessThanOrEqual(4)` received 302. The transcript does not settle at the tail in that environment, so the PR's headline evidence is currently unproven outside the author's machine.
##########
apps/desktop/e2e/transcript-scroll.spec.ts:
##########
@@ -0,0 +1,408 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { expect, test, COMPOSER_INPUT } from './fixtures';
+import type { Page } from '@playwright/test';
+
+/**
+ * Where the transcript is looking, in a real Chromium with a real scroller.
+ *
+ * Two rounds of this work shipped green and wrong, both times because the
+ * instrument could not see the property being claimed: a CLS measurement is
+ * blind to scroll position, and a linkedom harness decides the effect ordering
+ * its own assertions then confirm. Nothing below reads a ref or a flag — each
+ * test states where an element or the viewport ended up, and the app has to put
+ * it there.
+ *
+ * Positions are asserted against an element or against the scroller's own end,
+ * never as a pixel delta: a delta is satisfiable by two wrongs (the content
+ * grew by as much as the view moved), which is the bug class that produced the
+ * `scrollHeight`-difference compensation this replaces.
+ */
+
+const SCROLLER = '[data-chat-scroll-container="true"]';
+const REGENERATE = /^重新生成回答/;
+/** Astryx's dock affordance, relabelled by `ChatSurfaceLayout`. */
+const SCROLL_TO_BOTTOM = /^滚动主对话到底部$/;
+
+/** Sixty lines: more than one viewport once the fake backend echoes it back. */
+const LONG_PROMPT = Array.from(
+ { length: 60 },
+ (_, index) => `第 ${index} 行:这一段用来把转录推过滚动视口的高度。`,
+).join('\n');
+
+function distanceToTail(page: Page): Promise<number> {
+ return scrollMetrics(page).then((metrics) => metrics.distance);
+}
+
+/**
+ * The distance plus the three numbers it came from. A failure that reports only
+ * the distance cannot say whether the transcript grew past the reader or the
+ * viewport shrank under them, and those have different causes.
+ */
+function scrollMetrics(page: Page): Promise<{
+ distance: number;
+ scrollTop: number;
+ scrollHeight: number;
+ clientHeight: number;
+}> {
+ return page.evaluate((selector) => {
+ const root = document.querySelector(selector);
+ if (!root) throw new Error('the chat scroll container is missing');
+ return {
+ distance: Math.round(root.scrollHeight - root.scrollTop - root.clientHeight),
+ scrollTop: Math.round(root.scrollTop),
+ scrollHeight: root.scrollHeight,
+ clientHeight: root.clientHeight,
+ };
+ }, SCROLLER);
+}
+
+/**
+ * Whether the dock affordance is actually offered. It is always in the DOM —
+ * Astryx toggles opacity and pointer-events — so presence proves nothing and
+ * `toBeVisible` passes on the transparent one.
+ */
+function scrollButtonOffered(page: Page): Promise<boolean> {
+ return page.evaluate((name) => {
+ const button = [...document.querySelectorAll('button')].find(
+ (candidate) => candidate.getAttribute('aria-label') === name
+ || candidate.textContent?.trim() === name,
+ );
+ if (!button) throw new Error(`the "${name}" affordance is missing`);
+ const style = getComputedStyle(button);
+ return style.pointerEvents !== 'none' && Number(style.opacity) > 0.5;
+ }, '滚动主对话到底部');
+}
+
+function turnTop(page: Page, turnId: string): Promise<number> {
+ return page.evaluate((id) => {
+ const turn = document.querySelector(`[data-turn-id="${CSS.escape(id)}"]`);
+ if (!turn) throw new Error(`turn ${id} is not mounted`);
+ return Math.round(turn.getBoundingClientRect().top);
+ }, turnId);
+}
+
+/**
+ * Sample the tail through the frames a growing transcript produces.
+ *
+ * Read at the start of each frame, which is one frame behind the pin: the
+ * content commits, the next frame's layout delivers the resize, and the write
+ * lands before that frame paints. So the view can only ever be behind by what
+ * arrived since the last delivery — never more, and never cumulatively. That is
+ * what `worstLag` against `worstFrameGrowth` states, and it is a property no
+ * fixed pixel budget can express: a transcript that stopped following instead
+ * falls behind by the whole of `grewBy`.
+ */
+function measureTailLag(page: Page, frames: number): Promise<{
+ worstLag: number;
+ worstFrameGrowth: number;
+ grewBy: number;
+ viewportHeight: number;
+}> {
+ return page.evaluate(([selector, frameCount]) => new Promise<{
+ worstLag: number;
+ worstFrameGrowth: number;
+ grewBy: number;
+ viewportHeight: number;
+ }>((resolve) => {
+ const root = document.querySelector(selector as string);
+ if (!root) throw new Error('the chat scroll container is missing');
+ const startedAt = root.scrollHeight;
+ let previousScrollHeight = startedAt;
+ let worstLag = 0;
+ let worstFrameGrowth = 0;
+ let left = frameCount as number;
+ const tick = (): void => {
+ const settledTail = previousScrollHeight - root.clientHeight;
+ worstLag = Math.max(worstLag, Math.abs(root.scrollTop - settledTail));
+ worstFrameGrowth = Math.max(worstFrameGrowth, root.scrollHeight - previousScrollHeight);
+ previousScrollHeight = root.scrollHeight;
+ // Stops on the content, not on a frame count: when the answer starts
+ // arriving is the backend's business, and a fixed window can expire
+ // before it does.
+ const enough = root.scrollHeight - startedAt > root.clientHeight;
+ if (enough || --left <= 0) {
+ resolve({
+ worstLag: Math.round(worstLag),
+ worstFrameGrowth: Math.round(worstFrameGrowth),
+ grewBy: Math.round(root.scrollHeight - startedAt),
+ viewportHeight: root.clientHeight,
+ });
+ } else requestAnimationFrame(tick);
+ };
+ requestAnimationFrame(tick);
+ }), [SCROLLER, frames] as const);
+}
+
+async function sendPrompt(page: Page, text: string): Promise<void> {
+ const composer = page.locator(COMPOSER_INPUT);
+ await composer.fill(text);
+ await composer.press('Enter');
+}
+
+/** Answered turns, so a second send can be waited for without a stale match. */
+function answeredTurns(page: Page) {
+ return page.getByRole('button', { name: REGENERATE });
+}
+
+async function scrollTranscriptTo(page: Page, top: number): Promise<void> {
+ await page.evaluate(([selector, position]) => {
+ const root = document.querySelector(selector as string);
+ if (!root) throw new Error('the chat scroll container is missing');
+ root.scrollTop = position as number;
+ }, [SCROLLER, top] as const);
+ await waitForPaintedFrames(page);
+}
+
+async function waitForPaintedFrames(page: Page, count = 3): Promise<void> {
+ await page.evaluate((frames) => new Promise<void>((resolve) => {
+ const tick = (left: number) => {
+ if (left <= 0) {
+ resolve();
+ return;
+ }
+ requestAnimationFrame(() => tick(left - 1));
+ };
+ tick(frames);
+ }), count);
+}
+
+test('a streaming answer keeps the viewport at the tail', async ({ window: page }) => {
+ // A full fake-backend turn, streamed nine characters at a time.
+ test.slow();
+ await page.setViewportSize({ width: 900, height: 700 });
+ await sendPrompt(page, LONG_PROMPT);
+
+ // Measured through the stream, not only at the end: the failure this guards
+ // against is the tail slipping away *while* content arrives, which a single
+ // reading afterwards cannot tell apart from a view dragged back at the last
+ // delta.
+ const lag = await measureTailLag(page, 1_200);
+ await expect(answeredTurns(page)).toHaveCount(1, { timeout: 30_000 });
+
+ // The samples have to have covered more than a viewport of real growth, or
+ // every reading above is a stationary transcript and proves nothing.
+ expect(lag.grewBy).toBeGreaterThan(lag.viewportHeight);
+ expect(lag.worstLag).toBeLessThanOrEqual(lag.worstFrameGrowth + 8);
+ const settled = await scrollMetrics(page);
+ expect(settled.distance, JSON.stringify(settled)).toBeLessThanOrEqual(4);
+ expect(await scrollButtonOffered(page)).toBe(false);
+});
+
+test('content that arrives after the reader scrolls up does not pull them back', async ({
+ window: page,
+}) => {
+ test.slow();
+ await page.setViewportSize({ width: 900, height: 700 });
+ await sendPrompt(page, LONG_PROMPT);
+ await expect(answeredTurns(page)).toHaveCount(1, { timeout: 30_000 });
+
+ const transcript = page.locator('.maka-chat-message-list');
+ await transcript.hover();
+ await page.mouse.wheel(0, -500);
+ await waitForPaintedFrames(page);
+ const before = await distanceToTail(page);
+ expect(before).toBeGreaterThan(100);
+ expect(await scrollButtonOffered(page)).toBe(true);
+
+ const anchorTurnId = await page.evaluate(() => {
+ const turn = document.querySelector<HTMLElement>('[data-turn-id]');
+ const turnId = turn?.dataset.turnId;
+ if (!turnId) throw new Error('the transcript has no mounted turn');
+ return turnId;
+ });
+ const anchorTop = await turnTop(page, anchorTurnId);
+
+ await sendPrompt(page, LONG_PROMPT);
+ await expect(answeredTurns(page)).toHaveCount(2, { timeout: 30_000 });
+ await waitForPaintedFrames(page);
+
+ // The turn the reader was on is still where it was. Everything that arrived,
+ // arrived below them.
+ expect(Math.abs((await turnTop(page, anchorTurnId)) - anchorTop)).toBeLessThanOrEqual(4);
+ expect(await distanceToTail(page)).toBeGreaterThan(before);
+});
+
+test('a gesture a nested scroller consumed does not release the tail', async ({
Review Comment:
[P1] Same failure signature in CI on the exact head: distance 302 vs the ≤4 budget. Either the authority misses a settle path on this platform or the measurement is environment-fragile — the distinction needs answering before this spec can stand as evidence.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] refactor(ui): make the transcript the only writer of its scroll position [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han commented on code in PR #4105:
URL: https://github.com/apache/maka/pull/4105#discussion_r3885912355
##########
packages/ui/src/use-chat-scroll.ts:
##########
@@ -17,183 +17,107 @@
* under the License.
*/
+/**
+ * The transcript's scroll commands, and the seam that hands the scroller to the
+ * authority that owns it (`transcript-scroll-authority.ts`).
+ *
+ * A command is one-shot: jump to a turn the reader picked, or ask for earlier
+ * history. Each releases the pin first, and the authority writes nothing while
+ * the pin is released — so a command cannot be fighting a policy, which is the
+ * shape every previous round of this code had. Nothing here compensates for
+ * content that lands above the reader; `overflow-anchor: auto` does that
+ * continuously, and for free.
+ */
+
import { useEffect, useRef, useState, type RefObject } from 'react';
import type { StoredMessage } from '@maka/core/session';
-import { createArrivalBottomPin, type ArrivalBottomPin } from './arrival-bottom-pin.js';
-import { captureChatScrollAnchor, restoreChatScrollAnchor } from './chat-scroll-anchor.js';
+import { useTranscriptScrollAuthority } from './transcript-scroll-authority.js';
export function useChatScroll(input: {
scrollRef: RefObject<HTMLElement | null>;
sessionId?: string;
- hasTurns: boolean;
messages: readonly StoredMessage[];
target?: { turnId: string; nonce: number };
behavior: ScrollBehavior;
hasOlderHistory?: boolean;
- historyLoadPending?: boolean;
onLoadEarlierHistory?(): Promise<void> | void;
- latestNavigationNonce?: number;
}) {
const [highlightedTurnId, setHighlightedTurnId] = useState<string | null>(null);
- const arrivalPin = useRef<ArrivalBottomPin | null>(null);
+ const authority = useTranscriptScrollAuthority();
const loadEarlierRef = useRef(input.onLoadEarlierHistory);
loadEarlierRef.current = input.onLoadEarlierHistory;
- const sessionIdRef = useRef(input.sessionId);
- sessionIdRef.current = input.sessionId;
- const historyLoadPendingRef = useRef(input.historyLoadPending);
- historyLoadPendingRef.current = input.historyLoadPending;
const canLoadEarlier = input.onLoadEarlierHistory !== undefined;
- const earlierLoadRequest = useRef<object | null>(null);
- const requestEarlierRef = useRef<() => void>(() => {});
+ const handledTarget = useRef<string | null>(null);
+ // A passive effect, not a layout one: the scroller is Astryx's layout root,
+ // an ancestor, and React attaches a parent's ref after its children's layout
+ // effects have already run. The growth signal is a ResizeObserver delivery,
+ // which lands after passive effects, so this is still installed in time.
+ useEffect(() => authority.attach(input.scrollRef.current), [authority, input.scrollRef]);
+
+ // A new conversation arrives at its tail. Nothing special positions it: the
+ // pin is set here and the first fill is growth like any other, so it takes
+ // the one path instead of a first-fill path of its own.
useEffect(() => {
- earlierLoadRequest.current = null;
+ authority.pinToTail();
}, [input.sessionId]);
useEffect(() => {
const root = input.scrollRef.current;
if (!root || !input.hasOlderHistory || !canLoadEarlier) return;
- let previousScrollTop = root.scrollTop;
+ // Asking twice is the loader's problem, not this one's: it refuses a
+ // request while one is in flight, and asking for history the reader
+ // already has is idempotent anyway.
const requestEarlier = (): void => {
- if (historyLoadPendingRef.current || earlierLoadRequest.current) return;
- const scrollHeight = root.scrollHeight;
- const anchor = captureChatScrollAnchor(root);
- const sessionId = sessionIdRef.current;
- const request = {};
- earlierLoadRequest.current = request;
- arrivalPin.current?.release();
- void Promise.resolve(loadEarlierRef.current?.()).catch(() => undefined).finally(() => {
- window.requestAnimationFrame(() => {
- if (
- earlierLoadRequest.current === request &&
- sessionIdRef.current === sessionId &&
- input.scrollRef.current === root &&
- root.isConnected &&
- !restoreChatScrollAnchor(root, anchor)
- ) {
- root.scrollTop += root.scrollHeight - scrollHeight;
- }
- if (earlierLoadRequest.current === request) earlierLoadRequest.current = null;
- });
- });
+ // Nothing here touches the pin. Asking for history is not a decision
+ // about where the viewport belongs: if the reader scrolled up to ask,
+ // the authority already saw that scroll and released; if the transcript
+ // is merely short enough that its tail is also near its start, they are
+ // still following it and must keep following it.
+ //
+ // The browser anchors the reader against everything that lands above
+ // them, with one exception: it declines while the scroller sits at zero,
+ // which is exactly where a wheel asks for history. One pixel is the whole
+ // fix — measured in Chromium, an insert of 501px above the reader moves
+ // `scrollTop` by 501 at an offset of 1 and by 0 at an offset of 0.
+ if (root.scrollTop < 1) root.scrollTop = 1;
+ void Promise.resolve(loadEarlierRef.current?.()).catch(() => undefined);
};
+ // Position, not direction: a shrinking transcript also lowers `scrollTop`.
const nearStart = (): boolean =>
root.scrollTop <= Math.max(640, root.clientHeight * 2);
const onScroll = (): void => {
- const nextScrollTop = root.scrollTop;
- if (nextScrollTop < previousScrollTop && nearStart()) requestEarlier();
- previousScrollTop = nextScrollTop;
+ if (nearStart()) requestEarlier();
Review Comment:
Confirmed, and fixed together with your other note in `94f1c031c` — both are the same cause.
You are right that dropping `main`'s upward check made a pinned write enough to trigger this. The band contains the tail on any transcript shorter than about three viewports, so a session switch asked for history nobody wanted: the same speculative insert this branch's first commit removed.
I did not restore the `nextScrollTop < previousScrollTop` comparison, though — that is a second reading of the same event the authority is already interpreting, and re-deriving it here is what put this file and the pin out of step in the first place. The authority now publishes the distinction instead (`subscribeToReaderScroll`), and this handler subscribes to it rather than to `scroll`. It fires for the reader and for nothing else: not the authority's own tail writes, not native anchoring compensating for turns that landed above, not growth that outran a write, not the browser clamping the offset on a viewport resize. Told apart by scroll geometry — an event arriving with `scrollHeight` or `clientHeight` changed since the last one moved the reader's offset without their intent changing.
That last case was not hypothetical: with only `!pinned` guarding this handler, `setViewportSize` alone loaded the entire history in the E2E, because the clamp the browser applies on resize read as a gesture.
Covered by `following the tail does not ask for the history above it`, which fails without the change (the fixture loads 120 turns nobody asked for).
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] perf(ui): stop the transcript from moving after a session switch [maka]
Posted by "M4n5ter (via GitHub)" <gi...@apache.org>.
M4n5ter commented on code in PR #4105:
URL: https://github.com/apache/maka/pull/4105#discussion_r3882777021
##########
patches/@astryxdesign+core+0.5.0.patch:
##########
@@ -68,13 +74,18 @@ index ff9b9fa..346e01c 100644
+ return;
+ }
+ conversationKeyRef.current = conversationKey;
-+ scroll.lock();
++ // maka: a new conversation is a new initial fill, not content growth in
++ // the current one. `lock` alone re-enters through the spring, so the
++ // incoming transcript flies to the bottom instead of arriving there.
++ scroll.scrollToBottom({
Review Comment:
[P2] Preserve the new conversation's pending first fill until content is scrollable.
The normal session-switch path reaches this effect before the new transcript exists: `setActiveId()` synchronously clears `messages`, marks loading, and changes the key; `useActiveSessionEvents` fills the transcript later. Calling `scrollToBottom({ behavior: 'instant' })` here unconditionally clears `useChatStreamScroll`'s `initialFillPendingRef`, even when the empty/loading scroller has no overflow. When the transcript then arrives and ResizeObserver calls `scrollIfLocked()`, it takes the spring path instead of the synchronous first-fill jump. A production-hook probe with 400px of loading content followed by a 1,400px transcript left `scrollTop` at 0 (bottom was 1,000) and queued an animation frame.
This means a cold or delayed session switch can still visibly fly from the top to the bottom. Please reset the pending first-fill state on conversation change, or defer the instant jump until the new conversation is scrollable, and cover the empty-on-key-change → delayed-content sequence.
---
_Posted by an automated review agent operated by @M4n5ter. This is not an
independent human review and does not satisfy the committer review required by
CONTRIBUTING.md. A human is accountable for this comment — please push back if
anything here is wrong._
<details><summary>简体中文</summary>
_本条评论由 @M4n5ter 运行的自动化审查程序发出。它不构成 CONTRIBUTING.md
所要求的独立人类审查,也不能替代人类审查。有人类对本条评论负责,如有错误请直接指出。_
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] refactor(ui): make the transcript the only writer of its scroll position [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han commented on code in PR #4105:
URL: https://github.com/apache/maka/pull/4105#discussion_r3885913872
##########
packages/ui/src/use-chat-scroll.ts:
##########
@@ -17,183 +17,107 @@
* under the License.
*/
+/**
+ * The transcript's scroll commands, and the seam that hands the scroller to the
+ * authority that owns it (`transcript-scroll-authority.ts`).
+ *
+ * A command is one-shot: jump to a turn the reader picked, or ask for earlier
+ * history. Each releases the pin first, and the authority writes nothing while
+ * the pin is released — so a command cannot be fighting a policy, which is the
+ * shape every previous round of this code had. Nothing here compensates for
+ * content that lands above the reader; `overflow-anchor: auto` does that
+ * continuously, and for free.
+ */
+
import { useEffect, useRef, useState, type RefObject } from 'react';
import type { StoredMessage } from '@maka/core/session';
-import { createArrivalBottomPin, type ArrivalBottomPin } from './arrival-bottom-pin.js';
-import { captureChatScrollAnchor, restoreChatScrollAnchor } from './chat-scroll-anchor.js';
+import { useTranscriptScrollAuthority } from './transcript-scroll-authority.js';
export function useChatScroll(input: {
scrollRef: RefObject<HTMLElement | null>;
sessionId?: string;
- hasTurns: boolean;
messages: readonly StoredMessage[];
target?: { turnId: string; nonce: number };
behavior: ScrollBehavior;
hasOlderHistory?: boolean;
- historyLoadPending?: boolean;
onLoadEarlierHistory?(): Promise<void> | void;
- latestNavigationNonce?: number;
}) {
const [highlightedTurnId, setHighlightedTurnId] = useState<string | null>(null);
- const arrivalPin = useRef<ArrivalBottomPin | null>(null);
+ const authority = useTranscriptScrollAuthority();
const loadEarlierRef = useRef(input.onLoadEarlierHistory);
loadEarlierRef.current = input.onLoadEarlierHistory;
- const sessionIdRef = useRef(input.sessionId);
- sessionIdRef.current = input.sessionId;
- const historyLoadPendingRef = useRef(input.historyLoadPending);
- historyLoadPendingRef.current = input.historyLoadPending;
const canLoadEarlier = input.onLoadEarlierHistory !== undefined;
- const earlierLoadRequest = useRef<object | null>(null);
- const requestEarlierRef = useRef<() => void>(() => {});
+ const handledTarget = useRef<string | null>(null);
+ // A passive effect, not a layout one: the scroller is Astryx's layout root,
+ // an ancestor, and React attaches a parent's ref after its children's layout
+ // effects have already run. The growth signal is a ResizeObserver delivery,
+ // which lands after passive effects, so this is still installed in time.
+ useEffect(() => authority.attach(input.scrollRef.current), [authority, input.scrollRef]);
+
+ // A new conversation arrives at its tail. Nothing special positions it: the
+ // pin is set here and the first fill is growth like any other, so it takes
+ // the one path instead of a first-fill path of its own.
useEffect(() => {
- earlierLoadRequest.current = null;
+ authority.pinToTail();
}, [input.sessionId]);
useEffect(() => {
const root = input.scrollRef.current;
if (!root || !input.hasOlderHistory || !canLoadEarlier) return;
- let previousScrollTop = root.scrollTop;
+ // Asking twice is the loader's problem, not this one's: it refuses a
+ // request while one is in flight, and asking for history the reader
+ // already has is idempotent anyway.
const requestEarlier = (): void => {
- if (historyLoadPendingRef.current || earlierLoadRequest.current) return;
- const scrollHeight = root.scrollHeight;
- const anchor = captureChatScrollAnchor(root);
- const sessionId = sessionIdRef.current;
- const request = {};
- earlierLoadRequest.current = request;
- arrivalPin.current?.release();
- void Promise.resolve(loadEarlierRef.current?.()).catch(() => undefined).finally(() => {
- window.requestAnimationFrame(() => {
- if (
- earlierLoadRequest.current === request &&
- sessionIdRef.current === sessionId &&
- input.scrollRef.current === root &&
- root.isConnected &&
- !restoreChatScrollAnchor(root, anchor)
- ) {
- root.scrollTop += root.scrollHeight - scrollHeight;
- }
- if (earlierLoadRequest.current === request) earlierLoadRequest.current = null;
- });
- });
+ // Nothing here touches the pin. Asking for history is not a decision
+ // about where the viewport belongs: if the reader scrolled up to ask,
+ // the authority already saw that scroll and released; if the transcript
+ // is merely short enough that its tail is also near its start, they are
+ // still following it and must keep following it.
Review Comment:
Confirmed, and fixed in `94f1c031c` — same commit as your note above, because it is the same cause.
The wheel-at-zero half is exactly right and now reads as you describe: `deltaY < 0` near the start releases the pin and asks for history. There is no `scroll` to observe there — the scroller cannot move — so the gesture had to be caught where it happens. `wheel` is also the only one of these that bubbles, which is why it can be a listener on the root at all.
The other half you name — the request should not fire while they are at the tail — is fixed at the authority rather than here, because "was that the reader" is its question and this file was answering it a second time. It now publishes `subscribeToReaderScroll`, which fires for the reader and for nothing else, and this handler subscribes to that instead of to `scroll`.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
##########
apps/desktop/e2e/transcript-scroll.spec.ts:
##########
@@ -0,0 +1,408 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { expect, test, COMPOSER_INPUT } from './fixtures';
+import type { Page } from '@playwright/test';
+
+/**
+ * Where the transcript is looking, in a real Chromium with a real scroller.
+ *
+ * Two rounds of this work shipped green and wrong, both times because the
+ * instrument could not see the property being claimed: a CLS measurement is
+ * blind to scroll position, and a linkedom harness decides the effect ordering
+ * its own assertions then confirm. Nothing below reads a ref or a flag — each
+ * test states where an element or the viewport ended up, and the app has to put
+ * it there.
+ *
+ * Positions are asserted against an element or against the scroller's own end,
+ * never as a pixel delta: a delta is satisfiable by two wrongs (the content
+ * grew by as much as the view moved), which is the bug class that produced the
+ * `scrollHeight`-difference compensation this replaces.
+ */
+
+const SCROLLER = '[data-chat-scroll-container="true"]';
+const REGENERATE = /^重新生成回答/;
+/** Astryx's dock affordance, relabelled by `ChatSurfaceLayout`. */
+const SCROLL_TO_BOTTOM = /^滚动主对话到底部$/;
+
+/** Sixty lines: more than one viewport once the fake backend echoes it back. */
+const LONG_PROMPT = Array.from(
+ { length: 60 },
+ (_, index) => `第 ${index} 行:这一段用来把转录推过滚动视口的高度。`,
+).join('\n');
+
+function distanceToTail(page: Page): Promise<number> {
+ return scrollMetrics(page).then((metrics) => metrics.distance);
+}
+
+/**
+ * The distance plus the three numbers it came from. A failure that reports only
+ * the distance cannot say whether the transcript grew past the reader or the
+ * viewport shrank under them, and those have different causes.
+ */
+function scrollMetrics(page: Page): Promise<{
+ distance: number;
+ scrollTop: number;
+ scrollHeight: number;
+ clientHeight: number;
+}> {
+ return page.evaluate((selector) => {
+ const root = document.querySelector(selector);
+ if (!root) throw new Error('the chat scroll container is missing');
+ return {
+ distance: Math.round(root.scrollHeight - root.scrollTop - root.clientHeight),
+ scrollTop: Math.round(root.scrollTop),
+ scrollHeight: root.scrollHeight,
+ clientHeight: root.clientHeight,
+ };
+ }, SCROLLER);
+}
+
+/**
+ * Whether the dock affordance is actually offered. It is always in the DOM —
+ * Astryx toggles opacity and pointer-events — so presence proves nothing and
+ * `toBeVisible` passes on the transparent one.
+ */
+function scrollButtonOffered(page: Page): Promise<boolean> {
+ return page.evaluate((name) => {
+ const button = [...document.querySelectorAll('button')].find(
+ (candidate) => candidate.getAttribute('aria-label') === name
+ || candidate.textContent?.trim() === name,
+ );
+ if (!button) throw new Error(`the "${name}" affordance is missing`);
+ const style = getComputedStyle(button);
+ return style.pointerEvents !== 'none' && Number(style.opacity) > 0.5;
+ }, '滚动主对话到底部');
+}
+
+function turnTop(page: Page, turnId: string): Promise<number> {
+ return page.evaluate((id) => {
+ const turn = document.querySelector(`[data-turn-id="${CSS.escape(id)}"]`);
+ if (!turn) throw new Error(`turn ${id} is not mounted`);
+ return Math.round(turn.getBoundingClientRect().top);
+ }, turnId);
+}
+
+/**
+ * Sample the tail through the frames a growing transcript produces.
+ *
+ * Read at the start of each frame, which is one frame behind the pin: the
+ * content commits, the next frame's layout delivers the resize, and the write
+ * lands before that frame paints. So the view can only ever be behind by what
+ * arrived since the last delivery — never more, and never cumulatively. That is
+ * what `worstLag` against `worstFrameGrowth` states, and it is a property no
+ * fixed pixel budget can express: a transcript that stopped following instead
+ * falls behind by the whole of `grewBy`.
+ */
+function measureTailLag(page: Page, frames: number): Promise<{
+ worstLag: number;
+ worstFrameGrowth: number;
+ grewBy: number;
+ viewportHeight: number;
+}> {
+ return page.evaluate(([selector, frameCount]) => new Promise<{
+ worstLag: number;
+ worstFrameGrowth: number;
+ grewBy: number;
+ viewportHeight: number;
+ }>((resolve) => {
+ const root = document.querySelector(selector as string);
+ if (!root) throw new Error('the chat scroll container is missing');
+ const startedAt = root.scrollHeight;
+ let previousScrollHeight = startedAt;
+ let worstLag = 0;
+ let worstFrameGrowth = 0;
+ let left = frameCount as number;
+ const tick = (): void => {
+ const settledTail = previousScrollHeight - root.clientHeight;
+ worstLag = Math.max(worstLag, Math.abs(root.scrollTop - settledTail));
+ worstFrameGrowth = Math.max(worstFrameGrowth, root.scrollHeight - previousScrollHeight);
+ previousScrollHeight = root.scrollHeight;
+ // Stops on the content, not on a frame count: when the answer starts
+ // arriving is the backend's business, and a fixed window can expire
+ // before it does.
+ const enough = root.scrollHeight - startedAt > root.clientHeight;
+ if (enough || --left <= 0) {
+ resolve({
+ worstLag: Math.round(worstLag),
+ worstFrameGrowth: Math.round(worstFrameGrowth),
+ grewBy: Math.round(root.scrollHeight - startedAt),
+ viewportHeight: root.clientHeight,
+ });
+ } else requestAnimationFrame(tick);
+ };
+ requestAnimationFrame(tick);
+ }), [SCROLLER, frames] as const);
+}
+
+async function sendPrompt(page: Page, text: string): Promise<void> {
+ const composer = page.locator(COMPOSER_INPUT);
+ await composer.fill(text);
+ await composer.press('Enter');
+}
+
+/** Answered turns, so a second send can be waited for without a stale match. */
+function answeredTurns(page: Page) {
+ return page.getByRole('button', { name: REGENERATE });
+}
+
+async function scrollTranscriptTo(page: Page, top: number): Promise<void> {
+ await page.evaluate(([selector, position]) => {
+ const root = document.querySelector(selector as string);
+ if (!root) throw new Error('the chat scroll container is missing');
+ root.scrollTop = position as number;
+ }, [SCROLLER, top] as const);
+ await waitForPaintedFrames(page);
+}
+
+async function waitForPaintedFrames(page: Page, count = 3): Promise<void> {
+ await page.evaluate((frames) => new Promise<void>((resolve) => {
+ const tick = (left: number) => {
+ if (left <= 0) {
+ resolve();
+ return;
+ }
+ requestAnimationFrame(() => tick(left - 1));
+ };
+ tick(frames);
+ }), count);
+}
+
+test('a streaming answer keeps the viewport at the tail', async ({ window: page }) => {
Review Comment:
You were right to hold the spec to this, and the failure was the product, not the environment.
Cause: `pinned` was re-derived from position on every scroll event the authority did not recognise as its own echo. Native anchoring compensating for turns landing above the reader, growth that outran a tail write, and the browser clamping the offset on a viewport resize all arrive as ordinary `scroll` events at an offset the authority did not write — so each one silently re-answered "is the reader following the tail", and each one could answer no. On Linux/xvfb the timing made that land 302px short; locally it did not. Same defect either way.
Fixed in `94f1c031c`. An event arriving with `scrollHeight` or `clientHeight` changed since the last one moved the reader's offset without changing their intent, so the pin is left alone and only the button's distance is recomputed. The reader's own gesture arrives with the geometry unchanged, and only that touches the pin.
Two earlier attempts on this branch did not fix it — a viewport `ResizeObserver`, then position-based echo detection — and both returned the same 302. They are in the history as separate commits with what they actually did claim; neither is credited with this.
Worth noting what your two comments proved beyond their own subject: the spec did its job. It failed on a real defect in an environment I do not have, and the number it printed (`distance`, `scrollTop`, `scrollHeight`, `clientHeight`) is what made the third diagnosis the right one.
CI on `94f1c031c` is still running; I will not claim this closed until that check is green.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
##########
apps/desktop/e2e/transcript-scroll.spec.ts:
##########
@@ -0,0 +1,408 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { expect, test, COMPOSER_INPUT } from './fixtures';
+import type { Page } from '@playwright/test';
+
+/**
+ * Where the transcript is looking, in a real Chromium with a real scroller.
+ *
+ * Two rounds of this work shipped green and wrong, both times because the
+ * instrument could not see the property being claimed: a CLS measurement is
+ * blind to scroll position, and a linkedom harness decides the effect ordering
+ * its own assertions then confirm. Nothing below reads a ref or a flag — each
+ * test states where an element or the viewport ended up, and the app has to put
+ * it there.
+ *
+ * Positions are asserted against an element or against the scroller's own end,
+ * never as a pixel delta: a delta is satisfiable by two wrongs (the content
+ * grew by as much as the view moved), which is the bug class that produced the
+ * `scrollHeight`-difference compensation this replaces.
+ */
+
+const SCROLLER = '[data-chat-scroll-container="true"]';
+const REGENERATE = /^重新生成回答/;
+/** Astryx's dock affordance, relabelled by `ChatSurfaceLayout`. */
+const SCROLL_TO_BOTTOM = /^滚动主对话到底部$/;
+
+/** Sixty lines: more than one viewport once the fake backend echoes it back. */
+const LONG_PROMPT = Array.from(
+ { length: 60 },
+ (_, index) => `第 ${index} 行:这一段用来把转录推过滚动视口的高度。`,
+).join('\n');
+
+function distanceToTail(page: Page): Promise<number> {
+ return scrollMetrics(page).then((metrics) => metrics.distance);
+}
+
+/**
+ * The distance plus the three numbers it came from. A failure that reports only
+ * the distance cannot say whether the transcript grew past the reader or the
+ * viewport shrank under them, and those have different causes.
+ */
+function scrollMetrics(page: Page): Promise<{
+ distance: number;
+ scrollTop: number;
+ scrollHeight: number;
+ clientHeight: number;
+}> {
+ return page.evaluate((selector) => {
+ const root = document.querySelector(selector);
+ if (!root) throw new Error('the chat scroll container is missing');
+ return {
+ distance: Math.round(root.scrollHeight - root.scrollTop - root.clientHeight),
+ scrollTop: Math.round(root.scrollTop),
+ scrollHeight: root.scrollHeight,
+ clientHeight: root.clientHeight,
+ };
+ }, SCROLLER);
+}
+
+/**
+ * Whether the dock affordance is actually offered. It is always in the DOM —
+ * Astryx toggles opacity and pointer-events — so presence proves nothing and
+ * `toBeVisible` passes on the transparent one.
+ */
+function scrollButtonOffered(page: Page): Promise<boolean> {
+ return page.evaluate((name) => {
+ const button = [...document.querySelectorAll('button')].find(
+ (candidate) => candidate.getAttribute('aria-label') === name
+ || candidate.textContent?.trim() === name,
+ );
+ if (!button) throw new Error(`the "${name}" affordance is missing`);
+ const style = getComputedStyle(button);
+ return style.pointerEvents !== 'none' && Number(style.opacity) > 0.5;
+ }, '滚动主对话到底部');
+}
+
+function turnTop(page: Page, turnId: string): Promise<number> {
+ return page.evaluate((id) => {
+ const turn = document.querySelector(`[data-turn-id="${CSS.escape(id)}"]`);
+ if (!turn) throw new Error(`turn ${id} is not mounted`);
+ return Math.round(turn.getBoundingClientRect().top);
+ }, turnId);
+}
+
+/**
+ * Sample the tail through the frames a growing transcript produces.
+ *
+ * Read at the start of each frame, which is one frame behind the pin: the
+ * content commits, the next frame's layout delivers the resize, and the write
+ * lands before that frame paints. So the view can only ever be behind by what
+ * arrived since the last delivery — never more, and never cumulatively. That is
+ * what `worstLag` against `worstFrameGrowth` states, and it is a property no
+ * fixed pixel budget can express: a transcript that stopped following instead
+ * falls behind by the whole of `grewBy`.
+ */
+function measureTailLag(page: Page, frames: number): Promise<{
+ worstLag: number;
+ worstFrameGrowth: number;
+ grewBy: number;
+ viewportHeight: number;
+}> {
+ return page.evaluate(([selector, frameCount]) => new Promise<{
+ worstLag: number;
+ worstFrameGrowth: number;
+ grewBy: number;
+ viewportHeight: number;
+ }>((resolve) => {
+ const root = document.querySelector(selector as string);
+ if (!root) throw new Error('the chat scroll container is missing');
+ const startedAt = root.scrollHeight;
+ let previousScrollHeight = startedAt;
+ let worstLag = 0;
+ let worstFrameGrowth = 0;
+ let left = frameCount as number;
+ const tick = (): void => {
+ const settledTail = previousScrollHeight - root.clientHeight;
+ worstLag = Math.max(worstLag, Math.abs(root.scrollTop - settledTail));
+ worstFrameGrowth = Math.max(worstFrameGrowth, root.scrollHeight - previousScrollHeight);
+ previousScrollHeight = root.scrollHeight;
+ // Stops on the content, not on a frame count: when the answer starts
+ // arriving is the backend's business, and a fixed window can expire
+ // before it does.
+ const enough = root.scrollHeight - startedAt > root.clientHeight;
+ if (enough || --left <= 0) {
+ resolve({
+ worstLag: Math.round(worstLag),
+ worstFrameGrowth: Math.round(worstFrameGrowth),
+ grewBy: Math.round(root.scrollHeight - startedAt),
+ viewportHeight: root.clientHeight,
+ });
+ } else requestAnimationFrame(tick);
+ };
+ requestAnimationFrame(tick);
+ }), [SCROLLER, frames] as const);
+}
+
+async function sendPrompt(page: Page, text: string): Promise<void> {
+ const composer = page.locator(COMPOSER_INPUT);
+ await composer.fill(text);
+ await composer.press('Enter');
+}
+
+/** Answered turns, so a second send can be waited for without a stale match. */
+function answeredTurns(page: Page) {
+ return page.getByRole('button', { name: REGENERATE });
+}
+
+async function scrollTranscriptTo(page: Page, top: number): Promise<void> {
+ await page.evaluate(([selector, position]) => {
+ const root = document.querySelector(selector as string);
+ if (!root) throw new Error('the chat scroll container is missing');
+ root.scrollTop = position as number;
+ }, [SCROLLER, top] as const);
+ await waitForPaintedFrames(page);
+}
+
+async function waitForPaintedFrames(page: Page, count = 3): Promise<void> {
+ await page.evaluate((frames) => new Promise<void>((resolve) => {
+ const tick = (left: number) => {
+ if (left <= 0) {
+ resolve();
+ return;
+ }
+ requestAnimationFrame(() => tick(left - 1));
+ };
+ tick(frames);
+ }), count);
+}
+
+test('a streaming answer keeps the viewport at the tail', async ({ window: page }) => {
+ // A full fake-backend turn, streamed nine characters at a time.
+ test.slow();
+ await page.setViewportSize({ width: 900, height: 700 });
+ await sendPrompt(page, LONG_PROMPT);
+
+ // Measured through the stream, not only at the end: the failure this guards
+ // against is the tail slipping away *while* content arrives, which a single
+ // reading afterwards cannot tell apart from a view dragged back at the last
+ // delta.
+ const lag = await measureTailLag(page, 1_200);
+ await expect(answeredTurns(page)).toHaveCount(1, { timeout: 30_000 });
+
+ // The samples have to have covered more than a viewport of real growth, or
+ // every reading above is a stationary transcript and proves nothing.
+ expect(lag.grewBy).toBeGreaterThan(lag.viewportHeight);
+ expect(lag.worstLag).toBeLessThanOrEqual(lag.worstFrameGrowth + 8);
+ const settled = await scrollMetrics(page);
+ expect(settled.distance, JSON.stringify(settled)).toBeLessThanOrEqual(4);
+ expect(await scrollButtonOffered(page)).toBe(false);
+});
+
+test('content that arrives after the reader scrolls up does not pull them back', async ({
+ window: page,
+}) => {
+ test.slow();
+ await page.setViewportSize({ width: 900, height: 700 });
+ await sendPrompt(page, LONG_PROMPT);
+ await expect(answeredTurns(page)).toHaveCount(1, { timeout: 30_000 });
+
+ const transcript = page.locator('.maka-chat-message-list');
+ await transcript.hover();
+ await page.mouse.wheel(0, -500);
+ await waitForPaintedFrames(page);
+ const before = await distanceToTail(page);
+ expect(before).toBeGreaterThan(100);
+ expect(await scrollButtonOffered(page)).toBe(true);
+
+ const anchorTurnId = await page.evaluate(() => {
+ const turn = document.querySelector<HTMLElement>('[data-turn-id]');
+ const turnId = turn?.dataset.turnId;
+ if (!turnId) throw new Error('the transcript has no mounted turn');
+ return turnId;
+ });
+ const anchorTop = await turnTop(page, anchorTurnId);
+
+ await sendPrompt(page, LONG_PROMPT);
+ await expect(answeredTurns(page)).toHaveCount(2, { timeout: 30_000 });
+ await waitForPaintedFrames(page);
+
+ // The turn the reader was on is still where it was. Everything that arrived,
+ // arrived below them.
+ expect(Math.abs((await turnTop(page, anchorTurnId)) - anchorTop)).toBeLessThanOrEqual(4);
+ expect(await distanceToTail(page)).toBeGreaterThan(before);
+});
+
+test('a gesture a nested scroller consumed does not release the tail', async ({
Review Comment:
Same cause and same fix as your note on line 187 — `94f1c031c`, with the CI verdict on that head still pending.
To answer the distinction you asked for directly: neither. The authority missed a settle path on every platform, and Linux/xvfb was only where the timing exposed it. The measurement is not fragile — a pinned transcript settling at the tail is the whole claim of this PR, and ≤4px is the right budget for it.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] refactor(ui): make the transcript the only writer of its scroll position [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han merged PR #4105:
URL: https://github.com/apache/maka/pull/4105
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] refactor(ui): make the transcript the only writer of its scroll position [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han commented on code in PR #4105:
URL: https://github.com/apache/maka/pull/4105#discussion_r3885914323
##########
patches/@astryxdesign+core+0.5.0.patch:
##########
@@ -68,13 +74,18 @@ index ff9b9fa..346e01c 100644
+ return;
+ }
+ conversationKeyRef.current = conversationKey;
-+ scroll.lock();
++ // maka: a new conversation is a new initial fill, not content growth in
++ // the current one. `lock` alone re-enters through the spring, so the
++ // incoming transcript flies to the bottom instead of arriving there.
++ scroll.scrollToBottom({
Review Comment:
Superseded on the current head: the mechanism this thread is about no longer exists. `initialFillPendingRef`, `resetInitialFill`, and the first-fill jump were removed by the scroll-authority refactor — a new conversation now arrives at its tail because `pinToTail()` is set on session change and the first fill is growth like any other, so there is no separate first-fill path left to re-arm. `packages/ui/src/__tests__/chat-conversation-swap-scroll.test.tsx` went with it.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] refactor(ui): make the transcript the only writer of its scroll position [maka]
Posted by "M4n5ter (via GitHub)" <gi...@apache.org>.
M4n5ter commented on code in PR #4105:
URL: https://github.com/apache/maka/pull/4105#discussion_r3885828058
##########
packages/ui/src/use-turn-virtualizer.ts:
##########
@@ -250,10 +251,11 @@ export function useTurnVirtualizer(input: {
) || changed;
}
}
- if (changed) {
- if (anchor) pendingAnchor.current = anchor;
- setGeometryRevision((revision) => revision + 1);
- }
+ if (changed) setGeometryRevision((revision) => revision + 1);
+ // Before the window work, and synchronously: this is the observer
+ // callback, so the pin still writes `scrollTop` in the same frame the
+ // content grew and the reader never sees the tail slip.
+ contentResizeRef.current?.();
Review Comment:
[P2] This is now the transcript's only content-growth signal, but it observes only virtual Turn wrappers. Supported nodes rendered outside those wrappers can still grow the scroll height: an ordinary Follow Up is projected as a `next_turn` transient below the Turns, and the no-tail live fallback and orphan conversation item use the same outside path in `chat-view.tsx`. Observing the fixed-height scroll root does not report overflow-content growth. In a production-hook DOM probe, the root and Turn were observed, the inserted `data-transient-message-id` node was not, `onContentResize` fired zero times, and a pinned viewport stayed at 600 after its expected bottom became 800. Current unit tests call `notifyContentResize()` manually, and the E2E streaming case grows inside an observed Turn, so neither guard covers this path. Please observe the actual transcript content box (or every outside growth source) and add a pinned `next_turn` optimistic-message regression.
---
_Posted by an automated review agent operated by @M4n5ter. This is not an
independent human review and does not satisfy the committer review required by
CONTRIBUTING.md. A human is accountable for this comment — please push back if
anything here is wrong._
<details><summary>简体中文</summary>
_本条评论由 @M4n5ter 运行的自动化审查程序发出。它不构成 CONTRIBUTING.md
所要求的独立人类审查,也不能替代人类审查。有人类对本条评论负责,如有错误请直接指出。_
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] perf(ui): stop the transcript from moving after a session switch [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han commented on code in PR #4105:
URL: https://github.com/apache/maka/pull/4105#discussion_r3883276637
##########
patches/@astryxdesign+core+0.5.0.patch:
##########
@@ -68,13 +74,18 @@ index ff9b9fa..346e01c 100644
+ return;
+ }
+ conversationKeyRef.current = conversationKey;
-+ scroll.lock();
++ // maka: a new conversation is a new initial fill, not content growth in
++ // the current one. `lock` alone re-enters through the spring, so the
++ // incoming transcript flies to the bottom instead of arriving there.
++ scroll.scrollToBottom({
Review Comment:
Already addressed at this commit: the `conversationKey` effect calls `scroll.resetInitialFill()` right after the instant jump (patch line 77), re-arming `initialFillPendingRef` so the delayed transcript takes the synchronous first-fill jump instead of the spring. Covered by `packages/ui/src/__tests__/chat-conversation-swap-scroll.test.tsx`, which drives the empty-on-key-change → delayed-content sequence and fails without the reset (scrollTop 0 instead of 4000).
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] refactor(ui): make the transcript the only writer of its scroll position [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han commented on code in PR #4105:
URL: https://github.com/apache/maka/pull/4105#discussion_r3885932051
##########
packages/ui/src/use-turn-virtualizer.ts:
##########
@@ -250,10 +251,11 @@ export function useTurnVirtualizer(input: {
) || changed;
}
}
- if (changed) {
- if (anchor) pendingAnchor.current = anchor;
- setGeometryRevision((revision) => revision + 1);
- }
+ if (changed) setGeometryRevision((revision) => revision + 1);
+ // Before the window work, and synchronously: this is the observer
+ // callback, so the pin still writes `scrollTop` in the same frame the
+ // content grew and the reader never sees the tail slip.
+ contentResizeRef.current?.();
Review Comment:
Verified, and fixed in `0d16c68e4`. Your probe was right about the mechanism and right about which paths escape it: the optimistic message, the no-tail live fallback, and the orphaned conversation item all render outside the turn wrappers, and a ResizeObserver on the scroller reports the viewport, never the overflow.
E2E confirms it on the ordinary Follow Up path — the new spec reads `closest('[data-virtual-turn-id]')` on the optimistic message and gets `null` — and the tail assertion fails with the fix reverted.
I did not fix it by enumerating the outside sources, though. An observer that knows which nodes matter can be wrong about it, and this is the second time it has been: the list would have to grow every time the transcript renders something new. The authority now observes the scroller's own children, which is what `scrollHeight` is made of whatever the transcript chose to put in them. Only the direct children — anything deeper grows one of them on the way to growing `scrollHeight`, or is out of flow and does not grow it — with a `MutationObserver` on `childList` keeping the set current.
That also merges the two signals the authority had: content growing and the viewport shrinking are the same event to it, so `notifyContentResize` and the `onContentResize` wiring through `useTurnVirtualizer` are gone. The virtualizer observes turns for the one thing turns are — its height index.
Regression: `a message the transcript renders outside a turn is followed too` in `apps/desktop/e2e/transcript-scroll.spec.ts`. It asserts the placement first, so if that message ever moves inside a wrapper the test says so rather than quietly stopping to cover this.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
##########
apps/desktop/e2e/transcript-scroll.spec.ts:
##########
@@ -0,0 +1,408 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { expect, test, COMPOSER_INPUT } from './fixtures';
+import type { Page } from '@playwright/test';
+
+/**
+ * Where the transcript is looking, in a real Chromium with a real scroller.
+ *
+ * Two rounds of this work shipped green and wrong, both times because the
+ * instrument could not see the property being claimed: a CLS measurement is
+ * blind to scroll position, and a linkedom harness decides the effect ordering
+ * its own assertions then confirm. Nothing below reads a ref or a flag — each
+ * test states where an element or the viewport ended up, and the app has to put
+ * it there.
+ *
+ * Positions are asserted against an element or against the scroller's own end,
+ * never as a pixel delta: a delta is satisfiable by two wrongs (the content
+ * grew by as much as the view moved), which is the bug class that produced the
+ * `scrollHeight`-difference compensation this replaces.
+ */
+
+const SCROLLER = '[data-chat-scroll-container="true"]';
+const REGENERATE = /^重新生成回答/;
+/** Astryx's dock affordance, relabelled by `ChatSurfaceLayout`. */
+const SCROLL_TO_BOTTOM = /^滚动主对话到底部$/;
+
+/** Sixty lines: more than one viewport once the fake backend echoes it back. */
+const LONG_PROMPT = Array.from(
+ { length: 60 },
+ (_, index) => `第 ${index} 行:这一段用来把转录推过滚动视口的高度。`,
+).join('\n');
+
+function distanceToTail(page: Page): Promise<number> {
+ return scrollMetrics(page).then((metrics) => metrics.distance);
+}
+
+/**
+ * The distance plus the three numbers it came from. A failure that reports only
+ * the distance cannot say whether the transcript grew past the reader or the
+ * viewport shrank under them, and those have different causes.
+ */
+function scrollMetrics(page: Page): Promise<{
+ distance: number;
+ scrollTop: number;
+ scrollHeight: number;
+ clientHeight: number;
+}> {
+ return page.evaluate((selector) => {
+ const root = document.querySelector(selector);
+ if (!root) throw new Error('the chat scroll container is missing');
+ return {
+ distance: Math.round(root.scrollHeight - root.scrollTop - root.clientHeight),
+ scrollTop: Math.round(root.scrollTop),
+ scrollHeight: root.scrollHeight,
+ clientHeight: root.clientHeight,
+ };
+ }, SCROLLER);
+}
+
+/**
+ * Whether the dock affordance is actually offered. It is always in the DOM —
+ * Astryx toggles opacity and pointer-events — so presence proves nothing and
+ * `toBeVisible` passes on the transparent one.
+ */
+function scrollButtonOffered(page: Page): Promise<boolean> {
+ return page.evaluate((name) => {
+ const button = [...document.querySelectorAll('button')].find(
+ (candidate) => candidate.getAttribute('aria-label') === name
+ || candidate.textContent?.trim() === name,
+ );
+ if (!button) throw new Error(`the "${name}" affordance is missing`);
+ const style = getComputedStyle(button);
+ return style.pointerEvents !== 'none' && Number(style.opacity) > 0.5;
+ }, '滚动主对话到底部');
+}
+
+function turnTop(page: Page, turnId: string): Promise<number> {
+ return page.evaluate((id) => {
+ const turn = document.querySelector(`[data-turn-id="${CSS.escape(id)}"]`);
+ if (!turn) throw new Error(`turn ${id} is not mounted`);
+ return Math.round(turn.getBoundingClientRect().top);
+ }, turnId);
+}
+
+/**
+ * Sample the tail through the frames a growing transcript produces.
+ *
+ * Read at the start of each frame, which is one frame behind the pin: the
+ * content commits, the next frame's layout delivers the resize, and the write
+ * lands before that frame paints. So the view can only ever be behind by what
+ * arrived since the last delivery — never more, and never cumulatively. That is
+ * what `worstLag` against `worstFrameGrowth` states, and it is a property no
+ * fixed pixel budget can express: a transcript that stopped following instead
+ * falls behind by the whole of `grewBy`.
+ */
+function measureTailLag(page: Page, frames: number): Promise<{
+ worstLag: number;
+ worstFrameGrowth: number;
+ grewBy: number;
+ viewportHeight: number;
+}> {
+ return page.evaluate(([selector, frameCount]) => new Promise<{
+ worstLag: number;
+ worstFrameGrowth: number;
+ grewBy: number;
+ viewportHeight: number;
+ }>((resolve) => {
+ const root = document.querySelector(selector as string);
+ if (!root) throw new Error('the chat scroll container is missing');
+ const startedAt = root.scrollHeight;
+ let previousScrollHeight = startedAt;
+ let worstLag = 0;
+ let worstFrameGrowth = 0;
+ let left = frameCount as number;
+ const tick = (): void => {
+ const settledTail = previousScrollHeight - root.clientHeight;
+ worstLag = Math.max(worstLag, Math.abs(root.scrollTop - settledTail));
+ worstFrameGrowth = Math.max(worstFrameGrowth, root.scrollHeight - previousScrollHeight);
+ previousScrollHeight = root.scrollHeight;
+ // Stops on the content, not on a frame count: when the answer starts
+ // arriving is the backend's business, and a fixed window can expire
+ // before it does.
+ const enough = root.scrollHeight - startedAt > root.clientHeight;
+ if (enough || --left <= 0) {
+ resolve({
+ worstLag: Math.round(worstLag),
+ worstFrameGrowth: Math.round(worstFrameGrowth),
+ grewBy: Math.round(root.scrollHeight - startedAt),
+ viewportHeight: root.clientHeight,
+ });
+ } else requestAnimationFrame(tick);
+ };
+ requestAnimationFrame(tick);
+ }), [SCROLLER, frames] as const);
+}
+
+async function sendPrompt(page: Page, text: string): Promise<void> {
+ const composer = page.locator(COMPOSER_INPUT);
+ await composer.fill(text);
+ await composer.press('Enter');
+}
+
+/** Answered turns, so a second send can be waited for without a stale match. */
+function answeredTurns(page: Page) {
+ return page.getByRole('button', { name: REGENERATE });
+}
+
+async function scrollTranscriptTo(page: Page, top: number): Promise<void> {
+ await page.evaluate(([selector, position]) => {
+ const root = document.querySelector(selector as string);
+ if (!root) throw new Error('the chat scroll container is missing');
+ root.scrollTop = position as number;
+ }, [SCROLLER, top] as const);
+ await waitForPaintedFrames(page);
+}
+
+async function waitForPaintedFrames(page: Page, count = 3): Promise<void> {
+ await page.evaluate((frames) => new Promise<void>((resolve) => {
+ const tick = (left: number) => {
+ if (left <= 0) {
+ resolve();
+ return;
+ }
+ requestAnimationFrame(() => tick(left - 1));
+ };
+ tick(frames);
+ }), count);
+}
+
+test('a streaming answer keeps the viewport at the tail', async ({ window: page }) => {
Review Comment:
CI is green on `94f1c031c` (`test` passed, 23m54s), so the 302 is closed: it was the defect described above, not the environment.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] refactor(ui): make the transcript the only writer of its scroll position [maka]
Posted by "jackwener (via GitHub)" <gi...@apache.org>.
jackwener commented on code in PR #4105:
URL: https://github.com/apache/maka/pull/4105#discussion_r3885800755
##########
packages/ui/src/use-chat-scroll.ts:
##########
@@ -17,183 +17,107 @@
* under the License.
*/
+/**
+ * The transcript's scroll commands, and the seam that hands the scroller to the
+ * authority that owns it (`transcript-scroll-authority.ts`).
+ *
+ * A command is one-shot: jump to a turn the reader picked, or ask for earlier
+ * history. Each releases the pin first, and the authority writes nothing while
+ * the pin is released — so a command cannot be fighting a policy, which is the
+ * shape every previous round of this code had. Nothing here compensates for
+ * content that lands above the reader; `overflow-anchor: auto` does that
+ * continuously, and for free.
+ */
+
import { useEffect, useRef, useState, type RefObject } from 'react';
import type { StoredMessage } from '@maka/core/session';
-import { createArrivalBottomPin, type ArrivalBottomPin } from './arrival-bottom-pin.js';
-import { captureChatScrollAnchor, restoreChatScrollAnchor } from './chat-scroll-anchor.js';
+import { useTranscriptScrollAuthority } from './transcript-scroll-authority.js';
export function useChatScroll(input: {
scrollRef: RefObject<HTMLElement | null>;
sessionId?: string;
- hasTurns: boolean;
messages: readonly StoredMessage[];
target?: { turnId: string; nonce: number };
behavior: ScrollBehavior;
hasOlderHistory?: boolean;
- historyLoadPending?: boolean;
onLoadEarlierHistory?(): Promise<void> | void;
- latestNavigationNonce?: number;
}) {
const [highlightedTurnId, setHighlightedTurnId] = useState<string | null>(null);
- const arrivalPin = useRef<ArrivalBottomPin | null>(null);
+ const authority = useTranscriptScrollAuthority();
const loadEarlierRef = useRef(input.onLoadEarlierHistory);
loadEarlierRef.current = input.onLoadEarlierHistory;
- const sessionIdRef = useRef(input.sessionId);
- sessionIdRef.current = input.sessionId;
- const historyLoadPendingRef = useRef(input.historyLoadPending);
- historyLoadPendingRef.current = input.historyLoadPending;
const canLoadEarlier = input.onLoadEarlierHistory !== undefined;
- const earlierLoadRequest = useRef<object | null>(null);
- const requestEarlierRef = useRef<() => void>(() => {});
+ const handledTarget = useRef<string | null>(null);
+ // A passive effect, not a layout one: the scroller is Astryx's layout root,
+ // an ancestor, and React attaches a parent's ref after its children's layout
+ // effects have already run. The growth signal is a ResizeObserver delivery,
+ // which lands after passive effects, so this is still installed in time.
+ useEffect(() => authority.attach(input.scrollRef.current), [authority, input.scrollRef]);
+
+ // A new conversation arrives at its tail. Nothing special positions it: the
+ // pin is set here and the first fill is growth like any other, so it takes
+ // the one path instead of a first-fill path of its own.
useEffect(() => {
- earlierLoadRequest.current = null;
+ authority.pinToTail();
}, [input.sessionId]);
useEffect(() => {
const root = input.scrollRef.current;
if (!root || !input.hasOlderHistory || !canLoadEarlier) return;
- let previousScrollTop = root.scrollTop;
+ // Asking twice is the loader's problem, not this one's: it refuses a
+ // request while one is in flight, and asking for history the reader
+ // already has is idempotent anyway.
const requestEarlier = (): void => {
- if (historyLoadPendingRef.current || earlierLoadRequest.current) return;
- const scrollHeight = root.scrollHeight;
- const anchor = captureChatScrollAnchor(root);
- const sessionId = sessionIdRef.current;
- const request = {};
- earlierLoadRequest.current = request;
- arrivalPin.current?.release();
- void Promise.resolve(loadEarlierRef.current?.()).catch(() => undefined).finally(() => {
- window.requestAnimationFrame(() => {
- if (
- earlierLoadRequest.current === request &&
- sessionIdRef.current === sessionId &&
- input.scrollRef.current === root &&
- root.isConnected &&
- !restoreChatScrollAnchor(root, anchor)
- ) {
- root.scrollTop += root.scrollHeight - scrollHeight;
- }
- if (earlierLoadRequest.current === request) earlierLoadRequest.current = null;
- });
- });
+ // Nothing here touches the pin. Asking for history is not a decision
+ // about where the viewport belongs: if the reader scrolled up to ask,
+ // the authority already saw that scroll and released; if the transcript
+ // is merely short enough that its tail is also near its start, they are
+ // still following it and must keep following it.
Review Comment:
**[P1]** Class ①. This comment is false at `scrollTop === 0`, which is the wheel-to-load path this file documents a few lines below.
`scroll` does not fire when the scroller is already at zero and the reader wheels up, so the authority never releases. After `9eb824f9a` this function also does not release. The load then grows the transcript under a still-pinned reader, and `notifyContentResize` writes `scrollTop = scrollHeight` — the history they asked for is scrolled off the bottom.
That is the normal gesture on a short latest window that still has older history: the whole transcript is on screen (pinned, distance to tail ≈ 0), and they wheel up for more context.
Minimal fix: treat a wheel-up at zero as the reader (`releasePin()` there). Do not keep the pin on a history request the reader actually made. The “short transcript must keep following” case is the other half of this P1: that request should not fire at all while they are at the tail.
<details><summary>简体中文</summary>
**[P1]** 可达性 ①。这条注释在 `scrollTop === 0` 时不成立,而那正是下面写的滚轮加载路径。
已经在 0 时向上滚,不会有 `scroll`,authority 不会松钉。`9eb824f9a` 之后这个函数自己也不松。历史加载后钉住仍在,`notifyContentResize` 把 `scrollTop` 写成 `scrollHeight`,读者刚要的更早内容被甩到下面去了。
短的最新窗口后面还有更早历史时,这是正常手势:整段都在屏上(钉住,离尾部 ≈ 0),向上滚是在要更早上下文。
最小修法:把 0 处向上滚轮当成读者(在那里 `releasePin()`)。读者真正要历史时不要保持钉住。「短转录必须继续跟随」是这个 P1 的另一半:人还在尾部时,那个请求根本不该发出。
</details>
##########
packages/ui/src/use-chat-scroll.ts:
##########
@@ -17,183 +17,107 @@
* under the License.
*/
+/**
+ * The transcript's scroll commands, and the seam that hands the scroller to the
+ * authority that owns it (`transcript-scroll-authority.ts`).
+ *
+ * A command is one-shot: jump to a turn the reader picked, or ask for earlier
+ * history. Each releases the pin first, and the authority writes nothing while
+ * the pin is released — so a command cannot be fighting a policy, which is the
+ * shape every previous round of this code had. Nothing here compensates for
+ * content that lands above the reader; `overflow-anchor: auto` does that
+ * continuously, and for free.
+ */
+
import { useEffect, useRef, useState, type RefObject } from 'react';
import type { StoredMessage } from '@maka/core/session';
-import { createArrivalBottomPin, type ArrivalBottomPin } from './arrival-bottom-pin.js';
-import { captureChatScrollAnchor, restoreChatScrollAnchor } from './chat-scroll-anchor.js';
+import { useTranscriptScrollAuthority } from './transcript-scroll-authority.js';
export function useChatScroll(input: {
scrollRef: RefObject<HTMLElement | null>;
sessionId?: string;
- hasTurns: boolean;
messages: readonly StoredMessage[];
target?: { turnId: string; nonce: number };
behavior: ScrollBehavior;
hasOlderHistory?: boolean;
- historyLoadPending?: boolean;
onLoadEarlierHistory?(): Promise<void> | void;
- latestNavigationNonce?: number;
}) {
const [highlightedTurnId, setHighlightedTurnId] = useState<string | null>(null);
- const arrivalPin = useRef<ArrivalBottomPin | null>(null);
+ const authority = useTranscriptScrollAuthority();
const loadEarlierRef = useRef(input.onLoadEarlierHistory);
loadEarlierRef.current = input.onLoadEarlierHistory;
- const sessionIdRef = useRef(input.sessionId);
- sessionIdRef.current = input.sessionId;
- const historyLoadPendingRef = useRef(input.historyLoadPending);
- historyLoadPendingRef.current = input.historyLoadPending;
const canLoadEarlier = input.onLoadEarlierHistory !== undefined;
- const earlierLoadRequest = useRef<object | null>(null);
- const requestEarlierRef = useRef<() => void>(() => {});
+ const handledTarget = useRef<string | null>(null);
+ // A passive effect, not a layout one: the scroller is Astryx's layout root,
+ // an ancestor, and React attaches a parent's ref after its children's layout
+ // effects have already run. The growth signal is a ResizeObserver delivery,
+ // which lands after passive effects, so this is still installed in time.
+ useEffect(() => authority.attach(input.scrollRef.current), [authority, input.scrollRef]);
+
+ // A new conversation arrives at its tail. Nothing special positions it: the
+ // pin is set here and the first fill is growth like any other, so it takes
+ // the one path instead of a first-fill path of its own.
useEffect(() => {
- earlierLoadRequest.current = null;
+ authority.pinToTail();
}, [input.sessionId]);
useEffect(() => {
const root = input.scrollRef.current;
if (!root || !input.hasOlderHistory || !canLoadEarlier) return;
- let previousScrollTop = root.scrollTop;
+ // Asking twice is the loader's problem, not this one's: it refuses a
+ // request while one is in flight, and asking for history the reader
+ // already has is idempotent anyway.
const requestEarlier = (): void => {
- if (historyLoadPendingRef.current || earlierLoadRequest.current) return;
- const scrollHeight = root.scrollHeight;
- const anchor = captureChatScrollAnchor(root);
- const sessionId = sessionIdRef.current;
- const request = {};
- earlierLoadRequest.current = request;
- arrivalPin.current?.release();
- void Promise.resolve(loadEarlierRef.current?.()).catch(() => undefined).finally(() => {
- window.requestAnimationFrame(() => {
- if (
- earlierLoadRequest.current === request &&
- sessionIdRef.current === sessionId &&
- input.scrollRef.current === root &&
- root.isConnected &&
- !restoreChatScrollAnchor(root, anchor)
- ) {
- root.scrollTop += root.scrollHeight - scrollHeight;
- }
- if (earlierLoadRequest.current === request) earlierLoadRequest.current = null;
- });
- });
+ // Nothing here touches the pin. Asking for history is not a decision
+ // about where the viewport belongs: if the reader scrolled up to ask,
+ // the authority already saw that scroll and released; if the transcript
+ // is merely short enough that its tail is also near its start, they are
+ // still following it and must keep following it.
+ //
+ // The browser anchors the reader against everything that lands above
+ // them, with one exception: it declines while the scroller sits at zero,
+ // which is exactly where a wheel asks for history. One pixel is the whole
+ // fix — measured in Chromium, an insert of 501px above the reader moves
+ // `scrollTop` by 501 at an offset of 1 and by 0 at an offset of 0.
+ if (root.scrollTop < 1) root.scrollTop = 1;
+ void Promise.resolve(loadEarlierRef.current?.()).catch(() => undefined);
};
+ // Position, not direction: a shrinking transcript also lowers `scrollTop`.
const nearStart = (): boolean =>
root.scrollTop <= Math.max(640, root.clientHeight * 2);
const onScroll = (): void => {
- const nextScrollTop = root.scrollTop;
- if (nextScrollTop < previousScrollTop && nearStart()) requestEarlier();
- previousScrollTop = nextScrollTop;
+ if (nearStart()) requestEarlier();
Review Comment:
**[P1]** Class ①. This calls `requestEarlier()` on every `scroll` while `scrollTop` is inside `max(640, 2×clientHeight)`, including the authority's own pin writes.
On `main` this handler required an upward move (`nextScrollTop < previousScrollTop`). Dropping that check means a pinned write to the tail is enough. For any latest window shorter than about three viewports, the tail is still inside that band, so session switch → `pinToTail()` / `notifyContentResize()` → `scroll` → load earlier history the reader did not ask for. That is the same speculative insert the first commit of this PR existed to remove.
The Playwright specs do not assert that a session switch leaves history unloaded, and they assign `scrollTop` (which un-pins) before testing a top-of-scroller load.
Minimal fix: do not ask for earlier history while the reader is at the tail, or while the authority is the one writing. An upward-only check, or skipping the request when distance-to-tail is within the pin threshold, both close it.
<details><summary>简体中文</summary>
**[P1]** 可达性 ①。只要 `scrollTop` 落在 `max(640, 2×clientHeight)` 里,任何 `scroll` 都会 `requestEarlier()`,包括 authority 自己钉住时写下的那次。
`main` 上这里要求向上移动(`nextScrollTop < previousScrollTop`)。去掉这个判断后,钉住写到尾部就够触发。最新窗口短于大约三屏时,尾部仍在加载带里,于是会话切换 → `pinToTail()` / `notifyContentResize()` → `scroll` → 加载读者没要的更早历史。这就是这个 PR 第一笔提交要删掉的那种预取。
Playwright 没有断言会话切换不会加载更早历史,并且测试顶端加载前会先赋值 `scrollTop`(这会松钉)。
最小修法:读者在尾部时、或当前是 authority 在写时,不要请求更早历史。只在向上滚动时请求,或距离尾部仍在钉住阈值内就跳过,都可以关掉这条路径。
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] refactor(ui): make the transcript the only writer of its scroll position [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han commented on code in PR #4105:
URL: https://github.com/apache/maka/pull/4105#discussion_r3886288327
##########
apps/desktop/e2e/transcript-scroll.spec.ts:
##########
@@ -0,0 +1,408 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { expect, test, COMPOSER_INPUT } from './fixtures';
+import type { Page } from '@playwright/test';
+
+/**
+ * Where the transcript is looking, in a real Chromium with a real scroller.
+ *
+ * Two rounds of this work shipped green and wrong, both times because the
+ * instrument could not see the property being claimed: a CLS measurement is
+ * blind to scroll position, and a linkedom harness decides the effect ordering
+ * its own assertions then confirm. Nothing below reads a ref or a flag — each
+ * test states where an element or the viewport ended up, and the app has to put
+ * it there.
+ *
+ * Positions are asserted against an element or against the scroller's own end,
+ * never as a pixel delta: a delta is satisfiable by two wrongs (the content
+ * grew by as much as the view moved), which is the bug class that produced the
+ * `scrollHeight`-difference compensation this replaces.
+ */
+
+const SCROLLER = '[data-chat-scroll-container="true"]';
+const REGENERATE = /^重新生成回答/;
+/** Astryx's dock affordance, relabelled by `ChatSurfaceLayout`. */
+const SCROLL_TO_BOTTOM = /^滚动主对话到底部$/;
+
+/** Sixty lines: more than one viewport once the fake backend echoes it back. */
+const LONG_PROMPT = Array.from(
+ { length: 60 },
+ (_, index) => `第 ${index} 行:这一段用来把转录推过滚动视口的高度。`,
+).join('\n');
+
+function distanceToTail(page: Page): Promise<number> {
+ return scrollMetrics(page).then((metrics) => metrics.distance);
+}
+
+/**
+ * The distance plus the three numbers it came from. A failure that reports only
+ * the distance cannot say whether the transcript grew past the reader or the
+ * viewport shrank under them, and those have different causes.
+ */
+function scrollMetrics(page: Page): Promise<{
+ distance: number;
+ scrollTop: number;
+ scrollHeight: number;
+ clientHeight: number;
+}> {
+ return page.evaluate((selector) => {
+ const root = document.querySelector(selector);
+ if (!root) throw new Error('the chat scroll container is missing');
+ return {
+ distance: Math.round(root.scrollHeight - root.scrollTop - root.clientHeight),
+ scrollTop: Math.round(root.scrollTop),
+ scrollHeight: root.scrollHeight,
+ clientHeight: root.clientHeight,
+ };
+ }, SCROLLER);
+}
+
+/**
+ * Whether the dock affordance is actually offered. It is always in the DOM —
+ * Astryx toggles opacity and pointer-events — so presence proves nothing and
+ * `toBeVisible` passes on the transparent one.
+ */
+function scrollButtonOffered(page: Page): Promise<boolean> {
+ return page.evaluate((name) => {
+ const button = [...document.querySelectorAll('button')].find(
+ (candidate) => candidate.getAttribute('aria-label') === name
+ || candidate.textContent?.trim() === name,
+ );
+ if (!button) throw new Error(`the "${name}" affordance is missing`);
+ const style = getComputedStyle(button);
+ return style.pointerEvents !== 'none' && Number(style.opacity) > 0.5;
+ }, '滚动主对话到底部');
+}
+
+function turnTop(page: Page, turnId: string): Promise<number> {
+ return page.evaluate((id) => {
+ const turn = document.querySelector(`[data-turn-id="${CSS.escape(id)}"]`);
+ if (!turn) throw new Error(`turn ${id} is not mounted`);
+ return Math.round(turn.getBoundingClientRect().top);
+ }, turnId);
+}
+
+/**
+ * Sample the tail through the frames a growing transcript produces.
+ *
+ * Read at the start of each frame, which is one frame behind the pin: the
+ * content commits, the next frame's layout delivers the resize, and the write
+ * lands before that frame paints. So the view can only ever be behind by what
+ * arrived since the last delivery — never more, and never cumulatively. That is
+ * what `worstLag` against `worstFrameGrowth` states, and it is a property no
+ * fixed pixel budget can express: a transcript that stopped following instead
+ * falls behind by the whole of `grewBy`.
+ */
+function measureTailLag(page: Page, frames: number): Promise<{
+ worstLag: number;
+ worstFrameGrowth: number;
+ grewBy: number;
+ viewportHeight: number;
+}> {
+ return page.evaluate(([selector, frameCount]) => new Promise<{
+ worstLag: number;
+ worstFrameGrowth: number;
+ grewBy: number;
+ viewportHeight: number;
+ }>((resolve) => {
+ const root = document.querySelector(selector as string);
+ if (!root) throw new Error('the chat scroll container is missing');
+ const startedAt = root.scrollHeight;
+ let previousScrollHeight = startedAt;
+ let worstLag = 0;
+ let worstFrameGrowth = 0;
+ let left = frameCount as number;
+ const tick = (): void => {
+ const settledTail = previousScrollHeight - root.clientHeight;
+ worstLag = Math.max(worstLag, Math.abs(root.scrollTop - settledTail));
+ worstFrameGrowth = Math.max(worstFrameGrowth, root.scrollHeight - previousScrollHeight);
+ previousScrollHeight = root.scrollHeight;
+ // Stops on the content, not on a frame count: when the answer starts
+ // arriving is the backend's business, and a fixed window can expire
+ // before it does.
+ const enough = root.scrollHeight - startedAt > root.clientHeight;
+ if (enough || --left <= 0) {
+ resolve({
+ worstLag: Math.round(worstLag),
+ worstFrameGrowth: Math.round(worstFrameGrowth),
+ grewBy: Math.round(root.scrollHeight - startedAt),
+ viewportHeight: root.clientHeight,
+ });
+ } else requestAnimationFrame(tick);
+ };
+ requestAnimationFrame(tick);
+ }), [SCROLLER, frames] as const);
+}
+
+async function sendPrompt(page: Page, text: string): Promise<void> {
+ const composer = page.locator(COMPOSER_INPUT);
+ await composer.fill(text);
+ await composer.press('Enter');
+}
+
+/** Answered turns, so a second send can be waited for without a stale match. */
+function answeredTurns(page: Page) {
+ return page.getByRole('button', { name: REGENERATE });
+}
+
+async function scrollTranscriptTo(page: Page, top: number): Promise<void> {
+ await page.evaluate(([selector, position]) => {
+ const root = document.querySelector(selector as string);
+ if (!root) throw new Error('the chat scroll container is missing');
+ root.scrollTop = position as number;
+ }, [SCROLLER, top] as const);
+ await waitForPaintedFrames(page);
+}
+
+async function waitForPaintedFrames(page: Page, count = 3): Promise<void> {
+ await page.evaluate((frames) => new Promise<void>((resolve) => {
+ const tick = (left: number) => {
+ if (left <= 0) {
+ resolve();
+ return;
+ }
+ requestAnimationFrame(() => tick(left - 1));
+ };
+ tick(frames);
+ }), count);
+}
+
+test('a streaming answer keeps the viewport at the tail', async ({ window: page }) => {
Review Comment:
Closed, with the CI evidence you asked for.
`test` is green on the current head `944f23420` (24m22s, run 33244891995), which is this branch rebased onto `main`. It was also green on `94f1c031c`, the commit that fixed the cause. The 302 was the product, not the environment: `pinned` was being re-derived from position on every scroll event the authority did not recognise as its own echo, so native anchoring, growth that outran a tail write, and resize clamping each silently re-answered "is the reader following the tail" — and each could answer no. Linux/xvfb only made the timing land where my machine did not.
To your either/or directly: the authority missed a settle path on every platform, and the measurement is not fragile. A pinned transcript settling at the tail is this PR's whole claim, and ≤4px is the right budget for it.
One thing your two comments proved beyond their own subject: the spec did its job. It failed on a real defect in an environment I do not have, and the metrics it prints (`distance`, `scrollTop`, `scrollHeight`, `clientHeight`) are what made the third diagnosis the right one — the first two attempts on this branch were wrong and are in the history claiming only what they actually did.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
##########
apps/desktop/e2e/transcript-scroll.spec.ts:
##########
@@ -0,0 +1,408 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { expect, test, COMPOSER_INPUT } from './fixtures';
+import type { Page } from '@playwright/test';
+
+/**
+ * Where the transcript is looking, in a real Chromium with a real scroller.
+ *
+ * Two rounds of this work shipped green and wrong, both times because the
+ * instrument could not see the property being claimed: a CLS measurement is
+ * blind to scroll position, and a linkedom harness decides the effect ordering
+ * its own assertions then confirm. Nothing below reads a ref or a flag — each
+ * test states where an element or the viewport ended up, and the app has to put
+ * it there.
+ *
+ * Positions are asserted against an element or against the scroller's own end,
+ * never as a pixel delta: a delta is satisfiable by two wrongs (the content
+ * grew by as much as the view moved), which is the bug class that produced the
+ * `scrollHeight`-difference compensation this replaces.
+ */
+
+const SCROLLER = '[data-chat-scroll-container="true"]';
+const REGENERATE = /^重新生成回答/;
+/** Astryx's dock affordance, relabelled by `ChatSurfaceLayout`. */
+const SCROLL_TO_BOTTOM = /^滚动主对话到底部$/;
+
+/** Sixty lines: more than one viewport once the fake backend echoes it back. */
+const LONG_PROMPT = Array.from(
+ { length: 60 },
+ (_, index) => `第 ${index} 行:这一段用来把转录推过滚动视口的高度。`,
+).join('\n');
+
+function distanceToTail(page: Page): Promise<number> {
+ return scrollMetrics(page).then((metrics) => metrics.distance);
+}
+
+/**
+ * The distance plus the three numbers it came from. A failure that reports only
+ * the distance cannot say whether the transcript grew past the reader or the
+ * viewport shrank under them, and those have different causes.
+ */
+function scrollMetrics(page: Page): Promise<{
+ distance: number;
+ scrollTop: number;
+ scrollHeight: number;
+ clientHeight: number;
+}> {
+ return page.evaluate((selector) => {
+ const root = document.querySelector(selector);
+ if (!root) throw new Error('the chat scroll container is missing');
+ return {
+ distance: Math.round(root.scrollHeight - root.scrollTop - root.clientHeight),
+ scrollTop: Math.round(root.scrollTop),
+ scrollHeight: root.scrollHeight,
+ clientHeight: root.clientHeight,
+ };
+ }, SCROLLER);
+}
+
+/**
+ * Whether the dock affordance is actually offered. It is always in the DOM —
+ * Astryx toggles opacity and pointer-events — so presence proves nothing and
+ * `toBeVisible` passes on the transparent one.
+ */
+function scrollButtonOffered(page: Page): Promise<boolean> {
+ return page.evaluate((name) => {
+ const button = [...document.querySelectorAll('button')].find(
+ (candidate) => candidate.getAttribute('aria-label') === name
+ || candidate.textContent?.trim() === name,
+ );
+ if (!button) throw new Error(`the "${name}" affordance is missing`);
+ const style = getComputedStyle(button);
+ return style.pointerEvents !== 'none' && Number(style.opacity) > 0.5;
+ }, '滚动主对话到底部');
+}
+
+function turnTop(page: Page, turnId: string): Promise<number> {
+ return page.evaluate((id) => {
+ const turn = document.querySelector(`[data-turn-id="${CSS.escape(id)}"]`);
+ if (!turn) throw new Error(`turn ${id} is not mounted`);
+ return Math.round(turn.getBoundingClientRect().top);
+ }, turnId);
+}
+
+/**
+ * Sample the tail through the frames a growing transcript produces.
+ *
+ * Read at the start of each frame, which is one frame behind the pin: the
+ * content commits, the next frame's layout delivers the resize, and the write
+ * lands before that frame paints. So the view can only ever be behind by what
+ * arrived since the last delivery — never more, and never cumulatively. That is
+ * what `worstLag` against `worstFrameGrowth` states, and it is a property no
+ * fixed pixel budget can express: a transcript that stopped following instead
+ * falls behind by the whole of `grewBy`.
+ */
+function measureTailLag(page: Page, frames: number): Promise<{
+ worstLag: number;
+ worstFrameGrowth: number;
+ grewBy: number;
+ viewportHeight: number;
+}> {
+ return page.evaluate(([selector, frameCount]) => new Promise<{
+ worstLag: number;
+ worstFrameGrowth: number;
+ grewBy: number;
+ viewportHeight: number;
+ }>((resolve) => {
+ const root = document.querySelector(selector as string);
+ if (!root) throw new Error('the chat scroll container is missing');
+ const startedAt = root.scrollHeight;
+ let previousScrollHeight = startedAt;
+ let worstLag = 0;
+ let worstFrameGrowth = 0;
+ let left = frameCount as number;
+ const tick = (): void => {
+ const settledTail = previousScrollHeight - root.clientHeight;
+ worstLag = Math.max(worstLag, Math.abs(root.scrollTop - settledTail));
+ worstFrameGrowth = Math.max(worstFrameGrowth, root.scrollHeight - previousScrollHeight);
+ previousScrollHeight = root.scrollHeight;
+ // Stops on the content, not on a frame count: when the answer starts
+ // arriving is the backend's business, and a fixed window can expire
+ // before it does.
+ const enough = root.scrollHeight - startedAt > root.clientHeight;
+ if (enough || --left <= 0) {
+ resolve({
+ worstLag: Math.round(worstLag),
+ worstFrameGrowth: Math.round(worstFrameGrowth),
+ grewBy: Math.round(root.scrollHeight - startedAt),
+ viewportHeight: root.clientHeight,
+ });
+ } else requestAnimationFrame(tick);
+ };
+ requestAnimationFrame(tick);
+ }), [SCROLLER, frames] as const);
+}
+
+async function sendPrompt(page: Page, text: string): Promise<void> {
+ const composer = page.locator(COMPOSER_INPUT);
+ await composer.fill(text);
+ await composer.press('Enter');
+}
+
+/** Answered turns, so a second send can be waited for without a stale match. */
+function answeredTurns(page: Page) {
+ return page.getByRole('button', { name: REGENERATE });
+}
+
+async function scrollTranscriptTo(page: Page, top: number): Promise<void> {
+ await page.evaluate(([selector, position]) => {
+ const root = document.querySelector(selector as string);
+ if (!root) throw new Error('the chat scroll container is missing');
+ root.scrollTop = position as number;
+ }, [SCROLLER, top] as const);
+ await waitForPaintedFrames(page);
+}
+
+async function waitForPaintedFrames(page: Page, count = 3): Promise<void> {
+ await page.evaluate((frames) => new Promise<void>((resolve) => {
+ const tick = (left: number) => {
+ if (left <= 0) {
+ resolve();
+ return;
+ }
+ requestAnimationFrame(() => tick(left - 1));
+ };
+ tick(frames);
+ }), count);
+}
+
+test('a streaming answer keeps the viewport at the tail', async ({ window: page }) => {
+ // A full fake-backend turn, streamed nine characters at a time.
+ test.slow();
+ await page.setViewportSize({ width: 900, height: 700 });
+ await sendPrompt(page, LONG_PROMPT);
+
+ // Measured through the stream, not only at the end: the failure this guards
+ // against is the tail slipping away *while* content arrives, which a single
+ // reading afterwards cannot tell apart from a view dragged back at the last
+ // delta.
+ const lag = await measureTailLag(page, 1_200);
+ await expect(answeredTurns(page)).toHaveCount(1, { timeout: 30_000 });
+
+ // The samples have to have covered more than a viewport of real growth, or
+ // every reading above is a stationary transcript and proves nothing.
+ expect(lag.grewBy).toBeGreaterThan(lag.viewportHeight);
+ expect(lag.worstLag).toBeLessThanOrEqual(lag.worstFrameGrowth + 8);
+ const settled = await scrollMetrics(page);
+ expect(settled.distance, JSON.stringify(settled)).toBeLessThanOrEqual(4);
+ expect(await scrollButtonOffered(page)).toBe(false);
+});
+
+test('content that arrives after the reader scrolls up does not pull them back', async ({
+ window: page,
+}) => {
+ test.slow();
+ await page.setViewportSize({ width: 900, height: 700 });
+ await sendPrompt(page, LONG_PROMPT);
+ await expect(answeredTurns(page)).toHaveCount(1, { timeout: 30_000 });
+
+ const transcript = page.locator('.maka-chat-message-list');
+ await transcript.hover();
+ await page.mouse.wheel(0, -500);
+ await waitForPaintedFrames(page);
+ const before = await distanceToTail(page);
+ expect(before).toBeGreaterThan(100);
+ expect(await scrollButtonOffered(page)).toBe(true);
+
+ const anchorTurnId = await page.evaluate(() => {
+ const turn = document.querySelector<HTMLElement>('[data-turn-id]');
+ const turnId = turn?.dataset.turnId;
+ if (!turnId) throw new Error('the transcript has no mounted turn');
+ return turnId;
+ });
+ const anchorTop = await turnTop(page, anchorTurnId);
+
+ await sendPrompt(page, LONG_PROMPT);
+ await expect(answeredTurns(page)).toHaveCount(2, { timeout: 30_000 });
+ await waitForPaintedFrames(page);
+
+ // The turn the reader was on is still where it was. Everything that arrived,
+ // arrived below them.
+ expect(Math.abs((await turnTop(page, anchorTurnId)) - anchorTop)).toBeLessThanOrEqual(4);
+ expect(await distanceToTail(page)).toBeGreaterThan(before);
+});
+
+test('a gesture a nested scroller consumed does not release the tail', async ({
Review Comment:
Closed, with the CI evidence you asked for.
`test` is green on the current head `944f23420` (24m22s, run 33244891995), which is this branch rebased onto `main`. It was also green on `94f1c031c`, the commit that fixed the cause. The 302 was the product, not the environment: `pinned` was being re-derived from position on every scroll event the authority did not recognise as its own echo, so native anchoring, growth that outran a tail write, and resize clamping each silently re-answered "is the reader following the tail" — and each could answer no. Linux/xvfb only made the timing land where my machine did not.
To your either/or directly: the authority missed a settle path on every platform, and the measurement is not fragile. A pinned transcript settling at the tail is this PR's whole claim, and ≤4px is the right budget for it.
One thing your two comments proved beyond their own subject: the spec did its job. It failed on a real defect in an environment I do not have, and the metrics it prints (`distance`, `scrollTop`, `scrollHeight`, `clientHeight`) are what made the third diagnosis the right one — the first two attempts on this branch were wrong and are in the history claiming only what they actually did.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org