stage: merge #5635 (7811b13f49)

static/ui.js

@@ -839,11 +839,17 @@ function _captureMessageViewportAnchor(){
     const rect=row.getBoundingClientRect();
     if(rect.bottom>containerRect.top+1){
       const sessionIdx=Number(row&&row.dataset&&row.dataset.sessionMsgIdx);
+      // Record the current top-spacer (virtual topPad) height so the compensation
+      // path can fall back to a topPad-delta shift when the anchor row itself is
+      // recycled out of the render window after a measurement-driven re-render.
+      const spacer=container.querySelector('[data-virtual-spacer="before"]');
+      const topPadBefore=spacer?parseFloat(spacer.style.height||'0')||0:0;
       return {
         rawIdx,
         sessionIdx:Number.isFinite(sessionIdx)?sessionIdx:_messageSessionIndexForRawIdx(rawIdx),
         key:row&&row.dataset?String(row.dataset.messageAnchorKey||''):'',
         topOffset:rect.top-containerRect.top,
+        topPadBefore,
       };
     }
   }
@@ -994,8 +1000,43 @@ function _compensateScrollForMeasurementDelta(renderFn){
     const spacer=container.querySelector('[data-virtual-spacer="before"]');
     if(!spacer||parseFloat(spacer.style.height||'0')<=0) return;
   }
-  const row=container.querySelector(`[data-msg-idx="${anchorBefore.rawIdx}"]`);
-  if(!row) return;
+  // Re-find the anchor row after the measurement-driven re-render. The primary
+  // lookup is by rawIdx (the DOM index), but on a big virtualized session a large
+  // scroll delta can RECYCLE the old anchor row out of the render window entirely
+  // (verified via real-device telemetry: DOM collapsed to 1 row, scrollHeight
+  // lurched by tens of thousands of px). The old code did `if(!row) return` here,
+  // abandoning compensation → the full estimated↔measured height lurch hit
+  // scrollTop uncompensated and threw the viewport to the top (the recurring
+  // mobile scroll jump-back). Fall back to the stable sessionIdx anchor (captured in
+  // _captureMessageViewportAnchor) before giving up, mirroring the "recover via
+  // sessionIdx when the primary anchor key is gone" approach used elsewhere but for
+  // the virtualization-measurement compensation path.
+  let row=container.querySelector(`[data-msg-idx="${anchorBefore.rawIdx}"]`);
+  if(!row&&Number.isFinite(Number(anchorBefore.sessionIdx))){
+    row=container.querySelector(`[data-session-msg-idx="${anchorBefore.sessionIdx}"]`);
+  }
+  if(!row){
+    // Anchor row is no longer rendered (recycled out of the virtual window). We
+    // cannot measure its live offset, but we CAN keep the viewport visually
+    // stable by compensating for the top-spacer (topPad) height change: the
+    // whole reason scrollHeight lurched is that the estimated topPad was replaced
+    // by a measured one. Shift scrollTop by that same delta so content under the
+    // viewport does not appear to jump. Without this the browser lands at an
+    // uncompensated absolute scrollTop against a wildly different scrollHeight.
+    const spacerAfter=container.querySelector('[data-virtual-spacer="before"]');
+    const topPadAfter=spacerAfter?parseFloat(spacerAfter.style.height||'0')||0:0;
+    const topPadBefore=Number(anchorBefore.topPadBefore);
+    if(Number.isFinite(topPadBefore)){
+      const padDelta=topPadAfter-topPadBefore;
+      if(Math.abs(padDelta)>=2){
+        _programmaticScroll=true;_programmaticScrollSetAt=performance.now();
+        container.scrollTop=Math.max(0,scrollTopBefore+padDelta);
+        _lastScrollTop=container.scrollTop;
+        _deferClearProgrammaticScroll();
+      }
+    }
+    return;
+  }
   const containerRect=container.getBoundingClientRect();
   const rowRect=row.getBoundingClientRect();
   const actualOffset=rowRect.top-containerRect.top;

tests/test_issue4346_vscroll_recycled_anchor_jumpback.py

