From be5ee8f5030ba6940c1d040f0e68d97bafeaa7be Mon Sep 17 00:00:00 2001 From: Kevin Meinon <3797846+KevinMeinon@users.noreply.github.com> Date: Wed, 16 Sep 2026 16:38:43 +0200 Subject: [PATCH] perf(composer): make the single-row autoResize fast path reachable (long-session typing lag) (#7604) * perf(composer): make the single-row autoResize fast path reachable autoResize() skips the height:'auto' -> scrollHeight -> restore round trip only when the composer is already at its one-row height, but the guard compared el.offsetHeight against the CSS min-height (44px) while the composer's natural one-row border box is line-height + vertical padding + borders (~48px at the stock font). 48 <= ceil(44)+1 is false, so the skip was unreachable in a real browser: every append keystroke wrote style.height twice, and the scrollHeight read that follows forces a synchronous layout of the whole document, transcript included. The cost therefore grows with the rendered transcript, which is the "typing gets laggy in long sessions" report. Accept the natural one-row height as the skip ceiling, computed from the same already-read getComputedStyle. The strict finite-pixel gate is kept for every value involved, so a percentage / calc / normal line-height, padding or min-height still fails closed to the full resize (the #6349 re-gate property is preserved, and test_issue5514_composer_grow_scroll_pin.py still pins it). Measured on a 758-message / 5 MB session (31k DOM nodes rendered) with the browser Event Timing API (input -> next paint) at 6x CPU throttle: a keystroke went from 168ms to ~136ms median echo latency (72ms with the transcript detached), 96ms to ~72ms in the windowed state, and the style writes per 8 keystrokes from 16 to 2. The #5514 invariant holds: an oversized composer still takes the full resize because its offsetHeight is far above the one-row ceiling. Tests: - new tests/test_long_session_composer_typing_latency.py runs the real autoResize() body from messages.js in a node sandbox whose getComputedStyle returns the composer's computed styles; it is red before this change (2 failed, 27 passed) and green after (29 passed). - tests/test_issue5514_composer_grow_scroll_pin.py source pins updated to the new guard shape. * test(docs): address #7604 review - behavior-only tests + composer sizing docs Review feedback on the PR flagged two repo-rule items: tests that pin source strings, and an undocumented runtime behavior change. Both are addressed here. - tests/test_long_session_composer_typing_latency.py: delete test_fix_is_pinned_in_source. It asserted exact JS declarations, which the repo testing guidance rejects and which the harness in this module already covers by executing the REAL autoResize() body. In its place add test_natural_row_skip_survives_every_append_of_a_typed_word, which drives the actual typing loop ('' -> 'h' -> ... -> 'hello') and asserts zero height writes on every keystroke. Red on the pre-fix tree, green with the fix. - tests/test_issue5514_composer_grow_scroll_pin.py: convert test_single_line_growth_skips_the_height_round_trip from source-string pins to a behavior fixture whose getComputedStyle returns the composer's real computed styles. It asserts the observable outcome: a one-row append at the natural 48px row takes no height write, while an oversized 176px composer still remeasures back to one row. Red before the fix (2 writes), green after. - docs/UIUX-GUIDE.md: document the composer sizing invariants in the composer section - the single-row skip is a hot path and must stay reachable, its ceiling is the natural one-row height or the CSS min-height (whichever is larger), everything else still remeasures, and non-px computed values fail closed. ./scripts/test.sh on the two touched modules: 29 passed (3 failed on the pre-fix tree). Neighbour sweep over the 51 composer/scroll/typing modules: 386 passed. ruff check clean. * fix(tests,docs): derive composer test dimensions from style.css (#7604 review) Greptile's re-review is right: the fixtures hardcoded the 18px `data-font-size=large` composer (line-height 29.7px -> a 47.7px one-row box) as if it were the stylesheet's 16px default (44.4px). At the default the pre-fix ceiling (ceil(44)+1 = 45px) DID clear the one-row box, so the regression is font-size dependent - the 16px default reaches the skip, the 18px and 20px appearance settings do not - and not a stock-configuration bug. The runtime change is unaffected and unchanged. - new `tests/_composer_metrics.py`: parses `static/style.css` (the base `textarea#msg` rule, the `data-font-size` overrides, the skin overrides) and exposes each configuration's box metrics - natural one-row height, the integer offsetHeight a browser reports, and both the pre-fix and post-fix ceilings - so a fixture cannot encode the wrong font size again. - `tests/test_long_session_composer_typing_latency.py`: parameterized over every configuration the stylesheet can produce, with `test_the_regression_was_font_size_dependent_not_stock` pinning the grounding. - `tests/test_issue5514_composer_grow_scroll_pin.py`: the converted single-row test uses the same derived metrics. - `docs/UIUX-GUIDE.md`: the sizing section now states that the natural row tracks the appearance font size (44 / 48 / 51px) while `min-height` stays 44px, which is why a min-height-only ceiling silently disables the skip. Pre-fix tree: `4 failed, 31 passed` - exactly the large-18px and xlarge-20px parameterizations plus the two tests that assert the reported configuration. Post-fix tree: `35 passed`. Neighbour sweep over the 51 composer/scroll/typing modules: `392 passed`. ruff clean. --- docs/UIUX-GUIDE.md | 27 ++ static/messages.js | 25 +- tests/_composer_metrics.py | 210 ++++++++++++++ ...test_issue5514_composer_grow_scroll_pin.py | 95 ++++++- ...st_long_session_composer_typing_latency.py | 262 ++++++++++++++++++ 5 files changed, 604 insertions(+), 15 deletions(-) create mode 100644 tests/_composer_metrics.py create mode 100644 tests/test_long_session_composer_typing_latency.py diff --git a/docs/UIUX-GUIDE.md b/docs/UIUX-GUIDE.md index 0fe44958..6c38b4b3 100644 --- a/docs/UIUX-GUIDE.md +++ b/docs/UIUX-GUIDE.md @@ -146,6 +146,33 @@ When adding a control, consider where users will find it on both wide desktop an mobile. If a setting or quota/control surface does not fit in the composer, route it through the appropriate Control Center panel instead of squeezing the footer. +### Composer sizing + +The composer grows with its content up to a 200px cap. Where the browser supports +`field-sizing: content` (the stylesheet sets it on `textarea#msg`, with +`field-sizing: fixed` while the placeholder shows) CSS owns that; everywhere else +the JavaScript fallback (`autoResize()` in `static/messages.js`) measures, and it +runs on every keystroke - so treat it as a hot path and keep these invariants when +touching it (regression coverage: +`tests/test_long_session_composer_typing_latency.py`, which derives every +dimension from `static/style.css`, and +`tests/test_issue5514_composer_grow_scroll_pin.py`): + +- A single-row append that already fits its box skips the height round trip. That + round trip reads `scrollHeight`, which forces a synchronous layout of the whole + document, so its cost grows with the rendered transcript - this is the + long-session typing-lag class. Do not remove the skip. +- The skip's ceiling is the textarea's natural ONE-ROW height (`line-height` + + vertical padding + borders) or the CSS `min-height`, whichever is larger. + Compare against that natural row, never against `min-height` alone: the natural + row follows the appearance font size (44px at the 16px default, 48px at + `data-font-size=large`, 51px at `xlarge`), while `min-height` stays 44px, so a + min-height-only ceiling silently disables the skip for the larger sizes. +- Everything else still fully remeasures: an oversized composer, a replacement, a + shrink, a multi-line append, and session/draft restore. +- Non-pixel computed values (a percentage, `calc()`, `auto`) fail closed to the + full resize rather than enabling the skip from a bogus pixel parse. + ## Responsive behavior Mobile is not an afterthought. The repository documents a responsive layout with diff --git a/static/messages.js b/static/messages.js index 50f62a69..92a10712 100644 --- a/static/messages.js +++ b/static/messages.js @@ -7267,9 +7267,28 @@ function autoResize(){ // read as a bogus pixel number (parseFloat("50%")===50), which would wrongly // enable the fast path and leave the composer stuck tall. Reject anything that // is not exactly "px" so those cases fail closed to the full resize. - const _minHeightRaw=_isAppendOnly&&_fitsCurrentHeight?getComputedStyle(el).minHeight:''; - const _minHeight=/^(?:\d+(?:\.\d+)?|\.\d+)px$/.test(_minHeightRaw)?parseFloat(_minHeightRaw):NaN; - const _isAtMinimumHeight=Number.isFinite(_minHeight)&&el.offsetHeight<=Math.ceil(_minHeight)+1; + const _composerStyle=_isAppendOnly&&_fitsCurrentHeight?getComputedStyle(el):null; + const _composerPx=(raw)=>/^(?:\d+(?:\.\d+)?|\.\d+)px$/.test(raw==null?'':String(raw))?parseFloat(raw):NaN; + const _minHeightRaw=_composerStyle?_composerStyle.minHeight:''; + const _minHeight=_composerPx(_minHeightRaw); + // The ONE-ROW height the composer naturally settles at is line-height + + // vertical padding + borders (~48px at the stock font), which is TALLER than + // the CSS min-height (44px). Comparing el.offsetHeight against min-height + // alone therefore never matched in a real browser, so this skip was dead code + // and EVERY append keystroke paid the height:'auto' round trip below — and + // that scrollHeight read forces a synchronous layout of the whole document, + // transcript included. The cost grows with the rendered transcript: on a + // 758-message transcript (31k DOM nodes) a keystroke measured ~168ms median + // echo latency at 6x CPU throttle vs 72ms with the transcript detached. + // Accept the natural one-row height too; an oversized composer still takes the + // full resize because its offsetHeight exceeds that ceiling by far. + const _lineHeight=_composerStyle?_composerPx(_composerStyle.lineHeight):NaN; + const _naturalRowHeight=Number.isFinite(_lineHeight) + ?_lineHeight+(_composerPx(_composerStyle.paddingTop)||0)+(_composerPx(_composerStyle.paddingBottom)||0) + +(_composerPx(_composerStyle.borderTopWidth)||0)+(_composerPx(_composerStyle.borderBottomWidth)||0) + :NaN; + const _rowCeiling=Number.isFinite(_naturalRowHeight)&&Number.isFinite(_minHeight)?Math.max(_minHeight,_naturalRowHeight):_minHeight; + const _isAtMinimumHeight=Number.isFinite(_rowCeiling)&&el.offsetHeight<=Math.ceil(_rowCeiling)+1; if(_isAppendOnly&&_fitsCurrentHeight&&_isAtMinimumHeight){ _composerLastResizeValue=_nextValue; updateSendBtn(); diff --git a/tests/_composer_metrics.py b/tests/_composer_metrics.py new file mode 100644 index 00000000..44aef2d8 --- /dev/null +++ b/tests/_composer_metrics.py @@ -0,0 +1,210 @@ +"""Composer box metrics derived from the REAL stylesheet. + +``autoResize()`` in ``static/messages.js`` decides between skipping the height +round trip and doing a full remeasure from *computed* styles: the composer's +natural one-row border box is ``line-height + vertical padding + borders`` and it +is compared against the CSS ``min-height`` (see the guard discussed in +``docs/UIUX-GUIDE.md``, "Composer sizing"). + +Hardcoding those numbers in a test fixture is how the first #7604 fixture went +wrong: it used the 18px ``data-font-size="large"`` composer (29.7px line-height, +47.7px natural row) as if it were the stylesheet's 16px default. Parse +``static/style.css`` here instead and derive every dimension from it, so the +fixtures follow the stylesheet and cannot quietly encode the wrong font size. + +Usage:: + + from tests._composer_metrics import CONFIG_DEFAULT, CONFIG_LARGE, composer_metrics + + m = composer_metrics(CONFIG_LARGE) + m.natural_row # 47.7: what one row of text wants (border box) + m.offset_height # 48: what a browser reports (integer, min-height floored) + m.old_ceiling # ceil(min-height) + 1: the pre-#7604 skip ceiling + m.new_ceiling # ceil(max(min-height, natural row)) + 1: with the fix + m.computed # {"lineHeight": "29.7px", ...} for a getComputedStyle stub +""" +from __future__ import annotations + +import math +import re +from dataclasses import dataclass +from pathlib import Path + +STYLE_CSS = (Path(__file__).parents[1] / "static" / "style.css").read_text(encoding="utf-8") + + +def _declarations(rules: list[str]) -> dict[str, str]: + out: dict[str, str] = {} + for rule in rules: + for decl in rule.split(";"): + if ":" not in decl: + continue + prop, _, value = decl.partition(":") + out[prop.strip().lower()] = value.strip() + return out + + +def _rule(pattern: str, *, flags: int = re.M) -> str | None: + m = re.search(pattern, STYLE_CSS, flags) + return m.group(1) if m else None + + +def _px(value: str) -> float: + return float(value.replace("px", "").strip()) + + +def _padding_shorthand(value: str) -> tuple[float, float]: + """(padding-top, padding-bottom) from a CSS padding shorthand.""" + parts = value.split() + if len(parts) == 1: # all sides + return _px(parts[0]), _px(parts[0]) + if len(parts) == 2: # vertical horizontal + return _px(parts[0]), _px(parts[0]) + if len(parts) == 3: # top horizontal bottom + return _px(parts[0]), _px(parts[2]) + return _px(parts[0]), _px(parts[2]) + + +# The base composer rule: `textarea#msg{...}` (skin rules are prefixed with +# `:root[data-skin=...]`, so anchor the match at the start of a line). +_BASE = _declarations([_rule(r"^\s*textarea#msg\{([^}]*)\}") or ""]) +_BASE_FONT_SIZE = _px(_BASE["font-size"]) +_BASE_LINE_RATIO = float(_BASE["line-height"]) +_BASE_PAD_TOP, _BASE_PAD_BOTTOM = _padding_shorthand(_BASE["padding"]) +_MIN_HEIGHT = _px(_BASE["min-height"]) +# `border:none` / unset borders contribute nothing to the computed border box. +_BORDER = _px(_BASE.get("border-width", "0px")) if _BASE.get("border-width") else 0.0 + +# `:root[data-font-size="large"] #msg { font-size: 18px; }` overrides, i.e. the +# appearance setting a real user can switch on. +_FONT_SIZE_OVERRIDES = { + name: _px(size) + for name, size in re.findall( + r':root\[data-font-size="(\w+)"\]\s*#msg\s*\{\s*font-size:\s*([\d.]+px)', STYLE_CSS + ) +} + +# Skins that restyle the composer's font metrics, e.g. +# `:root[data-skin="graphite"] textarea#msg{...font-size:14px;...line-height:1.45;}`. +_SKIN_OVERRIDES = { + name: _declarations([body]) + for name, body in re.findall( + r':root\[data-skin="([\w-]+)"\]\s*textarea#msg\{([^}]*)\}', STYLE_CSS + ) +} + + +@dataclass(frozen=True) +class ComposerMetrics: + """One composer configuration, all values derived from ``static/style.css``.""" + + name: str + font_size: float + line_height: float + padding_top: float + padding_bottom: float + border_top: float + border_bottom: float + min_height: float + + @property + def natural_row(self) -> float: + """The border box one row of text wants (line-height + padding + borders).""" + return ( + self.line_height + + self.padding_top + + self.padding_bottom + + self.border_top + + self.border_bottom + ) + + @property + def offset_height(self) -> int: + """What a browser reports for a one-row composer: an integer, min-height floored.""" + return round(max(self.natural_row, self.min_height)) + + @property + def old_ceiling(self) -> int: + """The pre-#7604 skip ceiling: ``ceil(min-height) + 1`` (min-height only).""" + return math.ceil(self.min_height) + 1 + + @property + def new_ceiling(self) -> int: + """The skip ceiling with the natural one-row height considered.""" + return math.ceil(max(self.min_height, self.natural_row)) + 1 + + @property + def skip_was_reachable(self) -> bool: + return self.offset_height <= self.old_ceiling + + @property + def skip_is_reachable(self) -> bool: + return self.offset_height <= self.new_ceiling + + @property + def content_two_rows(self) -> int: + """scrollHeight for a two-row value in the same box.""" + return round(2 * self.line_height + self.padding_top + self.padding_bottom) + + @property + def oversized_box(self) -> int: + """A composer several rows tall that must still remeasure back down.""" + return round(self.natural_row + 3 * self.line_height) + + @property + def computed(self) -> dict[str, str]: + """The ``getComputedStyle`` payload the JS guard reads.""" + + def px(value: float) -> str: + return f"{value:g}px" + + return { + "minHeight": px(self.min_height), + "lineHeight": px(self.line_height), + "paddingTop": px(self.padding_top), + "paddingBottom": px(self.padding_bottom), + "borderTopWidth": px(self.border_top), + "borderBottomWidth": px(self.border_bottom), + "fontSize": px(self.font_size), + } + + +def composer_metrics( + font_size: float | None = None, + line_height_ratio: float | None = None, + *, + name: str = "custom", +) -> ComposerMetrics: + """Metrics for the stylesheet's composer at ``font_size`` (default: 16px).""" + size = _BASE_FONT_SIZE if font_size is None else font_size + ratio = _BASE_LINE_RATIO if line_height_ratio is None else line_height_ratio + return ComposerMetrics( + name=name, + font_size=size, + line_height=size * ratio, + padding_top=_BASE_PAD_TOP, + padding_bottom=_BASE_PAD_BOTTOM, + border_top=_BORDER, + border_bottom=_BORDER, + min_height=_MIN_HEIGHT, + ) + + +#: The stylesheet default (16px), i.e. `textarea#msg` with no appearance override. +CONFIG_DEFAULT = composer_metrics(name="default-16px") + +#: The `data-font-size` appearance overrides. +CONFIG_SMALL = composer_metrics(_FONT_SIZE_OVERRIDES.get("small"), name="small-14px") +CONFIG_LARGE = composer_metrics(_FONT_SIZE_OVERRIDES.get("large"), name="large-18px") +CONFIG_XLARGE = composer_metrics(_FONT_SIZE_OVERRIDES.get("xlarge"), name="xlarge-20px") + +#: A skin that restyles the composer metrics (14px / 1.45). +_SKIN = _SKIN_OVERRIDES.get("graphite", {}) +CONFIG_SKIN = composer_metrics( + _px(_SKIN["font-size"]) if "font-size" in _SKIN else None, + float(_SKIN["line-height"]) if "line-height" in _SKIN else None, + name="skin-graphite-14px", +) + +#: Every composer configuration the stylesheet can produce. +COMPOSER_CONFIGS = (CONFIG_DEFAULT, CONFIG_SMALL, CONFIG_LARGE, CONFIG_XLARGE, CONFIG_SKIN) diff --git a/tests/test_issue5514_composer_grow_scroll_pin.py b/tests/test_issue5514_composer_grow_scroll_pin.py index 6532f002..a90a5cf4 100644 --- a/tests/test_issue5514_composer_grow_scroll_pin.py +++ b/tests/test_issue5514_composer_grow_scroll_pin.py @@ -35,6 +35,8 @@ from pathlib import Path import pytest +from tests._composer_metrics import CONFIG_LARGE + ROOT = Path(__file__).parents[1] UI_JS = (ROOT / "static" / "ui.js").read_text(encoding="utf-8") MESSAGES_JS = (ROOT / "static" / "messages.js").read_text(encoding="utf-8") @@ -409,19 +411,88 @@ def test_resize_observer_installed_on_composer(): assert "can't strand" in BOOT_JS -def test_single_line_growth_skips_the_height_round_trip(): +def _run_composer_single_row_fixture(*, value: str, previous_value: str, + offset_height: int, content_height: int | None = None, + config=None): + """Run the REAL autoResize() body against a faithful textarea stub whose + ``getComputedStyle`` returns a real composer's computed styles, and report the + observable resize behaviour (height writes, settled height, repins). + + The configuration (and therefore every dimension) is derived from + ``static/style.css`` by ``tests/_composer_metrics.py`` instead of being typed + in here: the composer's box depends on the appearance font-size setting, and + hardcoding one configuration is exactly how the first revision of this test + encoded the 18px ``data-font-size=large`` composer as "stock". + + ``content_height`` is the height the content WANTS (default: one row); a real + textarea reports its box height in scrollHeight while the box is taller than + the content and the content height once the box collapses to ``height:'auto'``. + """ + node = shutil.which("node") + if not node: # pragma: no cover + pytest.skip("node not available") + config = CONFIG_LARGE if config is None else config + if content_height is None: + content_height = config.offset_height body = _autoresize_body() - assert "let _composerLastResizeValue='';" in MESSAGES_JS - assert "const _isAppendOnly=_nextValue.length>_composerLastResizeValue.length&&_nextValue.startsWith(_composerLastResizeValue);" in body - assert "const _fitsCurrentHeight=el.scrollHeight<=el.offsetHeight;" in body - assert "const _minHeightRaw=_isAppendOnly&&_fitsCurrentHeight?getComputedStyle(el).minHeight:'';" in body - assert "const _minHeight=/^(?:\\d+(?:\\.\\d+)?|\\.\\d+)px$/.test(_minHeightRaw)?parseFloat(_minHeightRaw):NaN;" in body - # The strict finite-pixel guard rejects a percentage/auto/calc min-height so a - # bogus parseFloat("50%")===50 can't wrongly enable the fast path (Codex #6349 re-gate). - assert "parseFloat(getComputedStyle(el).minHeight)" not in body # old lax parse is gone - assert "const _isAtMinimumHeight=Number.isFinite(_minHeight)&&el.offsetHeight<=Math.ceil(_minHeight)+1;" in body - assert "if(_isAppendOnly&&_fitsCurrentHeight&&_isAtMinimumHeight){" in body - assert "el.style.height='auto'" in body + harness = textwrap.dedent( + """ + let _composerAutoResizeRaf = 0; + let _composerLastResizeValue = %(previous_value)r; + let writes = 0, height = %(offset_height)s, repins = 0; + const NATURAL_ROW = %(natural_row)s; + const CONTENT_H = %(content_height)s; + const msg = { + value: %(value)r, + get offsetHeight() { return height; }, + get scrollHeight() { return height > NATURAL_ROW ? height : CONTENT_H; }, + style: { + set height(value) { writes += 1; height = value === 'auto' ? NATURAL_ROW : parseInt(value, 10); }, + get height() { return height + 'px'; }, + }, + }; + const messages = { scrollTop: 0 }; + const $ = (id) => id === 'msg' ? msg : id === 'messages' ? messages : null; + const COMPUTED = %(computed)s; + function getComputedStyle() { return COMPUTED; } + function updateSendBtn() {} + function _repinMessagesAfterComposerResize() { repins += 1; } + %(autoresize)s + autoResize(); + console.log(JSON.stringify({ writes, height, lastValue: _composerLastResizeValue, repins })); + """ + ) % { + "previous_value": previous_value, + "offset_height": offset_height, + "content_height": content_height, + "natural_row": config.offset_height, + "value": value, + "computed": json.dumps(config.computed), + "autoresize": body, + } + proc = subprocess.run([node, "-e", harness], capture_output=True, text=True, timeout=30) + assert proc.returncode == 0, f"node harness failed: {proc.stderr}" + return json.loads(proc.stdout) + + +def test_single_line_growth_skips_the_height_round_trip(): + # The one-row fast path must be REACHABLE. It gates on el.offsetHeight against + # the min-height ceiling, and the composer's natural one-row height + # (line-height + vertical padding + borders) is 47.7px in the reported + # configuration (the 18px `data-font-size=large` composer) against a pre-fix + # ceiling of ceil(44px)+1 = 45px - so every append at rest missed the skip and + # paid the height:'auto' round trip, whose scrollHeight read forces a + # synchronous full-document reflow (typing lag that grows with the rendered + # transcript). Behaviour, not source: the fixture runs the real autoResize(). + one_row = CONFIG_LARGE.offset_height + out = _run_composer_single_row_fixture(value="a", previous_value="", offset_height=one_row) + assert out == {"writes": 0, "height": one_row, "lastValue": "a", "repins": 0}, out + # ...and the widened ceiling stays BOUNDED: an oversized composer is far above + # one row, so it still remeasures back down (the #5514 shrink-back invariant). + oversized = _run_composer_single_row_fixture( + value="a", previous_value="", offset_height=CONFIG_LARGE.oversized_box + ) + assert oversized == {"writes": 2, "height": one_row, "lastValue": "a", "repins": 0}, oversized # --------------------------------------------------------------------------- def _run(scenario): diff --git a/tests/test_long_session_composer_typing_latency.py b/tests/test_long_session_composer_typing_latency.py new file mode 100644 index 00000000..7be3a81d --- /dev/null +++ b/tests/test_long_session_composer_typing_latency.py @@ -0,0 +1,262 @@ +"""Regression coverage for composer typing lag in long chat sessions. + +Symptom: in a long session, typing becomes laggy - keystrokes echo with a +visible delay. Measured on a 758-message / 5 MB session (31k DOM nodes +rendered), with the browser's own Event Timing API (input -> next paint) at 6x +CPU throttle: + + transcript rendered keystroke -> echo (median) + full (36 rows) @6x 168ms -> 136ms with this fix + windowed (11 rows) @6x 96ms -> 72ms with this fix + floor (transcript detached) 72ms + +Root cause: ``autoResize()`` in ``static/messages.js`` has a single-row fast path +that must skip the ``height:'auto'`` -> read ``scrollHeight`` -> restore round +trip. That round trip forces a SYNCHRONOUS layout of the whole document (the +transcript included), so its cost grows with the rendered transcript - exactly +the reported symptom. The guard that gates the skip compared ``el.offsetHeight`` +against the CSS ``min-height`` (44px -> a ceiling of ``ceil(44)+1 = 45px``) +instead of the composer's natural ONE-ROW height - ``line-height + vertical +padding + borders``. + +Which configurations that broke is a property of the stylesheet, so the numbers +below are DERIVED from ``static/style.css`` by ``tests/_composer_metrics.py`` +rather than typed into a fixture (an earlier revision of this module hardcoded +the 18px numbers and mislabelled them as the stock font size): + + composer config one-row box pre-fix ceiling skip fired? + default 16px 44px 45px yes (0.6px margin) + data-font-size=large (18px) 48px 45px NO - dead code + data-font-size=xlarge (20px) 51px 45px NO - dead code + skin graphite (14px/1.45) 44px 45px yes + +So the fix is font-size independent: it derives the skip ceiling from the same +computed styles the guard already reads, and therefore also covers the smaller +configurations (which only cleared the old ceiling by 0.6px, or purely because +``min-height`` floored the box). + +Fix: accept the natural one-row height (computed from line-height + padding + +borders) as the skip ceiling, failing closed to the old min-height-only +behaviour whenever those computed values are not strict px values (so a +percentage / ``normal`` line-height cannot wrongly enable the skip). + +These tests exercise the REAL ``autoResize()`` body extracted from messages.js in +a node sandbox whose ``getComputedStyle`` returns the requested composer's real +computed styles. ``test_skip_is_reachable_in_every_composer_configuration`` is +red on the pre-fix tree for the 18px/20px configurations and green after it. +""" +import json +import shutil +import subprocess +import textwrap +from pathlib import Path + +import pytest + +from tests._composer_metrics import ( + COMPOSER_CONFIGS, + CONFIG_DEFAULT, + CONFIG_LARGE, + CONFIG_SKIN, + CONFIG_SMALL, + CONFIG_XLARGE, +) + +ROOT = Path(__file__).parents[1] +MESSAGES_JS = (ROOT / "static" / "messages.js").read_text(encoding="utf-8") + + +def _autoresize_body() -> str: + start = MESSAGES_JS.find("function autoResize(") + assert start != -1 + end = MESSAGES_JS.find("function scheduleComposerAutoResize(", start) + assert end > start + return MESSAGES_JS[start:end] + + +def _run_autoresize(config, *, value: str, previous_value: str, box_height: int, + content_height: int, computed=None): + """Run the real autoResize() against a faithful textarea stub. + + ``box_height`` is the composer's current offsetHeight and ``content_height`` + the height its content WANTS (one row = the config's one-row box, two rows = + ``config.content_two_rows``); the stub models a real textarea, whose + scrollHeight is the box height while the box is taller than the content and + the content height once the box collapses to ``height:'auto'``. + """ + node = shutil.which("node") + assert node, "node is required for the autoResize harness" + body = _autoresize_body() + harness = textwrap.dedent( + """ + let _composerAutoResizeRaf = 0; + let _composerLastResizeValue = %(previous_value)r; + let writes = 0, height = %(box_height)s; + const NATURAL_ROW = %(natural_row)s; + const CONTENT_H = %(content_height)s; + const msg = { + value: %(value)r, + get offsetHeight() { return height; }, + get scrollHeight() { return height > NATURAL_ROW ? height : CONTENT_H; }, + style: { + set height(v) { writes += 1; height = v === 'auto' ? NATURAL_ROW : parseInt(v, 10); }, + get height() { return height + 'px'; }, + }, + }; + const messages = { scrollTop: 0 }; + const $ = (id) => id === 'msg' ? msg : id === 'messages' ? messages : null; + const COMPUTED = %(computed)s; + function getComputedStyle() { return COMPUTED; } + let sendUpdates = 0; + function updateSendBtn() { sendUpdates += 1; } + function _repinMessagesAfterComposerResize() {} + %(autoresize)s + autoResize(); + console.log(JSON.stringify({ writes, height, lastValue: _composerLastResizeValue, sendUpdates })); + """ + ) % { + "previous_value": previous_value, + "value": value, + "box_height": box_height, + "content_height": content_height, + "natural_row": config.offset_height, + "computed": json.dumps(config.computed if computed is None else computed), + "autoresize": body, + } + proc = subprocess.run([node, "-e", harness], capture_output=True, text=True, timeout=30) + assert proc.returncode == 0, proc.stderr + return json.loads(proc.stdout) + + +@pytest.mark.parametrize("config", COMPOSER_CONFIGS, ids=lambda c: c.name) +def test_skip_is_reachable_in_every_composer_configuration(config): + """THE typing-lag regression: at its natural one-row height an append + keystroke must NOT run the height round trip, in every configuration the + stylesheet can produce. + + Pre-fix the one-row box was compared against the 44px CSS min-height (a + 45px ceiling). That still cleared the 16px default (44px) by 0.6px, but the + 18px/20px ``data-font-size`` composers sit at 48px/51px, where the skip was + dead code and every keystroke forced a synchronous full-document reflow. + """ + out = _run_autoresize( + config, value="a", previous_value="", box_height=config.offset_height, + content_height=config.offset_height, + ) + assert out["writes"] == 0, f"one-row append must skip the resize round trip; got {out}" + assert out["height"] == config.offset_height, out + assert out["lastValue"] == "a", out + assert out["sendUpdates"] == 1, "the primary-button refresh must still run" + + +def test_the_regression_was_font_size_dependent_not_stock(): + """Grounding for this fix, straight from the stylesheet. + + This is the check the first revision of this module failed: it asserted a + 48px "stock" composer, which is the 18px ``data-font-size=large`` box. The + real default (16px) already reached the old ceiling, so the fix is about + making the skip font-size independent - not about a default the reporter + never ran. + """ + assert CONFIG_DEFAULT.offset_height <= CONFIG_DEFAULT.old_ceiling, ( + "the stylesheet default is expected to have reached the pre-fix ceiling" + ) + assert CONFIG_LARGE.offset_height > CONFIG_LARGE.old_ceiling + assert CONFIG_XLARGE.offset_height > CONFIG_XLARGE.old_ceiling + # ...and after the fix every configuration's one-row box clears the ceiling, + # including the two that were dead and the two that were already fine. + for config in COMPOSER_CONFIGS: + assert config.skip_is_reachable, f"{config.name} must reach the skip after the fix" + assert CONFIG_SMALL.offset_height == CONFIG_SMALL.min_height, ( + "the small configuration's box is floored by min-height, not by its text" + ) + assert CONFIG_SKIN.offset_height == CONFIG_SKIN.min_height + + +def test_typed_word_appends_skip_in_the_reported_configuration(): + """The reported lag configuration (18px composer) must skip on EVERY append + keystroke, not just the first: the real typing loop is '' -> 'h' -> 'he' ... + """ + previous = "" + for letter in "hello": + value = previous + letter + out = _run_autoresize( + CONFIG_LARGE, value=value, previous_value=previous, + box_height=CONFIG_LARGE.offset_height, + content_height=CONFIG_LARGE.offset_height, + ) + assert out["writes"] == 0, f"append {value!r} must skip; got {out}" + assert out["lastValue"] == value, out + previous = value + + +def test_oversized_composer_still_remeasures_to_its_natural_height(): + """The skip must not preserve an oversized composer (the #5514 invariant). + + A composer several rows tall holding a one-line value remeasures down to one + row: its offsetHeight is far above the natural one-row ceiling. + """ + out = _run_autoresize( + CONFIG_LARGE, value="short prefix", previous_value="short prefi", + box_height=CONFIG_LARGE.oversized_box, content_height=CONFIG_LARGE.offset_height, + ) + assert out["writes"] == 2, f"an oversized composer must remeasure; got {out}" + assert out["height"] == CONFIG_LARGE.offset_height, out + + +def test_single_line_delete_still_runs_the_height_round_trip(): + """Shrinking values are not append-only and must remeasure.""" + out = _run_autoresize( + CONFIG_LARGE, value="a", previous_value="hello", + box_height=CONFIG_LARGE.oversized_box, content_height=CONFIG_LARGE.offset_height, + ) + assert out["writes"] == 2, f"a delete must remeasure; got {out}" + assert out["height"] == CONFIG_LARGE.offset_height, out + + +def test_multi_line_append_still_runs_the_height_round_trip(): + """A newline append is not a one-row append: it must measure and grow.""" + out = _run_autoresize( + CONFIG_LARGE, value="a\nb", previous_value="a", + box_height=CONFIG_LARGE.offset_height, + content_height=CONFIG_LARGE.content_two_rows, + ) + assert out["writes"] == 2, f"newline growth must remeasure; got {out}" + assert out["height"] == CONFIG_LARGE.content_two_rows, out + + +def test_non_px_computed_values_fail_closed_to_min_height_only(): + """Without strict px padding/line-height values the skip falls back to the + pre-fix min-height-only semantics, i.e. the 18px one-row box remeasures.""" + out = _run_autoresize( + CONFIG_LARGE, value="a", previous_value="", + box_height=CONFIG_LARGE.offset_height, + content_height=CONFIG_LARGE.offset_height, + computed={"minHeight": "44px"}, + ) + assert out["writes"] == 2, f"must fail closed to the full resize; got {out}" + + +def test_percentage_line_height_fails_closed(): + """A percentage line-height is not a strict px value and must not enable the + skip (mirrors the #6349 min-height gate).""" + out = _run_autoresize( + CONFIG_LARGE, value="a", previous_value="", + box_height=CONFIG_LARGE.offset_height, + content_height=CONFIG_LARGE.offset_height, + computed={**CONFIG_LARGE.computed, "lineHeight": "165%"}, + ) + assert out["writes"] == 2, f"percentage line-height must fail closed; got {out}" + + +def test_percentage_min_height_fails_closed(): + """A percentage min-height must not be read as a pixel number by + ``parseFloat`` (``parseFloat('50%') === 50``) - the #6349 property, still in + force after this change: no strict px min-height means no reachable ceiling.""" + out = _run_autoresize( + CONFIG_DEFAULT, value="a", previous_value="", + box_height=CONFIG_DEFAULT.offset_height, + content_height=CONFIG_DEFAULT.offset_height, + computed={**CONFIG_DEFAULT.computed, "minHeight": "50%"}, + ) + assert out["writes"] == 2, f"percentage min-height must fail closed; got {out}"