fix(desktop): clear "New messages" indicator at bottom and per conversation (#2205)

[cat0825]

Fixes#2205

Summary

The desktop chat surface's "New messages" scroll-to-bottom indicator now clears when the user scrolls back to the bottom, and its state resets fully when switching conversations. Repro screenshots are attached in the issue.

Root cause

Two independent problems in @astryxdesign/core@0.2.0:

1. Not dismissed at the bottom.useChatNewMessages only clears hasNewMessages via dismiss() (the indicator button). useChatStreamScroll re-locks auto-follow on scrollend but never reports "at bottom" back to the indicator, so the label stays visible after the user scrolls back down.
2. Leaks across conversations.ChatLayout is not remounted per conversation — lastMessageRef (pointing at the previous conversation's last message node) and the unlocked isLocked=false survive the switch, so the first message of a new conversation re-flags hasNewMessages.

Changes

• apps/desktop/src/renderer/app-shell.tsx, quote-companion-panel.tsx: key ChatSurfaceLayout per conversation so the scroll/new-message state is remounted and reset on conversation switch.
• patches/@astryxdesign+core+0.2.0.patch: adds the two @astryxdesign/core hunks for this fix (ChatLayout.js, useChatNewMessages.js). The file already carried the fix(ui): localize the field required/optional marker app-wide #2184FieldLabel localization hunks, so the patch now contains both sets; verified with git apply --check that the merged patch applies cleanly to a pristine @astryxdesign/core@0.2.0.
• patches/README.md: rationale and removal conditions for the new hunks, following the repo convention.
• apps/desktop/e2e/new-messages-indicator.spec.ts: Playwright regression coverage for both reported symptoms.

Verification

• New e2e spec passes; with the fix reverted the two new cases fail exactly as described in the issue (negative control).
• Regression: quote-companion, send-message, scroll-geometry, sidebar-navigation e2e suites (17 tests), packages/ui unit tests (316/316), desktop typecheck and biome all clean.

[Astro-Han]

Thanks for this — the fix is well-scoped and I verified it end to end in a browser harness against the patched dist: the badge appears on unlocked growth, clears on real scrollend at bottom, resets on conversation switch, and the composer element survives the switch (no remount — conversationKey is a plain prop, the patch resets in place, which also keeps it compatible with #2239's arrival-pin approach; that PR is still open, so nothing merged is at risk). The patch applies cleanly to a pristine @astryxdesign/core@0.2.0, and the patches/README.md conflict with the merged #2225 is purely mechanical (both appended sections; keep both).

P1 — the e2e selectors no longer match main: #2202 (merged after your branch point) localized the two Astryx keys. The spec's header comment claims "Maka's Astryx overrides do not cover these keys", but packages/ui/src/astryx-i18n.tsx:97-98 covers both — '@astryx.chatLayout.newMessages' → '跳到最新消息' and '@astryx.chatLayoutScrollButton.scrollToBottom' → '滚动到底部' — and the e2e window fixture runs zh. After a squash-merge both tests would fail at the first English-name assertion (scroll-geometry-adjacent, on the e2e shard). Please switch the selectors to '跳到最新消息' / '滚动到底部' (the rest of the spec already uses zh names) and fix the comment — otherwise the regression guard ships red and its negative control is void.

P3 (optional):

• growTranscriptOverflow only guarantees scrollHeight > clientHeight, but the 'Scroll to bottom' visibility depends on Astryx's 100px buttonThreshold — a compact 3-message transcript could fail at that assertion with a misleading error. Assert sh - ch > 100 with diagnostics instead.
• The "no remount → draft survives" constraint has no e2e guard: composer-skill-invocation / skill-draft-lifecycle only cover revision drafts, not cross-conversation persistence. Typing a draft in test 2 before switching (and asserting it survives) would pin the property that makes the prop-not-key design load-bearing. Cheap.
• The quote-companion side (conversationKey for the fork session) has no e2e coverage — acceptable per the representative-journey guidance, just noting the leak would go unnoticed there.

The fix itself is sound — happy to approve once the selectors are updated.

[cat0825]

Fixed the e2e failures — root cause was not the product fix, it was the spec itself (it had never run on Linux before this merge):

1. Locale: Maka's Astryx copy overrides (astryx-copy.ts) map the scroll buttons to zh ('跳到最新消息'/'滚动到底部') under the default zh fixture locale; the spec hardcoded the en names. Now matches both locales — the assertion is about the affordance appearing/clearing, not Astryx's copy.
2. Strict-mode violation: the prompt anchor rail (upstream perf(ui): seed transcript geometry before progressive fill #2237) renders a preview of sent text, so loose getByText(/Fake backend received: .../) matched two nodes. Now uses { exact: true } on the echo.
3. Threshold: growTranscriptOverflow required only sh > ch; Astryx flags scrolled-up only past buttonThreshold (100px), so a compact font could keep the button unrendered. Grows until sh - ch > 150px.

Verified locally: 2/2 pass. Pushed as 243b921.

[Astro-Han]

Thanks — the new commit resolves everything from the prior pass: the locale-agnostic matchers cover both the zh override (astryx-copy.ts:114-115) and Astryx's en defaults, the { exact: true } echoes correctly dodge the prompt-rail preview (which stays mounted via HoverCard), and the 150px growth bound (buttonThreshold 100 + headroom) is bounded and cannot hang. I also verified the conversationKey passthrough at both call sites (app-shell and quote-companion — the companion's companionSession?.id ?? sourceSession?.id is the right key, since each quote source forks a new session), the patch is exactly the 3 #2205-related hunks and applies cleanly to pristine 0.2.0, and both tests fail on the pre-fix product code.

One new finding, tracked as a follow-up rather than a blocker:

P2 (deferred, not blocking) — transient strict-mode double match during streaming. Between the fake backend's chunk 6 and 7 (~45ms window), the rail's reply preview normalizes+trims to exactly the query string while the transcript's first <p> also matches — two elements at a poll instant → Playwright's strict-mode violation is non-retriable, so the whole spec hard-fails for that run (probability is phase-dependent; author's local runs and CI are green). One-line fix: scope the echo locators to the message list (.maka-chat-message-list — the rail is a sibling), or assert after settle. If we see strict mode violation in future CI runs of this spec, that's the cause.

P3s (open, optional): the quote-companion conversationKey path still has no e2e coverage (the spec covers only the sessions surface), and the fixed 250ms wait could be a poll.

Merging now — thanks for the fast turnaround!

[cat0825]

Fixes#2205

Summary

The desktop chat surface's "New messages" scroll-to-bottom indicator now clears when the user scrolls back to the bottom, and its state resets fully when switching conversations. Repro screenshots are attached in the issue.

Root cause

Two independent problems in @astryxdesign/core@0.2.0:

1. Not dismissed at the bottom.useChatNewMessages only clears hasNewMessages via dismiss() (the indicator button). useChatStreamScroll re-locks auto-follow on scrollend but never reports "at bottom" back to the indicator, so the label stays visible after the user scrolls back down.
2. Leaks across conversations.ChatLayout is not remounted per conversation — lastMessageRef (pointing at the previous conversation's last message node) and the unlocked isLocked=false survive the switch, so the first message of a new conversation re-flags hasNewMessages.

Changes

• apps/desktop/src/renderer/app-shell.tsx, quote-companion-panel.tsx: key ChatSurfaceLayout per conversation so the scroll/new-message state is remounted and reset on conversation switch.
• patches/@astryxdesign+core+0.2.0.patch: adds the two @astryxdesign/core hunks for this fix (ChatLayout.js, useChatNewMessages.js). The file already carried the fix(ui): localize the field required/optional marker app-wide #2184FieldLabel localization hunks, so the patch now contains both sets; verified with git apply --check that the merged patch applies cleanly to a pristine @astryxdesign/core@0.2.0.
• patches/README.md: rationale and removal conditions for the new hunks, following the repo convention.
• apps/desktop/e2e/new-messages-indicator.spec.ts: Playwright regression coverage for both reported symptoms.

Verification

• New e2e spec passes; with the fix reverted the two new cases fail exactly as described in the issue (negative control).
• Regression: quote-companion, send-message, scroll-geometry, sidebar-navigation e2e suites (17 tests), packages/ui unit tests (316/316), desktop typecheck and biome all clean.

[Astro-Han]

Thanks for this — the fix is well-scoped and I verified it end to end in a browser harness against the patched dist: the badge appears on unlocked growth, clears on real scrollend at bottom, resets on conversation switch, and the composer element survives the switch (no remount — conversationKey is a plain prop, the patch resets in place, which also keeps it compatible with #2239's arrival-pin approach; that PR is still open, so nothing merged is at risk). The patch applies cleanly to a pristine @astryxdesign/core@0.2.0, and the patches/README.md conflict with the merged #2225 is purely mechanical (both appended sections; keep both).

P1 — the e2e selectors no longer match main: #2202 (merged after your branch point) localized the two Astryx keys. The spec's header comment claims "Maka's Astryx overrides do not cover these keys", but packages/ui/src/astryx-i18n.tsx:97-98 covers both — '@astryx.chatLayout.newMessages' → '跳到最新消息' and '@astryx.chatLayoutScrollButton.scrollToBottom' → '滚动到底部' — and the e2e window fixture runs zh. After a squash-merge both tests would fail at the first English-name assertion (scroll-geometry-adjacent, on the e2e shard). Please switch the selectors to '跳到最新消息' / '滚动到底部' (the rest of the spec already uses zh names) and fix the comment — otherwise the regression guard ships red and its negative control is void.

P3 (optional):

• growTranscriptOverflow only guarantees scrollHeight > clientHeight, but the 'Scroll to bottom' visibility depends on Astryx's 100px buttonThreshold — a compact 3-message transcript could fail at that assertion with a misleading error. Assert sh - ch > 100 with diagnostics instead.
• The "no remount → draft survives" constraint has no e2e guard: composer-skill-invocation / skill-draft-lifecycle only cover revision drafts, not cross-conversation persistence. Typing a draft in test 2 before switching (and asserting it survives) would pin the property that makes the prop-not-key design load-bearing. Cheap.
• The quote-companion side (conversationKey for the fork session) has no e2e coverage — acceptable per the representative-journey guidance, just noting the leak would go unnoticed there.

The fix itself is sound — happy to approve once the selectors are updated.

[cat0825]

Fixed the e2e failures — root cause was not the product fix, it was the spec itself (it had never run on Linux before this merge):

1. Locale: Maka's Astryx copy overrides (astryx-copy.ts) map the scroll buttons to zh ('跳到最新消息'/'滚动到底部') under the default zh fixture locale; the spec hardcoded the en names. Now matches both locales — the assertion is about the affordance appearing/clearing, not Astryx's copy.
2. Strict-mode violation: the prompt anchor rail (upstream perf(ui): seed transcript geometry before progressive fill #2237) renders a preview of sent text, so loose getByText(/Fake backend received: .../) matched two nodes. Now uses { exact: true } on the echo.
3. Threshold: growTranscriptOverflow required only sh > ch; Astryx flags scrolled-up only past buttonThreshold (100px), so a compact font could keep the button unrendered. Grows until sh - ch > 150px.

Verified locally: 2/2 pass. Pushed as 243b921.

[Astro-Han]

Thanks — the new commit resolves everything from the prior pass: the locale-agnostic matchers cover both the zh override (astryx-copy.ts:114-115) and Astryx's en defaults, the { exact: true } echoes correctly dodge the prompt-rail preview (which stays mounted via HoverCard), and the 150px growth bound (buttonThreshold 100 + headroom) is bounded and cannot hang. I also verified the conversationKey passthrough at both call sites (app-shell and quote-companion — the companion's companionSession?.id ?? sourceSession?.id is the right key, since each quote source forks a new session), the patch is exactly the 3 #2205-related hunks and applies cleanly to pristine 0.2.0, and both tests fail on the pre-fix product code.

One new finding, tracked as a follow-up rather than a blocker:

P2 (deferred, not blocking) — transient strict-mode double match during streaming. Between the fake backend's chunk 6 and 7 (~45ms window), the rail's reply preview normalizes+trims to exactly the query string while the transcript's first <p> also matches — two elements at a poll instant → Playwright's strict-mode violation is non-retriable, so the whole spec hard-fails for that run (probability is phase-dependent; author's local runs and CI are green). One-line fix: scope the echo locators to the message list (.maka-chat-message-list — the rail is a sibling), or assert after settle. If we see strict mode violation in future CI runs of this spec, that's the cause.

P3s (open, optional): the quote-companion conversationKey path still has no e2e coverage (the spec covers only the sessions surface), and the fixed 250ms wait could be a poll.

Merging now — thanks for the fast turnaround!

[cat0825]

Fixes#2205

Summary

The desktop chat surface's "New messages" scroll-to-bottom indicator now clears when the user scrolls back to the bottom, and its state resets fully when switching conversations. Repro screenshots are attached in the issue.

Root cause

Two independent problems in @astryxdesign/core@0.2.0:

1. Not dismissed at the bottom.useChatNewMessages only clears hasNewMessages via dismiss() (the indicator button). useChatStreamScroll re-locks auto-follow on scrollend but never reports "at bottom" back to the indicator, so the label stays visible after the user scrolls back down.
2. Leaks across conversations.ChatLayout is not remounted per conversation — lastMessageRef (pointing at the previous conversation's last message node) and the unlocked isLocked=false survive the switch, so the first message of a new conversation re-flags hasNewMessages.

Changes

• apps/desktop/src/renderer/app-shell.tsx, quote-companion-panel.tsx: key ChatSurfaceLayout per conversation so the scroll/new-message state is remounted and reset on conversation switch.
• patches/@astryxdesign+core+0.2.0.patch: adds the two @astryxdesign/core hunks for this fix (ChatLayout.js, useChatNewMessages.js). The file already carried the fix(ui): localize the field required/optional marker app-wide #2184FieldLabel localization hunks, so the patch now contains both sets; verified with git apply --check that the merged patch applies cleanly to a pristine @astryxdesign/core@0.2.0.
• patches/README.md: rationale and removal conditions for the new hunks, following the repo convention.
• apps/desktop/e2e/new-messages-indicator.spec.ts: Playwright regression coverage for both reported symptoms.

Verification

• New e2e spec passes; with the fix reverted the two new cases fail exactly as described in the issue (negative control).
• Regression: quote-companion, send-message, scroll-geometry, sidebar-navigation e2e suites (17 tests), packages/ui unit tests (316/316), desktop typecheck and biome all clean.

[Astro-Han]

Thanks for this — the fix is well-scoped and I verified it end to end in a browser harness against the patched dist: the badge appears on unlocked growth, clears on real scrollend at bottom, resets on conversation switch, and the composer element survives the switch (no remount — conversationKey is a plain prop, the patch resets in place, which also keeps it compatible with #2239's arrival-pin approach; that PR is still open, so nothing merged is at risk). The patch applies cleanly to a pristine @astryxdesign/core@0.2.0, and the patches/README.md conflict with the merged #2225 is purely mechanical (both appended sections; keep both).

P1 — the e2e selectors no longer match main: #2202 (merged after your branch point) localized the two Astryx keys. The spec's header comment claims "Maka's Astryx overrides do not cover these keys", but packages/ui/src/astryx-i18n.tsx:97-98 covers both — '@astryx.chatLayout.newMessages' → '跳到最新消息' and '@astryx.chatLayoutScrollButton.scrollToBottom' → '滚动到底部' — and the e2e window fixture runs zh. After a squash-merge both tests would fail at the first English-name assertion (scroll-geometry-adjacent, on the e2e shard). Please switch the selectors to '跳到最新消息' / '滚动到底部' (the rest of the spec already uses zh names) and fix the comment — otherwise the regression guard ships red and its negative control is void.

P3 (optional):

• growTranscriptOverflow only guarantees scrollHeight > clientHeight, but the 'Scroll to bottom' visibility depends on Astryx's 100px buttonThreshold — a compact 3-message transcript could fail at that assertion with a misleading error. Assert sh - ch > 100 with diagnostics instead.
• The "no remount → draft survives" constraint has no e2e guard: composer-skill-invocation / skill-draft-lifecycle only cover revision drafts, not cross-conversation persistence. Typing a draft in test 2 before switching (and asserting it survives) would pin the property that makes the prop-not-key design load-bearing. Cheap.
• The quote-companion side (conversationKey for the fork session) has no e2e coverage — acceptable per the representative-journey guidance, just noting the leak would go unnoticed there.

The fix itself is sound — happy to approve once the selectors are updated.

[cat0825]

Fixed the e2e failures — root cause was not the product fix, it was the spec itself (it had never run on Linux before this merge):

1. Locale: Maka's Astryx copy overrides (astryx-copy.ts) map the scroll buttons to zh ('跳到最新消息'/'滚动到底部') under the default zh fixture locale; the spec hardcoded the en names. Now matches both locales — the assertion is about the affordance appearing/clearing, not Astryx's copy.
2. Strict-mode violation: the prompt anchor rail (upstream perf(ui): seed transcript geometry before progressive fill #2237) renders a preview of sent text, so loose getByText(/Fake backend received: .../) matched two nodes. Now uses { exact: true } on the echo.
3. Threshold: growTranscriptOverflow required only sh > ch; Astryx flags scrolled-up only past buttonThreshold (100px), so a compact font could keep the button unrendered. Grows until sh - ch > 150px.

Verified locally: 2/2 pass. Pushed as 243b921.

[Astro-Han]

Thanks — the new commit resolves everything from the prior pass: the locale-agnostic matchers cover both the zh override (astryx-copy.ts:114-115) and Astryx's en defaults, the { exact: true } echoes correctly dodge the prompt-rail preview (which stays mounted via HoverCard), and the 150px growth bound (buttonThreshold 100 + headroom) is bounded and cannot hang. I also verified the conversationKey passthrough at both call sites (app-shell and quote-companion — the companion's companionSession?.id ?? sourceSession?.id is the right key, since each quote source forks a new session), the patch is exactly the 3 #2205-related hunks and applies cleanly to pristine 0.2.0, and both tests fail on the pre-fix product code.

One new finding, tracked as a follow-up rather than a blocker:

P2 (deferred, not blocking) — transient strict-mode double match during streaming. Between the fake backend's chunk 6 and 7 (~45ms window), the rail's reply preview normalizes+trims to exactly the query string while the transcript's first <p> also matches — two elements at a poll instant → Playwright's strict-mode violation is non-retriable, so the whole spec hard-fails for that run (probability is phase-dependent; author's local runs and CI are green). One-line fix: scope the echo locators to the message list (.maka-chat-message-list — the rail is a sibling), or assert after settle. If we see strict mode violation in future CI runs of this spec, that's the cause.

P3s (open, optional): the quote-companion conversationKey path still has no e2e coverage (the spec covers only the sessions surface), and the fixed 250ms wait could be a poll.

Merging now — thanks for the fast turnaround!

[cat0825]

Fixes#2205

Summary

The desktop chat surface's "New messages" scroll-to-bottom indicator now clears when the user scrolls back to the bottom, and its state resets fully when switching conversations. Repro screenshots are attached in the issue.

Root cause

Two independent problems in @astryxdesign/core@0.2.0:

1. Not dismissed at the bottom.useChatNewMessages only clears hasNewMessages via dismiss() (the indicator button). useChatStreamScroll re-locks auto-follow on scrollend but never reports "at bottom" back to the indicator, so the label stays visible after the user scrolls back down.
2. Leaks across conversations.ChatLayout is not remounted per conversation — lastMessageRef (pointing at the previous conversation's last message node) and the unlocked isLocked=false survive the switch, so the first message of a new conversation re-flags hasNewMessages.

Changes

• apps/desktop/src/renderer/app-shell.tsx, quote-companion-panel.tsx: key ChatSurfaceLayout per conversation so the scroll/new-message state is remounted and reset on conversation switch.
• patches/@astryxdesign+core+0.2.0.patch: adds the two @astryxdesign/core hunks for this fix (ChatLayout.js, useChatNewMessages.js). The file already carried the fix(ui): localize the field required/optional marker app-wide #2184FieldLabel localization hunks, so the patch now contains both sets; verified with git apply --check that the merged patch applies cleanly to a pristine @astryxdesign/core@0.2.0.
• patches/README.md: rationale and removal conditions for the new hunks, following the repo convention.
• apps/desktop/e2e/new-messages-indicator.spec.ts: Playwright regression coverage for both reported symptoms.

Verification

• New e2e spec passes; with the fix reverted the two new cases fail exactly as described in the issue (negative control).
• Regression: quote-companion, send-message, scroll-geometry, sidebar-navigation e2e suites (17 tests), packages/ui unit tests (316/316), desktop typecheck and biome all clean.

[Astro-Han]

Thanks for this — the fix is well-scoped and I verified it end to end in a browser harness against the patched dist: the badge appears on unlocked growth, clears on real scrollend at bottom, resets on conversation switch, and the composer element survives the switch (no remount — conversationKey is a plain prop, the patch resets in place, which also keeps it compatible with #2239's arrival-pin approach; that PR is still open, so nothing merged is at risk). The patch applies cleanly to a pristine @astryxdesign/core@0.2.0, and the patches/README.md conflict with the merged #2225 is purely mechanical (both appended sections; keep both).

P1 — the e2e selectors no longer match main: #2202 (merged after your branch point) localized the two Astryx keys. The spec's header comment claims "Maka's Astryx overrides do not cover these keys", but packages/ui/src/astryx-i18n.tsx:97-98 covers both — '@astryx.chatLayout.newMessages' → '跳到最新消息' and '@astryx.chatLayoutScrollButton.scrollToBottom' → '滚动到底部' — and the e2e window fixture runs zh. After a squash-merge both tests would fail at the first English-name assertion (scroll-geometry-adjacent, on the e2e shard). Please switch the selectors to '跳到最新消息' / '滚动到底部' (the rest of the spec already uses zh names) and fix the comment — otherwise the regression guard ships red and its negative control is void.

P3 (optional):

• growTranscriptOverflow only guarantees scrollHeight > clientHeight, but the 'Scroll to bottom' visibility depends on Astryx's 100px buttonThreshold — a compact 3-message transcript could fail at that assertion with a misleading error. Assert sh - ch > 100 with diagnostics instead.
• The "no remount → draft survives" constraint has no e2e guard: composer-skill-invocation / skill-draft-lifecycle only cover revision drafts, not cross-conversation persistence. Typing a draft in test 2 before switching (and asserting it survives) would pin the property that makes the prop-not-key design load-bearing. Cheap.
• The quote-companion side (conversationKey for the fork session) has no e2e coverage — acceptable per the representative-journey guidance, just noting the leak would go unnoticed there.

The fix itself is sound — happy to approve once the selectors are updated.

[cat0825]

Fixed the e2e failures — root cause was not the product fix, it was the spec itself (it had never run on Linux before this merge):

1. Locale: Maka's Astryx copy overrides (astryx-copy.ts) map the scroll buttons to zh ('跳到最新消息'/'滚动到底部') under the default zh fixture locale; the spec hardcoded the en names. Now matches both locales — the assertion is about the affordance appearing/clearing, not Astryx's copy.
2. Strict-mode violation: the prompt anchor rail (upstream perf(ui): seed transcript geometry before progressive fill #2237) renders a preview of sent text, so loose getByText(/Fake backend received: .../) matched two nodes. Now uses { exact: true } on the echo.
3. Threshold: growTranscriptOverflow required only sh > ch; Astryx flags scrolled-up only past buttonThreshold (100px), so a compact font could keep the button unrendered. Grows until sh - ch > 150px.

Verified locally: 2/2 pass. Pushed as 243b921.

[Astro-Han]

Thanks — the new commit resolves everything from the prior pass: the locale-agnostic matchers cover both the zh override (astryx-copy.ts:114-115) and Astryx's en defaults, the { exact: true } echoes correctly dodge the prompt-rail preview (which stays mounted via HoverCard), and the 150px growth bound (buttonThreshold 100 + headroom) is bounded and cannot hang. I also verified the conversationKey passthrough at both call sites (app-shell and quote-companion — the companion's companionSession?.id ?? sourceSession?.id is the right key, since each quote source forks a new session), the patch is exactly the 3 #2205-related hunks and applies cleanly to pristine 0.2.0, and both tests fail on the pre-fix product code.

One new finding, tracked as a follow-up rather than a blocker:

P2 (deferred, not blocking) — transient strict-mode double match during streaming. Between the fake backend's chunk 6 and 7 (~45ms window), the rail's reply preview normalizes+trims to exactly the query string while the transcript's first <p> also matches — two elements at a poll instant → Playwright's strict-mode violation is non-retriable, so the whole spec hard-fails for that run (probability is phase-dependent; author's local runs and CI are green). One-line fix: scope the echo locators to the message list (.maka-chat-message-list — the rail is a sibling), or assert after settle. If we see strict mode violation in future CI runs of this spec, that's the cause.

P3s (open, optional): the quote-companion conversationKey path still has no e2e coverage (the spec covers only the sessions surface), and the fixed 250ms wait could be a poll.

Merging now — thanks for the fast turnaround!

[cat0825]

Fixes#2205

Summary

The desktop chat surface's "New messages" scroll-to-bottom indicator now clears when the user scrolls back to the bottom, and its state resets fully when switching conversations. Repro screenshots are attached in the issue.

Root cause

Two independent problems in @astryxdesign/core@0.2.0:

1. Not dismissed at the bottom.useChatNewMessages only clears hasNewMessages via dismiss() (the indicator button). useChatStreamScroll re-locks auto-follow on scrollend but never reports "at bottom" back to the indicator, so the label stays visible after the user scrolls back down.
2. Leaks across conversations.ChatLayout is not remounted per conversation — lastMessageRef (pointing at the previous conversation's last message node) and the unlocked isLocked=false survive the switch, so the first message of a new conversation re-flags hasNewMessages.

Changes

• apps/desktop/src/renderer/app-shell.tsx, quote-companion-panel.tsx: key ChatSurfaceLayout per conversation so the scroll/new-message state is remounted and reset on conversation switch.
• patches/@astryxdesign+core+0.2.0.patch: adds the two @astryxdesign/core hunks for this fix (ChatLayout.js, useChatNewMessages.js). The file already carried the fix(ui): localize the field required/optional marker app-wide #2184FieldLabel localization hunks, so the patch now contains both sets; verified with git apply --check that the merged patch applies cleanly to a pristine @astryxdesign/core@0.2.0.
• patches/README.md: rationale and removal conditions for the new hunks, following the repo convention.
• apps/desktop/e2e/new-messages-indicator.spec.ts: Playwright regression coverage for both reported symptoms.

Verification

• New e2e spec passes; with the fix reverted the two new cases fail exactly as described in the issue (negative control).
• Regression: quote-companion, send-message, scroll-geometry, sidebar-navigation e2e suites (17 tests), packages/ui unit tests (316/316), desktop typecheck and biome all clean.

[Astro-Han]

Thanks for this — the fix is well-scoped and I verified it end to end in a browser harness against the patched dist: the badge appears on unlocked growth, clears on real scrollend at bottom, resets on conversation switch, and the composer element survives the switch (no remount — conversationKey is a plain prop, the patch resets in place, which also keeps it compatible with #2239's arrival-pin approach; that PR is still open, so nothing merged is at risk). The patch applies cleanly to a pristine @astryxdesign/core@0.2.0, and the patches/README.md conflict with the merged #2225 is purely mechanical (both appended sections; keep both).

P1 — the e2e selectors no longer match main: #2202 (merged after your branch point) localized the two Astryx keys. The spec's header comment claims "Maka's Astryx overrides do not cover these keys", but packages/ui/src/astryx-i18n.tsx:97-98 covers both — '@astryx.chatLayout.newMessages' → '跳到最新消息' and '@astryx.chatLayoutScrollButton.scrollToBottom' → '滚动到底部' — and the e2e window fixture runs zh. After a squash-merge both tests would fail at the first English-name assertion (scroll-geometry-adjacent, on the e2e shard). Please switch the selectors to '跳到最新消息' / '滚动到底部' (the rest of the spec already uses zh names) and fix the comment — otherwise the regression guard ships red and its negative control is void.

P3 (optional):

• growTranscriptOverflow only guarantees scrollHeight > clientHeight, but the 'Scroll to bottom' visibility depends on Astryx's 100px buttonThreshold — a compact 3-message transcript could fail at that assertion with a misleading error. Assert sh - ch > 100 with diagnostics instead.
• The "no remount → draft survives" constraint has no e2e guard: composer-skill-invocation / skill-draft-lifecycle only cover revision drafts, not cross-conversation persistence. Typing a draft in test 2 before switching (and asserting it survives) would pin the property that makes the prop-not-key design load-bearing. Cheap.
• The quote-companion side (conversationKey for the fork session) has no e2e coverage — acceptable per the representative-journey guidance, just noting the leak would go unnoticed there.

The fix itself is sound — happy to approve once the selectors are updated.

[cat0825]

Fixed the e2e failures — root cause was not the product fix, it was the spec itself (it had never run on Linux before this merge):

1. Locale: Maka's Astryx copy overrides (astryx-copy.ts) map the scroll buttons to zh ('跳到最新消息'/'滚动到底部') under the default zh fixture locale; the spec hardcoded the en names. Now matches both locales — the assertion is about the affordance appearing/clearing, not Astryx's copy.
2. Strict-mode violation: the prompt anchor rail (upstream perf(ui): seed transcript geometry before progressive fill #2237) renders a preview of sent text, so loose getByText(/Fake backend received: .../) matched two nodes. Now uses { exact: true } on the echo.
3. Threshold: growTranscriptOverflow required only sh > ch; Astryx flags scrolled-up only past buttonThreshold (100px), so a compact font could keep the button unrendered. Grows until sh - ch > 150px.

Verified locally: 2/2 pass. Pushed as 243b921.

[Astro-Han]

Thanks — the new commit resolves everything from the prior pass: the locale-agnostic matchers cover both the zh override (astryx-copy.ts:114-115) and Astryx's en defaults, the { exact: true } echoes correctly dodge the prompt-rail preview (which stays mounted via HoverCard), and the 150px growth bound (buttonThreshold 100 + headroom) is bounded and cannot hang. I also verified the conversationKey passthrough at both call sites (app-shell and quote-companion — the companion's companionSession?.id ?? sourceSession?.id is the right key, since each quote source forks a new session), the patch is exactly the 3 #2205-related hunks and applies cleanly to pristine 0.2.0, and both tests fail on the pre-fix product code.

One new finding, tracked as a follow-up rather than a blocker:

P2 (deferred, not blocking) — transient strict-mode double match during streaming. Between the fake backend's chunk 6 and 7 (~45ms window), the rail's reply preview normalizes+trims to exactly the query string while the transcript's first <p> also matches — two elements at a poll instant → Playwright's strict-mode violation is non-retriable, so the whole spec hard-fails for that run (probability is phase-dependent; author's local runs and CI are green). One-line fix: scope the echo locators to the message list (.maka-chat-message-list — the rail is a sibling), or assert after settle. If we see strict mode violation in future CI runs of this spec, that's the cause.

P3s (open, optional): the quote-companion conversationKey path still has no e2e coverage (the spec covers only the sessions surface), and the fixed 250ms wait could be a poll.

Merging now — thanks for the fast turnaround!

[cat0825]

Fixes#2205

Summary

The desktop chat surface's "New messages" scroll-to-bottom indicator now clears when the user scrolls back to the bottom, and its state resets fully when switching conversations. Repro screenshots are attached in the issue.

Root cause

Two independent problems in @astryxdesign/core@0.2.0:

1. Not dismissed at the bottom.useChatNewMessages only clears hasNewMessages via dismiss() (the indicator button). useChatStreamScroll re-locks auto-follow on scrollend but never reports "at bottom" back to the indicator, so the label stays visible after the user scrolls back down.
2. Leaks across conversations.ChatLayout is not remounted per conversation — lastMessageRef (pointing at the previous conversation's last message node) and the unlocked isLocked=false survive the switch, so the first message of a new conversation re-flags hasNewMessages.

Changes

• apps/desktop/src/renderer/app-shell.tsx, quote-companion-panel.tsx: key ChatSurfaceLayout per conversation so the scroll/new-message state is remounted and reset on conversation switch.
• patches/@astryxdesign+core+0.2.0.patch: adds the two @astryxdesign/core hunks for this fix (ChatLayout.js, useChatNewMessages.js). The file already carried the fix(ui): localize the field required/optional marker app-wide #2184FieldLabel localization hunks, so the patch now contains both sets; verified with git apply --check that the merged patch applies cleanly to a pristine @astryxdesign/core@0.2.0.
• patches/README.md: rationale and removal conditions for the new hunks, following the repo convention.
• apps/desktop/e2e/new-messages-indicator.spec.ts: Playwright regression coverage for both reported symptoms.

Verification

• New e2e spec passes; with the fix reverted the two new cases fail exactly as described in the issue (negative control).
• Regression: quote-companion, send-message, scroll-geometry, sidebar-navigation e2e suites (17 tests), packages/ui unit tests (316/316), desktop typecheck and biome all clean.

[Astro-Han]

Thanks for this — the fix is well-scoped and I verified it end to end in a browser harness against the patched dist: the badge appears on unlocked growth, clears on real scrollend at bottom, resets on conversation switch, and the composer element survives the switch (no remount — conversationKey is a plain prop, the patch resets in place, which also keeps it compatible with #2239's arrival-pin approach; that PR is still open, so nothing merged is at risk). The patch applies cleanly to a pristine @astryxdesign/core@0.2.0, and the patches/README.md conflict with the merged #2225 is purely mechanical (both appended sections; keep both).

P1 — the e2e selectors no longer match main: #2202 (merged after your branch point) localized the two Astryx keys. The spec's header comment claims "Maka's Astryx overrides do not cover these keys", but packages/ui/src/astryx-i18n.tsx:97-98 covers both — '@astryx.chatLayout.newMessages' → '跳到最新消息' and '@astryx.chatLayoutScrollButton.scrollToBottom' → '滚动到底部' — and the e2e window fixture runs zh. After a squash-merge both tests would fail at the first English-name assertion (scroll-geometry-adjacent, on the e2e shard). Please switch the selectors to '跳到最新消息' / '滚动到底部' (the rest of the spec already uses zh names) and fix the comment — otherwise the regression guard ships red and its negative control is void.

P3 (optional):

• growTranscriptOverflow only guarantees scrollHeight > clientHeight, but the 'Scroll to bottom' visibility depends on Astryx's 100px buttonThreshold — a compact 3-message transcript could fail at that assertion with a misleading error. Assert sh - ch > 100 with diagnostics instead.
• The "no remount → draft survives" constraint has no e2e guard: composer-skill-invocation / skill-draft-lifecycle only cover revision drafts, not cross-conversation persistence. Typing a draft in test 2 before switching (and asserting it survives) would pin the property that makes the prop-not-key design load-bearing. Cheap.
• The quote-companion side (conversationKey for the fork session) has no e2e coverage — acceptable per the representative-journey guidance, just noting the leak would go unnoticed there.

The fix itself is sound — happy to approve once the selectors are updated.

[cat0825]

Fixed the e2e failures — root cause was not the product fix, it was the spec itself (it had never run on Linux before this merge):

1. Locale: Maka's Astryx copy overrides (astryx-copy.ts) map the scroll buttons to zh ('跳到最新消息'/'滚动到底部') under the default zh fixture locale; the spec hardcoded the en names. Now matches both locales — the assertion is about the affordance appearing/clearing, not Astryx's copy.
2. Strict-mode violation: the prompt anchor rail (upstream perf(ui): seed transcript geometry before progressive fill #2237) renders a preview of sent text, so loose getByText(/Fake backend received: .../) matched two nodes. Now uses { exact: true } on the echo.
3. Threshold: growTranscriptOverflow required only sh > ch; Astryx flags scrolled-up only past buttonThreshold (100px), so a compact font could keep the button unrendered. Grows until sh - ch > 150px.

Verified locally: 2/2 pass. Pushed as 243b921.

[Astro-Han]

Thanks — the new commit resolves everything from the prior pass: the locale-agnostic matchers cover both the zh override (astryx-copy.ts:114-115) and Astryx's en defaults, the { exact: true } echoes correctly dodge the prompt-rail preview (which stays mounted via HoverCard), and the 150px growth bound (buttonThreshold 100 + headroom) is bounded and cannot hang. I also verified the conversationKey passthrough at both call sites (app-shell and quote-companion — the companion's companionSession?.id ?? sourceSession?.id is the right key, since each quote source forks a new session), the patch is exactly the 3 #2205-related hunks and applies cleanly to pristine 0.2.0, and both tests fail on the pre-fix product code.

One new finding, tracked as a follow-up rather than a blocker:

P2 (deferred, not blocking) — transient strict-mode double match during streaming. Between the fake backend's chunk 6 and 7 (~45ms window), the rail's reply preview normalizes+trims to exactly the query string while the transcript's first <p> also matches — two elements at a poll instant → Playwright's strict-mode violation is non-retriable, so the whole spec hard-fails for that run (probability is phase-dependent; author's local runs and CI are green). One-line fix: scope the echo locators to the message list (.maka-chat-message-list — the rail is a sibling), or assert after settle. If we see strict mode violation in future CI runs of this spec, that's the cause.

P3s (open, optional): the quote-companion conversationKey path still has no e2e coverage (the spec covers only the sessions surface), and the fixed 250ms wait could be a poll.

Merging now — thanks for the fast turnaround!

[cat0825]

Fixes#2205

Summary

The desktop chat surface's "New messages" scroll-to-bottom indicator now clears when the user scrolls back to the bottom, and its state resets fully when switching conversations. Repro screenshots are attached in the issue.

Root cause

Two independent problems in @astryxdesign/core@0.2.0:

1. Not dismissed at the bottom.useChatNewMessages only clears hasNewMessages via dismiss() (the indicator button). useChatStreamScroll re-locks auto-follow on scrollend but never reports "at bottom" back to the indicator, so the label stays visible after the user scrolls back down.
2. Leaks across conversations.ChatLayout is not remounted per conversation — lastMessageRef (pointing at the previous conversation's last message node) and the unlocked isLocked=false survive the switch, so the first message of a new conversation re-flags hasNewMessages.

Changes

• apps/desktop/src/renderer/app-shell.tsx, quote-companion-panel.tsx: key ChatSurfaceLayout per conversation so the scroll/new-message state is remounted and reset on conversation switch.
• patches/@astryxdesign+core+0.2.0.patch: adds the two @astryxdesign/core hunks for this fix (ChatLayout.js, useChatNewMessages.js). The file already carried the fix(ui): localize the field required/optional marker app-wide #2184FieldLabel localization hunks, so the patch now contains both sets; verified with git apply --check that the merged patch applies cleanly to a pristine @astryxdesign/core@0.2.0.
• patches/README.md: rationale and removal conditions for the new hunks, following the repo convention.
• apps/desktop/e2e/new-messages-indicator.spec.ts: Playwright regression coverage for both reported symptoms.

Verification

• New e2e spec passes; with the fix reverted the two new cases fail exactly as described in the issue (negative control).
• Regression: quote-companion, send-message, scroll-geometry, sidebar-navigation e2e suites (17 tests), packages/ui unit tests (316/316), desktop typecheck and biome all clean.

[Astro-Han]

Thanks for this — the fix is well-scoped and I verified it end to end in a browser harness against the patched dist: the badge appears on unlocked growth, clears on real scrollend at bottom, resets on conversation switch, and the composer element survives the switch (no remount — conversationKey is a plain prop, the patch resets in place, which also keeps it compatible with #2239's arrival-pin approach; that PR is still open, so nothing merged is at risk). The patch applies cleanly to a pristine @astryxdesign/core@0.2.0, and the patches/README.md conflict with the merged #2225 is purely mechanical (both appended sections; keep both).

P1 — the e2e selectors no longer match main: #2202 (merged after your branch point) localized the two Astryx keys. The spec's header comment claims "Maka's Astryx overrides do not cover these keys", but packages/ui/src/astryx-i18n.tsx:97-98 covers both — '@astryx.chatLayout.newMessages' → '跳到最新消息' and '@astryx.chatLayoutScrollButton.scrollToBottom' → '滚动到底部' — and the e2e window fixture runs zh. After a squash-merge both tests would fail at the first English-name assertion (scroll-geometry-adjacent, on the e2e shard). Please switch the selectors to '跳到最新消息' / '滚动到底部' (the rest of the spec already uses zh names) and fix the comment — otherwise the regression guard ships red and its negative control is void.

P3 (optional):

• growTranscriptOverflow only guarantees scrollHeight > clientHeight, but the 'Scroll to bottom' visibility depends on Astryx's 100px buttonThreshold — a compact 3-message transcript could fail at that assertion with a misleading error. Assert sh - ch > 100 with diagnostics instead.
• The "no remount → draft survives" constraint has no e2e guard: composer-skill-invocation / skill-draft-lifecycle only cover revision drafts, not cross-conversation persistence. Typing a draft in test 2 before switching (and asserting it survives) would pin the property that makes the prop-not-key design load-bearing. Cheap.
• The quote-companion side (conversationKey for the fork session) has no e2e coverage — acceptable per the representative-journey guidance, just noting the leak would go unnoticed there.

The fix itself is sound — happy to approve once the selectors are updated.

[cat0825]

Fixed the e2e failures — root cause was not the product fix, it was the spec itself (it had never run on Linux before this merge):

1. Locale: Maka's Astryx copy overrides (astryx-copy.ts) map the scroll buttons to zh ('跳到最新消息'/'滚动到底部') under the default zh fixture locale; the spec hardcoded the en names. Now matches both locales — the assertion is about the affordance appearing/clearing, not Astryx's copy.
2. Strict-mode violation: the prompt anchor rail (upstream perf(ui): seed transcript geometry before progressive fill #2237) renders a preview of sent text, so loose getByText(/Fake backend received: .../) matched two nodes. Now uses { exact: true } on the echo.
3. Threshold: growTranscriptOverflow required only sh > ch; Astryx flags scrolled-up only past buttonThreshold (100px), so a compact font could keep the button unrendered. Grows until sh - ch > 150px.

Verified locally: 2/2 pass. Pushed as 243b921.

[Astro-Han]

Thanks — the new commit resolves everything from the prior pass: the locale-agnostic matchers cover both the zh override (astryx-copy.ts:114-115) and Astryx's en defaults, the { exact: true } echoes correctly dodge the prompt-rail preview (which stays mounted via HoverCard), and the 150px growth bound (buttonThreshold 100 + headroom) is bounded and cannot hang. I also verified the conversationKey passthrough at both call sites (app-shell and quote-companion — the companion's companionSession?.id ?? sourceSession?.id is the right key, since each quote source forks a new session), the patch is exactly the 3 #2205-related hunks and applies cleanly to pristine 0.2.0, and both tests fail on the pre-fix product code.

One new finding, tracked as a follow-up rather than a blocker:

P2 (deferred, not blocking) — transient strict-mode double match during streaming. Between the fake backend's chunk 6 and 7 (~45ms window), the rail's reply preview normalizes+trims to exactly the query string while the transcript's first <p> also matches — two elements at a poll instant → Playwright's strict-mode violation is non-retriable, so the whole spec hard-fails for that run (probability is phase-dependent; author's local runs and CI are green). One-line fix: scope the echo locators to the message list (.maka-chat-message-list — the rail is a sibling), or assert after settle. If we see strict mode violation in future CI runs of this spec, that's the cause.

P3s (open, optional): the quote-companion conversationKey path still has no e2e coverage (the spec covers only the sessions surface), and the fixed 250ms wait could be a poll.

Merging now — thanks for the fast turnaround!

[cat0825]

Fixes#2205

Summary

The desktop chat surface's "New messages" scroll-to-bottom indicator now clears when the user scrolls back to the bottom, and its state resets fully when switching conversations. Repro screenshots are attached in the issue.

Root cause

Two independent problems in @astryxdesign/core@0.2.0:

1. Not dismissed at the bottom.useChatNewMessages only clears hasNewMessages via dismiss() (the indicator button). useChatStreamScroll re-locks auto-follow on scrollend but never reports "at bottom" back to the indicator, so the label stays visible after the user scrolls back down.
2. Leaks across conversations.ChatLayout is not remounted per conversation — lastMessageRef (pointing at the previous conversation's last message node) and the unlocked isLocked=false survive the switch, so the first message of a new conversation re-flags hasNewMessages.

Changes

• apps/desktop/src/renderer/app-shell.tsx, quote-companion-panel.tsx: key ChatSurfaceLayout per conversation so the scroll/new-message state is remounted and reset on conversation switch.
• patches/@astryxdesign+core+0.2.0.patch: adds the two @astryxdesign/core hunks for this fix (ChatLayout.js, useChatNewMessages.js). The file already carried the fix(ui): localize the field required/optional marker app-wide #2184FieldLabel localization hunks, so the patch now contains both sets; verified with git apply --check that the merged patch applies cleanly to a pristine @astryxdesign/core@0.2.0.
• patches/README.md: rationale and removal conditions for the new hunks, following the repo convention.
• apps/desktop/e2e/new-messages-indicator.spec.ts: Playwright regression coverage for both reported symptoms.

Verification

• New e2e spec passes; with the fix reverted the two new cases fail exactly as described in the issue (negative control).
• Regression: quote-companion, send-message, scroll-geometry, sidebar-navigation e2e suites (17 tests), packages/ui unit tests (316/316), desktop typecheck and biome all clean.

[Astro-Han]

Thanks for this — the fix is well-scoped and I verified it end to end in a browser harness against the patched dist: the badge appears on unlocked growth, clears on real scrollend at bottom, resets on conversation switch, and the composer element survives the switch (no remount — conversationKey is a plain prop, the patch resets in place, which also keeps it compatible with #2239's arrival-pin approach; that PR is still open, so nothing merged is at risk). The patch applies cleanly to a pristine @astryxdesign/core@0.2.0, and the patches/README.md conflict with the merged #2225 is purely mechanical (both appended sections; keep both).

P1 — the e2e selectors no longer match main: #2202 (merged after your branch point) localized the two Astryx keys. The spec's header comment claims "Maka's Astryx overrides do not cover these keys", but packages/ui/src/astryx-i18n.tsx:97-98 covers both — '@astryx.chatLayout.newMessages' → '跳到最新消息' and '@astryx.chatLayoutScrollButton.scrollToBottom' → '滚动到底部' — and the e2e window fixture runs zh. After a squash-merge both tests would fail at the first English-name assertion (scroll-geometry-adjacent, on the e2e shard). Please switch the selectors to '跳到最新消息' / '滚动到底部' (the rest of the spec already uses zh names) and fix the comment — otherwise the regression guard ships red and its negative control is void.

P3 (optional):

• growTranscriptOverflow only guarantees scrollHeight > clientHeight, but the 'Scroll to bottom' visibility depends on Astryx's 100px buttonThreshold — a compact 3-message transcript could fail at that assertion with a misleading error. Assert sh - ch > 100 with diagnostics instead.
• The "no remount → draft survives" constraint has no e2e guard: composer-skill-invocation / skill-draft-lifecycle only cover revision drafts, not cross-conversation persistence. Typing a draft in test 2 before switching (and asserting it survives) would pin the property that makes the prop-not-key design load-bearing. Cheap.
• The quote-companion side (conversationKey for the fork session) has no e2e coverage — acceptable per the representative-journey guidance, just noting the leak would go unnoticed there.

The fix itself is sound — happy to approve once the selectors are updated.

[cat0825]

Fixed the e2e failures — root cause was not the product fix, it was the spec itself (it had never run on Linux before this merge):

1. Locale: Maka's Astryx copy overrides (astryx-copy.ts) map the scroll buttons to zh ('跳到最新消息'/'滚动到底部') under the default zh fixture locale; the spec hardcoded the en names. Now matches both locales — the assertion is about the affordance appearing/clearing, not Astryx's copy.
2. Strict-mode violation: the prompt anchor rail (upstream perf(ui): seed transcript geometry before progressive fill #2237) renders a preview of sent text, so loose getByText(/Fake backend received: .../) matched two nodes. Now uses { exact: true } on the echo.
3. Threshold: growTranscriptOverflow required only sh > ch; Astryx flags scrolled-up only past buttonThreshold (100px), so a compact font could keep the button unrendered. Grows until sh - ch > 150px.

Verified locally: 2/2 pass. Pushed as 243b921.

[Astro-Han]

Thanks — the new commit resolves everything from the prior pass: the locale-agnostic matchers cover both the zh override (astryx-copy.ts:114-115) and Astryx's en defaults, the { exact: true } echoes correctly dodge the prompt-rail preview (which stays mounted via HoverCard), and the 150px growth bound (buttonThreshold 100 + headroom) is bounded and cannot hang. I also verified the conversationKey passthrough at both call sites (app-shell and quote-companion — the companion's companionSession?.id ?? sourceSession?.id is the right key, since each quote source forks a new session), the patch is exactly the 3 #2205-related hunks and applies cleanly to pristine 0.2.0, and both tests fail on the pre-fix product code.

One new finding, tracked as a follow-up rather than a blocker:

P2 (deferred, not blocking) — transient strict-mode double match during streaming. Between the fake backend's chunk 6 and 7 (~45ms window), the rail's reply preview normalizes+trims to exactly the query string while the transcript's first <p> also matches — two elements at a poll instant → Playwright's strict-mode violation is non-retriable, so the whole spec hard-fails for that run (probability is phase-dependent; author's local runs and CI are green). One-line fix: scope the echo locators to the message list (.maka-chat-message-list — the rail is a sibling), or assert after settle. If we see strict mode violation in future CI runs of this spec, that's the cause.

P3s (open, optional): the quote-companion conversationKey path still has no e2e coverage (the spec covers only the sessions surface), and the fixed 250ms wait could be a poll.

Merging now — thanks for the fast turnaround!