@@ -0,0 +1,281 @@
+"""Regression tests for the virtualization scroll-compensation jump-back.
+
+Root cause: `_compensateScrollForMeasurementDelta` re-found its viewport anchor
+ONLY by rawIdx and bailed with a bare `if(!row) return` when that row was
+recycled out of the render window. On big sessions containing multi-thousand-px
+turns, a large scroll delta recycles the anchor row, so the estimated->measured
+topPad swap (a scrollHeight change of tens of thousands of px) hit scrollTop
+uncompensated and threw the viewport to the top -- the recurring mobile scroll
+jump-back.
+
+Fix: when the rawIdx row is gone, fall back to (a) the stable sessionIdx anchor,
+and (b) if that is also unrendered, compensate by the top-spacer (topPad) height
+delta captured before the re-render.
+
+Every behavioral test below is designed to FAIL on the known-buggy version
+(bare `if(!row) return`) and PASS only on the fixed version.
+"""
+import json
+import pathlib
+import shutil
+import subprocess
+import tempfile
+
+import pytest
+
+ROOT = pathlib.Path(__file__).parent.parent
+UI_JS_PATH = ROOT / "static" / "ui.js"
+NODE = shutil.which("node")
+
+pytestmark = pytest.mark.skipif(NODE is None, reason="node not on PATH")
+
+
+def _run_node(source: str) -> str:
+    with tempfile.NamedTemporaryFile(
+        "w", suffix=".cjs", encoding="utf-8", dir=ROOT, delete=False
+    ) as script:
+        script.write(source)
+        script_path = pathlib.Path(script.name)
+    try:
+        result = subprocess.run(
+            [NODE, str(script_path)],
+            cwd=str(ROOT),
+            capture_output=True,
+            text=True,
+            timeout=30,
+        )
+    finally:
+        script_path.unlink(missing_ok=True)
+    if result.returncode != 0:
+        raise RuntimeError(result.stderr)
+    return result.stdout.strip()
+
+
+def _extract_func_script(js: str) -> str:
+    # extractFunc brace-matches the function body but SKIPS braces that live
+    # inside string / template / regex literals and comments, so a future edit
+    # adding e.g. `warn('expected {k}')` inside an extracted function cannot
+    # desync the depth counter (greptile P2 on this PR). Built with a plain
+    # string (not an f-string) so the JS braces need no doubling.
+    prelude = "const src = " + json.dumps(js) + ";\n"
+    body = r"""
+function extractFunc(name) {
+  const re = new RegExp('function\\s+' + name + '\\s*\\(');
+  const start = src.search(re);
+  if (start < 0) throw new Error(name + ' not found');
+  let i = src.indexOf('{', start);
+  let depth = 1; i++;
+  let str = null;      // current string/template delimiter, or null
+  let inLine = false;  // inside // line comment
+  let inBlock = false; // inside /* block comment */
+  let inRegex = false; // inside / regex literal /
+  let prev = '';       // last significant (non-space) code char, for regex detection
+  while (depth > 0 && i < src.length) {
+    const c = src[i];
+    const n = src[i + 1];
+    if (inLine) { if (c === '\n') inLine = false; i++; continue; }
+    if (inBlock) { if (c === '*' && n === '/') { inBlock = false; i++; } i++; continue; }
+    if (str) {
+      if (c === '\\') { i += 2; continue; }
+      if (c === str) str = null;
+      i++; continue;
+    }
+    if (inRegex) {
+      if (c === '\\') { i += 2; continue; }
+      if (c === '/') inRegex = false;
+      i++; continue;
+    }
+    if (c === '/' && n === '/') { inLine = true; i += 2; continue; }
+    if (c === '/' && n === '*') { inBlock = true; i += 2; continue; }
+    if (c === '"' || c === "'" || c === '`') { str = c; i++; continue; }
+    // A '/' starts a regex only where a value is expected, i.e. after an
+    // operator/paren/comma — not after an identifier/number/closing paren.
+    if (c === '/' && !'})]0123456789'.includes(prev) && !/[A-Za-z_$]/.test(prev)) {
+      inRegex = true; i++; continue;
+    }
+    if (c === '{') depth++;
+    else if (c === '}') depth--;
+    if (c.trim()) prev = c;
+    i++;
+  }
+  return src.slice(start, i);
+}"""
+    return prelude + body
+
+
+def test_compensate_recovers_via_session_idx_when_rawidx_row_recycled():
+    """When the rawIdx anchor row is recycled out of the render window but the
+    SAME row is still locatable by its stable data-session-msg-idx, the
+    compensation must recover via the sessionIdx lookup and shift scrollTop by
+    the measured delta -- NOT bail out (the buggy `if(!row) return`)."""
+    js = UI_JS_PATH.read_text(encoding="utf-8")
+    source = _extract_func_script(js) + """
+let scrollTopValue = 20000;
+let scrollHistory = [];
+const container = {
+  get scrollTop(){ return scrollTopValue; },
+  set scrollTop(v){ scrollHistory.push(v); scrollTopValue = v; },
+  getBoundingClientRect(){ return {top: 0, bottom: 600}; },
+  classList: { add(){}, remove(){} },
+  querySelector(selector){
+    // rawIdx row is RECYCLED OUT (gone); the sessionIdx row IS still rendered.
+    if(selector.indexOf('[data-msg-idx="42"]') !== -1) return null;
+    if(selector.indexOf('[data-session-msg-idx="42"]') !== -1){
+      return { getBoundingClientRect(){ return {top: 150}; } };
+    }
+    if(selector.indexOf('virtual-spacer') !== -1) return {style:{height:'1000px'}};
+    return null;
+  },
+};
+function $(id){ return id === 'messages' ? container : null; }
+function _captureMessageViewportAnchor(){
+  return {rawIdx: 42, sessionIdx: 42, topOffset: 100, topPadBefore: 1000};
+}
+let _programmaticScroll = false;
+let _programmaticScrollSetAt = 0;
+let _programmaticScrollResetTimer = 0;
+let _lastScrollTop = 0;
+const performance = { now(){ return 1000; } };
+function clearTimeout(){}
+function setTimeout(cb){ cb(); return 1; }
+function _deferClearProgrammaticScroll(){}
+function requestAnimationFrame(cb){ cb(); }
+eval(extractFunc('_compensateScrollForMeasurementDelta'));
+_compensateScrollForMeasurementDelta(()=>{});
+console.log(JSON.stringify({scrollHistory}));
+"""
+    metrics = json.loads(_run_node(source))
+    # sessionIdx row found at top=150; anchor.topOffset=100 -> delta = 150-100 = 50
+    # scrollTop shifts 20000 + 50 = 20050 (recovered, NOT abandoned).
+    assert metrics["scrollHistory"] == [20050], (
+        "compensation must recover via data-session-msg-idx when the rawIdx row "
+        "is recycled out (buggy code bailed with `if(!row) return`, leaving "
+        "scrollHistory empty)"
+    )
+
+
+def test_compensate_uses_toppad_delta_when_anchor_row_fully_recycled():
+    """When BOTH the rawIdx and sessionIdx rows are recycled out (large scroll
+    delta on a big virtualized session), the compensation must fall back to the
+    top-spacer (topPad) height delta so the huge estimated->measured scrollHeight
+    lurch does not throw the viewport to the top. Buggy code abandoned entirely
+    and left scrollTop uncompensated."""
+    js = UI_JS_PATH.read_text(encoding="utf-8")
+    source = _extract_func_script(js) + """
+let scrollTopValue = 20500;
+let scrollHistory = [];
+// topPad shrank from 20000 (estimated) to 1000 (measured): a -19000px lurch.
+const container = {
+  get scrollTop(){ return scrollTopValue; },
+  set scrollTop(v){ scrollHistory.push(v); scrollTopValue = v; },
+  getBoundingClientRect(){ return {top: 0, bottom: 600}; },
+  classList: { add(){}, remove(){} },
+  querySelector(selector){
+    // both anchor lookups miss (row recycled out entirely)
+    if(selector.indexOf('data-msg-idx') !== -1) return null;
+    if(selector.indexOf('data-session-msg-idx') !== -1) return null;
+    // the top spacer now measures 1000px (was 20000 at capture time)
+    if(selector.indexOf('virtual-spacer') !== -1) return {style:{height:'1000px'}};
+    return null;
+  },
+};
+function $(id){ return id === 'messages' ? container : null; }
+function _captureMessageViewportAnchor(){
+  return {rawIdx: 42, sessionIdx: 42, topOffset: 100, topPadBefore: 20000};
+}
+let _programmaticScroll = false;
+let _programmaticScrollSetAt = 0;
+let _lastScrollTop = 0;
+const performance = { now(){ return 1000; } };
+function clearTimeout(){}
+function setTimeout(cb){ cb(); return 1; }
+function _deferClearProgrammaticScroll(){}
+function requestAnimationFrame(cb){ cb(); }
+eval(extractFunc('_compensateScrollForMeasurementDelta'));
+_compensateScrollForMeasurementDelta(()=>{});
+console.log(JSON.stringify({scrollHistory}));
+"""
+    metrics = json.loads(_run_node(source))
+    # padDelta = topPadAfter(1000) - topPadBefore(20000) = -19000
+    # scrollTop = max(0, 20500 + (-19000)) = 1500 -- compensated for the lurch.
+    assert metrics["scrollHistory"] == [1500], (
+        "compensation must shift scrollTop by the topPad delta (-19000) when the "
+        "anchor row is fully recycled; buggy code left scrollTop uncompensated "
+        "(empty scrollHistory) and the viewport was thrown to the top"
+    )
+
+
+def test_compensate_still_bails_cleanly_when_no_toppad_before_captured():
+    """Defensive: if the captured anchor has no topPadBefore (older shape) AND
+    the row is gone, the fallback must NOT throw and must NOT mutate scrollTop."""
+    js = UI_JS_PATH.read_text(encoding="utf-8")
+    source = _extract_func_script(js) + """
+let scrollTopValue = 500;
+let scrollTopMutated = false;
+const container = {
+  get scrollTop(){ return scrollTopValue; },
+  set scrollTop(v){ scrollTopMutated = true; scrollTopValue = v; },
+  getBoundingClientRect(){ return {top: 0, bottom: 600}; },
+  classList: { add(){}, remove(){} },
+  querySelector(selector){
+    if(selector.indexOf('virtual-spacer') !== -1) return {style:{height:'1000px'}};
+    return null;  // no anchor row by any lookup
+  },
+};
+function $(id){ return id === 'messages' ? container : null; }
+function _captureMessageViewportAnchor(){
+  // no topPadBefore field (older capture shape) -> fallback must no-op safely
+  return {rawIdx: 42, sessionIdx: 42, topOffset: 100};
+}
+let _programmaticScroll = false;
+let _lastScrollTop = 0;
+const performance = { now(){ return 1000; } };
+function clearTimeout(){}
+function setTimeout(cb){ cb(); return 1; }
+function _deferClearProgrammaticScroll(){}
+function requestAnimationFrame(cb){ cb(); }
+eval(extractFunc('_compensateScrollForMeasurementDelta'));
+_compensateScrollForMeasurementDelta(()=>{});
+console.log(JSON.stringify({scrollTopMutated}));
+"""
+    metrics = json.loads(_run_node(source))
+    assert metrics["scrollTopMutated"] is False, (
+        "with no topPadBefore captured and no anchor row, the fallback must "
+        "leave scrollTop untouched (NaN-guarded), not throw or write garbage"
+    )
+
+
+def test_extract_func_skips_braces_inside_string_and_regex_literals():
+    """The Node-harness extractFunc must brace-match on real code structure only,
+    skipping braces inside string / template / regex literals and comments. A
+    naive depth counter desyncs on a bare '{' or '}' inside a string literal and
+    truncates the extract (greptile P2). This locks the robust behavior."""
+    tricky = (
+        "function _tricky(){\n"
+        "  const a = 'has a bare } brace';\n"
+        "  const b = \"and an open { one\";\n"
+        "  const c = `template ${'x'} literal`;\n"
+        "  const re = /\\}[{]/;  // regex with unbalanced-looking braces\n"
+        "  // a line comment with } and {\n"
+        "  /* block comment with { and } */\n"
+        "  return a.length + b.length + c.length + (re ? 1 : 0);\n"
+        "}\n"
+    )
+    source = _extract_func_script(tricky) + """
+const extracted = extractFunc('_tricky');
+// The extract must end at the REAL closing brace (full function), not a premature
+// one desynced by a string/regex brace. Eval it and call it to prove it is whole.
+eval(extracted);
+console.log(JSON.stringify({
+  endsAtRealBrace: extracted.trimEnd().endsWith('}'),
+  hasReturn: extracted.indexOf('return a.length') !== -1,
+  callable: typeof _tricky === 'function',
+  result: _tricky(),
+}));
+"""
+    metrics = json.loads(_run_node(source))
+    assert metrics["hasReturn"] is True, "extract truncated before the return statement"
+    assert metrics["endsAtRealBrace"] is True
+    assert metrics["callable"] is True
+    # 'has a bare } brace'(18) + 'and an open { one'(17) + 'template x literal'(18) + 1
+    assert metrics["result"] == 54