Files
hermes-webui/tests/test_issue4006_auto_scroll_follow_default.py
T
a662e37f89 fix(settings): honor persisted auto-follow across boot failures and partial bodies (#6856)
* fix(settings): honor persisted auto-follow across boot failures and partial bodies (#6819)

The Auto-follow new content toggle was silently re-enabled by two paths:
- boot.js settings-fetch-failure fallback hardcoded _autoScrollFollow=true,
  clobbering an explicit OFF for the whole session (post-turn scroll yanks
  until refresh);
- _applySavedSettingsUi assigned body.auto_scroll_follow!==false
  unconditionally, so a partial settings body without the key evaluated
  undefined!==false -> true.

Fix: mirror the resolved value into profile-namespaced localStorage
(JSON map keyed by active profile), make the boot fallback read the mirror
instead of hardcoding ON, and only override the runtime flag when a settings
body actually owns the key. Fresh users (no mirror) still default to ON.

Tests: update test_issue4006 fallback assertion; add
test_issue6819_auto_scroll_follow_persistence covering source-shape guards
and extracted-helper behavior (fresh default, persisted OFF, profile
isolation, partial-body no-clobber, save-path persist).

* fix(settings): resolve auto-follow mirror profile from cookie before S.activeProfile init (#6856 review)

P1 (greptile-apps review): the profile-namespaced mirror was keyed by
S.activeProfile, but the boot settings-fetch-failure fallback runs before
boot resolves S.activeProfile, so a non-default profile with a failed fetch
read the 'default' mirror entry. Resolve the key from the hermes_profile
cookie (set per-client by /api/profile/switch, readable synchronously at
page load) first, falling back to S.activeProfile, then 'default'.

P2: document the persistence contract in the helper block (storage shape,
profile-key resolution order, read semantics, upgrade path).

Tests: extend the extracted-helper node test with the P1 scenario (cookie
authoritative over stale S.activeProfile, per-profile isolation via cookie,
no-cookie fallback) and add source-shape guards for the cookie read.

* fix(settings): defer auto-follow mirror read until profile resolves (P1 follow-up)

greptile-apps review: the hermes_profile cookie is HttpOnly — document.cookie
can never read it, so the cookie-based profile key from 39fed70b always fell
back to 'default', re-introducing the wrong-namespace bug for non-default
profiles on a failed settings fetch.

Correct model: the active profile is only known asynchronously via
/api/profile/active. The boot settings-fetch-failure path now sets a safe
default (ON) plus a deferred-reapply flag; once S.activeProfile resolves,
the mirror is read with the profile-correct namespace and the flag clears.
The success path (which already applied+persisted the authoritative value)
never touches the flag.

Tests updated to the deferred model: source-shape guard asserts the re-apply
runs AFTER profile resolution (fallback < profile-resolve < reapply ordering),
no document.cookie read in the profile key, and the node behavioral test
simulates boot-failure -> profile-resolve -> deferred re-apply.

* fix(settings): defer auto-follow mirror write until profile resolves (P1 round 3)

greptile-apps review: the SUCCESS boot path called _persistAutoScrollFollow
before S.activeProfile initialized, so a non-default profile's successful
settings fetch stored its value in the 'default' namespace. A later
failed-fetch boot would then find no entry for the real profile and restore
auto-follow ON.

The success path now defers the mirror write the same way the failure path
defers its read: it records the value in _autoScrollFollowDeferredPersist,
and the post-profile-resolution block persists it under the correct
namespace (then clears the flag). Both deferred branches now run together
after S.activeProfile resolves.

Tests: success path must not call _persistAutoScrollFollow before profile
resolution (source-shape), deferred-persist anchor ordered after profile
resolve, and the node behavioral test covers success-boot -> profile-resolve
-> deferred write under the resolved profile (and NOT under default).

* fix(settings): make auto-follow mirror global to match the backend contract (maintainer review)

Maintainer review on #6856: the mirror was profile-keyed, but the backend
authority is GLOBAL — SETTINGS_FILE = STATE_DIR / 'settings.json'
(api/config.py), and /api/settings calls load_settings() with no
profile-specific file. A profile-keyed map leaves a freshly-opened profile
with no mirror entry, so a failed settings fetch on that profile wrongly
restored ON despite the global OFF — the original failure stayed
reproducible.

Revert to a single global last-known value ('1'/'0' under one key):
- _persistAutoScrollFollow / _readPersistedAutoScrollFollow now use a
  plain scalar, not a JSON profile map.
- Boot success path persists synchronously (no deferred write — the global
  mirror has no profile dimension).
- Boot failure fallback reads synchronously (no deferred re-apply — no
  profile resolution needed).
- Remove _autoScrollFollowProfileKey, _autoScrollFollowDeferredPersist,
  _autoScrollFollowDeferredReapply entirely.

Tests: behavioral node test now asserts the maintainer-requested regression
(global OFF survives profile change + failed fetch, single scalar storage);
source-shape guards assert the profile map and deferred machinery are gone;
boot success persists; partial-body and save-path assertions unchanged.
14/14 pass.

* fix(settings): never persist synthesized auto-follow from a failed autosave (Greptile P1)

Greptile review on #6856: in _autosaveAppearanceSettings, when the POST
fails (saved is falsy), '!saved' synthesizes _autoScrollFollow=true, and
the added persist call wrote that synthesized ON into the mirror. A later
boot settings-fetch failure would then restore ON from the corrupted
mirror, reintroducing post-turn scroll yanks for a user who explicitly
disabled auto-follow.

Persist ONLY from an explicit boolean in the server response:
  if(saved && typeof saved.auto_scroll_follow==='boolean' && typeof _persistAutoScrollFollow==='function'){
    _persistAutoScrollFollow(saved.auto_scroll_follow);
  }
A failed save or a partial response without the key leaves the mirror
untouched (last authoritative value preserved).

Tests: source-shape guard asserts the explicit-boolean requirement; the
node behavioral test replicates the failed-autosave path (mirror stays
OFF) and the successful-autosave path (explicit boolean persists).

* fix(settings): sync auto-follow mirror on successful settings-panel GET

Gate review (Codex) found a stale-mirror gap in #6856's own contract: a
successful loadSettingsPanel() GET applied the authoritative auto_scroll_follow
value to window._autoScrollFollow but did not persist the global mirror, so an
explicit OFF applied via the panel left a stale ON mirror that a later
boot-fetch failure would restore (reopening #6819). Persist from the panel GET
too, only when the server sent an explicit boolean (never a synthesized
default) — matching the boot + autosave contract. Adds a bite-tested regression.

Co-authored-by: sand01chi <sand01chi@users.noreply.github.com>

---------

Co-authored-by: sand01chi <sand01chi@users.noreply.github.com>
Co-authored-by: nesquena-hermes <nesquena-hermes@users.noreply.github.com>
2026-08-09 03:36:49 +00:00

191 lines
9.4 KiB
Python

"""Pin the auto_scroll_follow setting's default + hydration consistency (#4006).
The viewport-follow default is `True` (Codex/Claude-Code-style sticky bottom:
follow new output to the bottom while streaming, but a deliberate scroll-up
unpins and is respected). This pins the default in every place it is read so a
future edit can't silently flip it or, worse, default it ON in config.py while
hydrating it OFF in the browser (the classic default-mismatch bug, where an
existing user with no saved value sees the feature as disabled).
"""
import json
import pathlib
import re
import shutil
import subprocess
import textwrap
import pytest
ROOT = pathlib.Path(__file__).resolve().parent.parent
def _read(rel):
return (ROOT / rel).read_text(encoding="utf-8")
def test_auto_scroll_follow_default_is_true_in_config():
src = _read("api/config.py")
assert re.search(r'["\']auto_scroll_follow["\']\s*:\s*True', src), (
"auto_scroll_follow must default to True in _SETTINGS_DEFAULTS "
"(sticky-bottom follow; scroll-up unpins)"
)
def test_auto_scroll_follow_in_bool_keys():
src = _read("api/config.py")
m = re.search(r"_SETTINGS_BOOL_KEYS\s*=\s*\{([^}]+)\}", src, re.DOTALL)
assert m, "_SETTINGS_BOOL_KEYS not found"
assert "auto_scroll_follow" in m.group(1), (
"auto_scroll_follow must be in _SETTINGS_BOOL_KEYS so it round-trips as a bool"
)
def test_boot_hydration_defaults_true_when_setting_absent():
"""boot.js must hydrate _autoScrollFollow as True when the saved settings
omit the key — `!!s.auto_scroll_follow` would wrongly default it OFF for
every existing user, contradicting the config.py default."""
src = _read("static/boot.js")
# Settings path: default-true read (=== false), not the truthy-coerce form,
# and the resolved value is persisted into the #6819 mirror (global —
# matches the one global settings.json, per maintainer review on #6856).
assert "window._autoScrollFollow=_persistAutoScrollFollow(s.auto_scroll_follow!==false)" in src, (
"boot.js settings path must default _autoScrollFollow True when absent "
"(and persist the resolved value into the #6819 mirror)"
)
assert "window._autoScrollFollow=!!s.auto_scroll_follow" not in src, (
"boot.js must not use !!s.auto_scroll_follow — that defaults the True "
"setting OFF for users with no saved value"
)
# Fallback (no-settings) path must also default true for a fresh user
# (no persisted mirror), while honoring a persisted OFF (#6819).
assert "window._autoScrollFollow=_readPersistedAutoScrollFollow()" in src, (
"boot.js fallback path must read the persisted auto-follow mirror "
"(defaults true for fresh users, honors saved OFF after a transient "
"settings-fetch failure)"
)
def test_settings_checkbox_renders_checked_by_default():
"""The Appearance checkbox must render checked when the setting is absent,
matching the True default (panels.js settings-load)."""
src = _read("static/panels.js")
assert "autoScrollFollowCb.checked=settings.auto_scroll_follow!==false" in src, (
"the auto-follow checkbox must default checked (=== false), not "
"!!settings.auto_scroll_follow which would render it unchecked by default"
)
def test_follow_gate_references_auto_scroll_follow_and_unpin():
"""The DOM-replace follow gate must consult both _autoScrollFollow (the
setting) and _messageUserUnpinned (the user's scroll-up), so the opt-out and
the read-while-streaming behaviors both hold."""
src = _read("static/ui.js")
assert "_shouldFollowMessagesOnDomReplace" in src
assert "_autoScrollFollow" in src and "_messageUserUnpinned" in src, (
"the follow gate must reference both _autoScrollFollow and _messageUserUnpinned"
)
def test_ui_sync_init_defaults_auto_scroll_follow_before_consumers():
"""ui.js must establish window._autoScrollFollow synchronously at the
top-level scroll-state section — before boot.js hydrates it from the awaited
settings request. ui.js/messages.js load before boot.js (index.html), so any
scroll listener / settle path / scrollIfPinned / DOM-replace gate running in
that window reads an undeclared binding and throws ReferenceError (#6606).
The typeof guard makes the default `true` WITHOUT clobbering an explicit
saved `false` if boot.js already hydrated it."""
src = _read("static/ui.js")
assert "if(typeof window._autoScrollFollow==='undefined'){ window._autoScrollFollow=true; }" in src, (
"ui.js must synchronously default window._autoScrollFollow to true before "
"boot.js hydration (bare reads otherwise throw ReferenceError pre-boot)"
)
# The sync init must sit BEFORE every consumer it protects in the file.
init_idx = src.index("window._autoScrollFollow=true;")
scroll_listener_idx = src.index("!movedUp && window._autoScrollFollow && _scrollPinned")
gate_idx = src.index("window._autoScrollFollow && !_messageUserUnpinned")
settle_idx = src.index("!window._autoScrollFollow&&!explicit")
pinned_idx = src.index("function scrollIfPinned")
for label, idx in (
("scroll listener", scroll_listener_idx),
("DOM-replace gate", gate_idx),
("settle guard", settle_idx),
("scrollIfPinned", pinned_idx),
):
assert init_idx < idx, f"sync init must precede the {label} consumer"
def test_ui_readers_use_explicit_window_owner():
"""Every _autoScrollFollow reader in ui.js must go through the explicit
window._autoScrollFollow owner — no bare undeclared reads that can throw
ReferenceError before boot.js assigns the global (#6606)."""
src = _read("static/ui.js")
# Any bare `_autoScrollFollow` token not preceded by `window.` is a bug.
for m in re.finditer(r"(?<![\w.])_autoScrollFollow", src):
before = src[max(0, m.start() - 7):m.start()]
assert before == "window.", (
f"bare _autoScrollFollow read at ui.js offset {m.start()}: "
f"...{src[max(0, m.start()-40):m.start()+20]!r}"
)
@pytest.mark.skipif(shutil.which("node") is None, reason="node required for behavioral test")
def test_auto_scroll_follow_undefined_true_false_behavior():
"""Behavioral: extract the real follow expressions from ui.js and evaluate
them in Node across undefined / true / false:
- undefined must NOT throw (property read, not a bare undeclared binding)
and must not block an explicit bottom settlement;
- false must suppress automatic following (DOM-replace gate, settle guard,
scrollIfPinned) WITHOUT suppressing explicit bottom settlement;
- true keeps following enabled."""
src = _read("static/ui.js")
# Extract the exact live expressions (not hand-copied replicas).
dom_replace_expr = "window._autoScrollFollow && !_messageUserUnpinned && (_scrollPinned || _isMessagePaneNearBottom(120))"
assert dom_replace_expr in src, "DOM-replace follow gate expression changed"
settle_expr = "(!window._autoScrollFollow&&!explicit)||!_scrollPinned||_messageUserUnpinned||_recentNonMessageScrollIntent()"
assert settle_expr in src, "settle guard expression changed"
pinned_guard_expr = "!window._autoScrollFollow"
assert f"if({pinned_guard_expr}) return;" in src, "scrollIfPinned guard changed"
def run(expr, follow, explicit=False, pinned=True, unpinned=False, near=True):
follow_js = "undefined" if follow is None else ("true" if follow else "false")
harness = textwrap.dedent(f"""
const window = {{ _autoScrollFollow: {follow_js} }};
let _scrollPinned = {str(pinned).lower()};
let _messageUserUnpinned = {str(unpinned).lower()};
let explicit = {str(explicit).lower()};
function _isMessagePaneNearBottom(px){{ return {str(near).lower()}; }}
function _recentNonMessageScrollIntent(){{ return false; }}
let _out;
try {{
_out = ({expr});
}} catch (e) {{
console.error('THREW: ' + e);
process.exit(2);
}}
console.log(JSON.stringify(_out === undefined ? '__UNDEF__' : _out));
""")
res = subprocess.run(["node", "-e", harness], capture_output=True, text=True, timeout=30)
assert res.returncode == 0, res.stderr
val = json.loads(res.stdout.strip())
return None if val == "__UNDEF__" else val
# undefined: property read → falsy, never throws, explicit settle still allowed.
assert not run(dom_replace_expr, None) # undefined && … → undefined (falsy)
assert run(settle_expr, None) is True # suppress implicit settle
assert run(settle_expr, None, explicit=True) is False # explicit settle proceeds
assert run(pinned_guard_expr, None) is True # scrollIfPinned returns early
# true: follow enabled everywhere.
assert run(dom_replace_expr, True) is True
assert run(settle_expr, True) is False # implicit settle proceeds
assert run(pinned_guard_expr, True) is False # scrollIfPinned does NOT early-return
# false: automatic follow suppressed…
assert run(dom_replace_expr, False) is False
assert run(settle_expr, False) is True
assert run(pinned_guard_expr, False) is True
# …but an EXPLICIT bottom settlement still runs (false must not block ↓/open).
assert run(settle_expr, False, explicit=True) is False
# Unpinned reader is never re-followed, regardless of the setting.
assert run(dom_replace_expr, True, unpinned=True) is False