Release v0.51.334 — Release KX (new-message cue when scrolled up, #3631)

[nesquena-hermes]

Release v0.51.334 — Release KX (new-message cue when scrolled up)

Absorbs #3631 (@rodboev) — closes #3545. Deep-reviewed (Opus + Codex), rebased, fixed, and shipped.

What ships

If you scroll up to read while a turn is still arriving, a new message previously either landed silently off-screen or yanked you to the bottom. Now the jump-to-bottom button shows a "New message" cue you can click to catch up. Pinned / at-bottom readers still auto-follow to the latest response exactly as before.

Deep review (Opus + Codex + independent trace)

• Codex caught a CORE regression the green suite + Opus missed: the PR's preserve-scroll branch used if(_scrollPinned) scrollIfPinned(), but scrollIfPinned() skips the synchronous bottom write unless distance > 500 and its delayed settles can be cancelled by the DOM-rebuild scroll event — so a pinned reader could be left a few lines above the settled final response.
• Fix (maintainer, this PR): restored master's forced follow path — if(_followMessagesAfterDomReplace()) return; (which does a forced scrollToBottom()) for pinned/near-bottom users; only genuinely scrolled-up (unpinned, not near-bottom) users restore their viewport and get the new-message cue. Updated the 3 structure-pinning tests to assert the corrected shape while preserving their behavioral intent.
• Independently traced: snapshot-shape concern is clean (every snapshot comes from the single _captureMessageScrollSnapshot() producer, which now always includes scrollHeight; the only other path passes null, handled by an early-return). Cue-state (_newMessageCueVisible) is reset on every boundary — session switch, stream start, scrollToBottom(), and natural scroll-near-bottom.
• i18n: the PR shipped the two new session_new_message* keys to 12 of 13 locales — Polish (pl) was missing both. Added the Polish translations → all 13/13, locale-parity test green.

Greptile flags (all adjudicated)

• "double _syncScrollToBottomCue dispatch on near-bottom tick" → FOLD-cosmetic (idempotent synchronous style writes, no flicker)
• "24px growth threshold too small on fractional-DPR" → REJECT (tuning speculation, not a bug)
• "redundant _syncScrollToBottomCue after _clearNewMessageScrollCue in scrollToBottom" → FOLD-cosmetic (same as Portability #1)

Gate

• Full suite 8308 passed, 9 skipped, 3 xpassed (4:29)
• node -c ✓ · ESLint runtime CLEAN · scope-undef CLEAN · ruff forward CLEAN
• Opus: SHIP-safe · Codex: SAFE TO SHIP (after the pinned-user fix to its CORE finding)

Co-authored-by: rodboev rodboev@users.noreply.github.com

[greptile-apps]

Greptile Summary

Implements the "New message" cue on the jump-to-bottom button for users who scroll up during streaming, while keeping the existing forced-follow behavior for pinned and near-bottom readers. The core scroll-state machine gets a _newMessageCueVisible flag and three helpers (_syncScrollToBottomCue, _showNewMessageScrollCue, _clearNewMessageScrollCue) that are wired into every state-transition boundary.

• Regression fix included: the original PR used scrollIfPinned() in the pinned preserve-scroll branch, which skips the synchronous bottom write and can be cancelled by DOM-rebuild scroll events; this PR restores the forced _followMessagesAfterDomReplace() path so pinned readers always land at the true settled bottom.
• i18n: session_new_message / session_new_message_label added to all 13 locales (Polish was missing from the original contribution and is added here).
• State cleanup: the cue is cleared on session switch, stream start, natural scroll near-bottom, and explicit scrollToBottom() calls, covering all exit paths.

Confidence Score: 4/5

Safe to merge; the scroll-state machine changes are well-scoped and the Codex-caught regression (forced follow for pinned users) is correctly addressed.

The new-message cue logic is isolated behind a single boolean flag with consistent cleanup at every state boundary. The only findings are two style-level test observations: one test asserts ≥8 locale entries when 13 are present (weak lower bound), and one assertion embeds the exact inline comment wording making it fragile to reformatting. Neither affects runtime behavior.

The two updated structural-assertion tests (test_issue1690_scroll_completion.py and test_issue3545_streaming_scroll_cue.py) are worth a quick read if the codebase style for literal string tests is ever revisited.

Important Files Changed

Filename	Overview
static/ui.js	Core scroll-state machine change: adds _newMessageCueVisible flag, _syncScrollToBottomCue/show/clear helpers, and _maybeShowNewMessageScrollCue decision function; hooks them into renderMessages preserve-scroll path, scrollToBottom, and both reset functions. Codex regression (scrollIfPinned vs forced follow) is correctly fixed.
static/i18n.js	Adds session_new_message and session_new_message_label keys to all 13 locales; Polish (pl) added by maintainer to complete the set. Translations look accurate.
static/style.css	Adds pill styling for the --new-message button variant and makes .session-jump-btn__text visible inside it; the .session-jump-btn__text{display:none} rule that follows is intentionally unrelated to the new modifier class.
tests/test_issue3545_streaming_scroll_cue.py	New test file covering cue show/clear lifecycle, growth threshold logic, i18n key presence (asserts >=8 locales when 13 are present — weak lower bound but green), and CSS pill-style existence.
tests/test_issue1690_scroll_completion.py	Updated assertion now matches the exact multi-line code shape of the preserve-scroll branch including the comment text; structurally correct but brittle against whitespace/comment rewording.
tests/test_issue677.py	Widens scroll-listener search window from 1200 to 1400 chars and updates assertion from raw btn.style.display to _syncScrollToBottomCue call site; correct.
tests/test_session_jump_buttons.py	Single assertion updated to match the new _syncScrollToBottomCue call; no behavioral change to what is tested.
tests/test_tars_scroll_reset_regressions.py	Updated to assert that _maybeShowNewMessageScrollCue immediately follows _restoreMessageScrollSnapshot on the same source line (literal newline match); correct but brittle.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[renderMessages called\npreserveScroll=true] --> B[_captureMessageScrollSnapshot\nrecords scrollTop + scrollHeight]
    B --> C[DOM rebuilt]
    C --> D[_scrollAfterMessageRender]
    D --> E{_followMessagesAfterDomReplace?\npinned / near-bottom}
    E -- yes --> F[scrollToBottom forced\n_clearNewMessageScrollCue]
    E -- no scrolled-up user --> G[_restoreMessageScrollSnapshot\nrestores old scrollTop]
    G --> H[_maybeShowNewMessageScrollCue]
    H --> I{scrollHeight grew\n>24px AND distance>80?}
    I -- yes --> J[_showNewMessageScrollCue\nbtn shows New message pill]
    I -- no --> K[_syncScrollToBottomCue\npreserve current cue state]
    L[User scrolls naturally] --> M{nearBottom?\ndistance<250}
    M -- yes --> N[_clearNewMessageScrollCue\nbtn reverts to normal / hides]
    M -- no --> O[_syncScrollToBottomCue\nmaintain current cue state]
    P[scrollToBottom called\nor session/stream reset] --> Q[_clearNewMessageScrollCue\n_newMessageCueVisible=false]

Loading

_{Reviews (1): Last reviewed commit: "Release v0.51.334 — Release KX (new-mess..." | Re-trigger Greptile}

[greptile-apps]

Brittle literal-whitespace assertions on comment text

Both new assertions match exact whitespace and the inline comment "// Keep master's follow heuristic". If the block is ever reformatted or the comment is updated (e.g. issue number changes), this test will fail despite the code being functionally correct. The behavioral invariant being tested — that _followMessagesAfterDomReplace() comes before _restoreMessageScrollSnapshot and _maybeShowNewMessageScrollCue — could be checked with three independent in checks (already done in test_issue3545_streaming_scroll_cue.py) rather than a single multi-line string that encodes the exact comment wording.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[greptile-apps]

i18n locale count assertion is a weak lower bound

I18N_JS.count("session_new_message:") >= 8 passes even if only 8 of the 13 locales are present. The PR explicitly fixed a missing Polish locale to reach 13/13, so it's worth locking in that count — == 13 would catch any future locale being accidentally dropped.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[greptile-apps]

Pinning the count to == 13 matches the 13-locale parity the maintainer explicitly achieved and ensures future locale additions don't silently regress.

Suggested change

	assert I18N_JS.count("session_new_message:") >= 8
	assert I18N_JS.count("session_new_message_label:") >= 8
	assert I18N_JS.count("session_new_message:") == 13
	assert I18N_JS.count("session_new_message_label:") == 13

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!