fix(perf): per-burst retry budget lifecycle, reset only on external render (#6717)

static/ui.js

@@ -548,6 +548,7 @@ let _messageVirtualScrollRaf=0;
 let _messageVirtualWindowKey='';
 let _messageVirtualMeasurementCycleKey='';
 let _messageVirtualMeasurementRetryCount=0;
+let _messageVirtualMeasurementBurstActive=false;
 let _messageVirtualScrollActive=false;
 let _messageVirtualScrollSettleTimer=0;
 let _messageVirtualDeferredMeasurement=null;
@@ -595,6 +596,7 @@ function _clearMessageVirtualHeightCache(){
   _messageVirtualWindowKey='';
   _messageVirtualMeasurementCycleKey='';
   _messageVirtualMeasurementRetryCount=0;
+  _messageVirtualMeasurementBurstActive=false;
   _messageVirtualScrollActive=false;
   clearTimeout(_messageVirtualScrollSettleTimer);
   _messageVirtualScrollSettleTimer=0;
@@ -728,13 +730,32 @@ function _scheduleMessageVirtualMeasurementRefresh(windowMetrics){
   if(_messageVirtualMeasurementCycleKey!==cycleKey){
     _messageVirtualMeasurementCycleKey=cycleKey;
   }
-  if(_messageVirtualMeasurementRetryCount>=MESSAGE_VIRTUAL_MEASUREMENT_MAX_RERENDERS) return;
+  // Per-burst retry budget (#6717): the counter is preserved across ONLY the
+  // internally measurement-scheduled chain (requestAnimationFrame ->
+  // _scheduleMessageVirtualizedRender -> renderMessages -> re-measure -> here),
+  // so WebKit's A/B window-metric oscillation can never renew the budget and
+  // keep the rAF/measure loop alive (#6654). It is reset exactly once when a
+  // later EXTERNALLY initiated render begins a genuinely new measurement cycle
+  // (session load, message append, real content change) — never on cycle-key
+  // changes. A cycle-key change alone just records the new key above.
+  if(!_messageVirtualMeasurementBurstActive){
+    _messageVirtualMeasurementRetryCount=0;
+    _messageVirtualMeasurementBurstActive=true;
+  }
+  if(_messageVirtualMeasurementRetryCount>=MESSAGE_VIRTUAL_MEASUREMENT_MAX_RERENDERS){
+    // Budget exhausted: this burst is over (no further internal re-render is
+    // scheduled), so the next externally initiated cycle starts fresh instead
+    // of being starved of its own retries.
+    _messageVirtualMeasurementBurstActive=false;
+    return;
+  }
   _messageVirtualMeasurementRetryCount++;
   requestAnimationFrame(()=>{ _scheduleMessageVirtualizedRender(true); });
 }
 function _markMessageVirtualMeasurementsSettled(windowMetrics){
   _messageVirtualMeasurementCycleKey=_messageVirtualMeasurementCycleKeyFor(windowMetrics);
   _messageVirtualMeasurementRetryCount=0;
+  _messageVirtualMeasurementBurstActive=false;
 }
 function _messageVirtualHeightEntryMatches(previousEntry, nextEntry){
   return !!(

tests/test_issue500_message_list_virtualization.py

@@ -1112,6 +1112,7 @@ let _messageVirtualEstimatedRowHeight = 200;
 let _messageVirtualWindowKey = 'old-key';
 let _messageVirtualMeasurementCycleKey = 'old-cycle';
 let _messageVirtualMeasurementRetryCount = 3;
+let _messageVirtualMeasurementBurstActive = false;
 const MESSAGE_VIRTUAL_DEFAULT_ROW_HEIGHT = 140;
 // #4367 introduced per-role default heights; the combined _clearMessageVirtualHeightCache
 // resets the estimate via _messageVirtualDefaultHeightForRole('default'), so the harness
@@ -1146,81 +1147,116 @@ console.log(JSON.stringify({
 def test_measurement_retry_budget_is_global_across_cycle_key_oscillation():
     """#6654: _scheduleMessageVirtualMeasurementRefresh must NOT reset
     _messageVirtualMeasurementRetryCount when _messageVirtualMeasurementCycleKey
-    changes. If WebKit alternates between two measured window-metric sets
-    (A -> B -> A -> B), each key transition used to reset the two-render budget,
-    so the rAF/geometry-measurement loop never terminated (~30% WebContent CPU).
-    The MESSAGE_VIRTUAL_MEASUREMENT_MAX_RERENDERS budget must cap the whole
-    convergence burst globally; it resets only when measurement settles
-    (_markMessageVirtualMeasurementsSettled) or the cache/session resets
-    (_clearMessageVirtualHeightCache)."""
+    changes INSIDE one measurement burst. If WebKit alternates between two
+    measured window-metric sets (A -> B -> A -> B), each key transition used to
+    reset the two-render budget, so the rAF/geometry-measurement loop never
+    terminated (~30% WebContent CPU). The MESSAGE_VIRTUAL_MEASUREMENT_MAX_RERENDERS
+    budget must cap the whole convergence burst; the burst ends at the cap (or
+    when measurement settles), so the NEXT externally initiated cycle gets a
+    fresh budget. The harness drives the thrash causally through the internal
+    chain (each scheduled render re-measures the next oscillating key)."""
     js = UI_JS_PATH.read_text(encoding="utf-8")
     source = _extract_func_script(js) + """
 let scheduled = 0;
 let _messageVirtualMeasurementCycleKey = '';
 let _messageVirtualMeasurementRetryCount = 0;
+let _messageVirtualMeasurementBurstActive = false;
 let _messageVirtualScrollActive = false;
 let _messageVirtualDeferredMeasurement = null;
 const MESSAGE_VIRTUAL_MEASUREMENT_MAX_RERENDERS = 2;
 function _messageVirtualMeasurementCycleKeyFor(w) { return w.key; }
-function _scheduleMessageVirtualizedRender(force) { scheduled++; }
-function requestAnimationFrame(cb) { cb(); }
-eval(extractFunc('_scheduleMessageVirtualMeasurementRefresh'));
-// WebKit alternates between two adjacent window-metric keys across
-// getBoundingClientRect passes. Before the fix every key transition reset the
-// budget, so all four passes scheduled a re-render.
-for (const key of ['A','B','A','B']) {
-  _scheduleMessageVirtualMeasurementRefresh({ key: key });
+// Drive the burst causally: each internally scheduled render re-measures and
+// re-enters the scheduler with the next WebKit-oscillated key.
+let keys = ['B','A','B','A'];
+let idx = 0;
+function _scheduleMessageVirtualizedRender(force){
+  scheduled++;
+  if(idx<keys.length){
+    _scheduleMessageVirtualMeasurementRefresh({ key: keys[idx++] });
+  }
 }
-console.log(JSON.stringify({
-  scheduled: scheduled,
-  retries: _messageVirtualMeasurementRetryCount,
-}));
-"""
-    metrics = json.loads(_run_node(source))
-    assert metrics["scheduled"] == 2, (
-        "cycle-key oscillation must not reset the retry budget: "
-        f"scheduled {metrics['scheduled']} re-renders (expected 2, the "
-        "MESSAGE_VIRTUAL_MEASUREMENT_MAX_RERENDERS cap for the whole burst)"
-    )
-    assert metrics["retries"] == 2, (
-        "retry count must keep accumulating across key changes: "
-        f"ended at {metrics['retries']} (expected 2)"
-    )
-
-
-def test_measurement_retry_budget_still_caps_stable_key_burst():
-    """#6654 guard: with a stable cycle key the budget must still cap the burst
-    at MESSAGE_VIRTUAL_MEASUREMENT_MAX_RERENDERS (the removed per-key reset
-    must not have weakened the stable-key path)."""
-    js = UI_JS_PATH.read_text(encoding="utf-8")
-    source = _extract_func_script(js) + """
-let scheduled = 0;
-let _messageVirtualMeasurementCycleKey = '';
-let _messageVirtualMeasurementRetryCount = 0;
-let _messageVirtualScrollActive = false;
-let _messageVirtualDeferredMeasurement = null;
-const MESSAGE_VIRTUAL_MEASUREMENT_MAX_RERENDERS = 2;
-function _messageVirtualMeasurementCycleKeyFor(w) { return w.key; }
-function _scheduleMessageVirtualizedRender(force) { scheduled++; }
-function requestAnimationFrame(cb) { cb(); }
+function requestAnimationFrame(cb){ cb(); }
 eval(extractFunc('_scheduleMessageVirtualMeasurementRefresh'));
-for (let i = 0; i < 6; i++) {
-  _scheduleMessageVirtualMeasurementRefresh({ key: 'A' });
-}
-// The budget is exhausted after two renders; a settled measurement resets it.
-_messageVirtualMeasurementRetryCount = 0;
+// External trigger measures key A; the internal chain then oscillates A->B->A->B.
 _scheduleMessageVirtualMeasurementRefresh({ key: 'A' });
 console.log(JSON.stringify({
   scheduled: scheduled,
   retries: _messageVirtualMeasurementRetryCount,
+  burstActive: _messageVirtualMeasurementBurstActive,
 }));
 """
     metrics = json.loads(_run_node(source))
-    assert metrics["scheduled"] == 3, (
-        "stable-key burst must stop at MESSAGE_VIRTUAL_MEASUREMENT_MAX_RERENDERS "
-        f"and resume only after a budget reset: scheduled {metrics['scheduled']} "
-        "(expected 3: 2 capped + 1 after explicit reset)"
+    assert metrics["scheduled"] == 2, (
+        "cycle-key oscillation must not reset the retry budget within a burst: "
+        f"scheduled {metrics['scheduled']} re-renders (expected 2, the "
+        "MESSAGE_VIRTUAL_MEASUREMENT_MAX_RERENDERS cap for the whole burst)"
     )
-    assert metrics["retries"] == 1, (
-        f"retry count after explicit reset must be 1, got {metrics['retries']}"
+    assert metrics["retries"] == 2, (
+        "retry count must keep accumulating across key changes within the burst: "
+        f"ended at {metrics['retries']} (expected 2)"
+    )
+    assert metrics["burstActive"] is False, (
+        "the burst must end once the budget is exhausted so the next externally "
+        f"initiated cycle can reset it: burstActive {metrics['burstActive']}"
+    )
+
+
+def test_external_measurement_after_thrash_gets_fresh_retry_budget():
+    """#6717 re-gate: exhausting the budget with the WebKit A/B thrash must NOT
+    starve a genuinely new externally initiated measurement cycle. Drive the
+    A/B oscillation causally until the per-burst budget is exhausted, then start
+    an independent changed-C measurement WITHOUT touching private state:
+    (a) C must still receive its own retries, and (b) C's own internal
+    oscillation must remain capped at MESSAGE_VIRTUAL_MEASUREMENT_MAX_RERENDERS."""
+    js = UI_JS_PATH.read_text(encoding="utf-8")
+    source = _extract_func_script(js) + """
+let scheduled = 0;
+let _messageVirtualMeasurementCycleKey = '';
+let _messageVirtualMeasurementRetryCount = 0;
+let _messageVirtualMeasurementBurstActive = false;
+let _messageVirtualScrollActive = false;
+let _messageVirtualDeferredMeasurement = null;
+const MESSAGE_VIRTUAL_MEASUREMENT_MAX_RERENDERS = 2;
+function _messageVirtualMeasurementCycleKeyFor(w) { return w.key; }
+let keys = [];
+let idx = 0;
+function _scheduleMessageVirtualizedRender(force){
+  scheduled++;
+  if(idx<keys.length){
+    _scheduleMessageVirtualMeasurementRefresh({ key: keys[idx++] });
+  }
+}
+function requestAnimationFrame(cb){ cb(); }
+eval(extractFunc('_scheduleMessageVirtualMeasurementRefresh'));
+// Phase 1 — drive the A/B thrash causally until the burst budget is exhausted.
+keys = ['B','A','B','A'];
+idx = 0;
+_scheduleMessageVirtualMeasurementRefresh({ key: 'A' });
+const afterThrash = scheduled;
+// Phase 2 — an independent genuine content change C arrives. It must NOT be
+// starved: it gets a fresh per-burst budget, and its own oscillation is capped.
+keys = ['D','C','D','C'];
+idx = 0;
+_scheduleMessageVirtualMeasurementRefresh({ key: 'C' });
+console.log(JSON.stringify({
+  afterThrash: afterThrash,
+  scheduled: scheduled,
+  retries: _messageVirtualMeasurementRetryCount,
+  burstActive: _messageVirtualMeasurementBurstActive,
+}));
+"""
+    metrics = json.loads(_run_node(source))
+    assert metrics["afterThrash"] == 2, (
+        "the A/B thrash burst must stop at MESSAGE_VIRTUAL_MEASUREMENT_"
+        f"MAX_RERENDERS: scheduled {metrics['afterThrash']} (expected 2)"
+    )
+    assert metrics["scheduled"] == 4, (
+        "a genuinely new external measurement cycle C must still receive its "
+        "own retries after the thrash exhausted the budget: "
+        f"scheduled {metrics['scheduled']} (expected 4 = 2 thrash + 2 for C)"
+    )
+    assert metrics["retries"] == 2, (
+        "C's own internal oscillation must remain capped at "
+        f"MESSAGE_VIRTUAL_MEASUREMENT_MAX_RERENDERS: retries "
+        f"{metrics['retries']} (expected 2)"
     )

tests/test_issue734_message_windowing.py

@@ -56,24 +56,38 @@ def test_measurement_rerenders_are_bounded_per_virtual_window_cycle():
     assert "const MESSAGE_VIRTUAL_MEASUREMENT_MAX_RERENDERS=2;" in UI_JS
     assert "function _messageVirtualMeasurementCycleKeyFor(windowMetrics)" in UI_JS
     assert "function _scheduleMessageVirtualMeasurementRefresh(windowMetrics)" in UI_JS
-    assert "if(_messageVirtualMeasurementRetryCount>=MESSAGE_VIRTUAL_MEASUREMENT_MAX_RERENDERS) return;" in UI_JS
+    assert "if(_messageVirtualMeasurementRetryCount>=MESSAGE_VIRTUAL_MEASUREMENT_MAX_RERENDERS){" in UI_JS
     assert "_scheduleMessageVirtualMeasurementRefresh(virtualWindow);" in UI_JS
     assert "_markMessageVirtualMeasurementsSettled(virtualWindow);" in UI_JS
 
 
 def test_measurement_retry_budget_not_reset_by_cycle_key_change():
-    """#6654: the cycle-key branch of _scheduleMessageVirtualMeasurementRefresh
+    """#6654/#6717: the cycle-key branch of _scheduleMessageVirtualMeasurementRefresh
     must record the new key but never reset _messageVirtualMeasurementRetryCount.
     Resetting on key change lets WebKit's A->B->A->B metric oscillation renew
-    the two-render budget forever, keeping the rAF/measure loop alive."""
+    the two-render budget forever, keeping the rAF/measure loop alive. The ONLY
+    reset allowed inside the scheduler is the per-burst one: when a NEW
+    externally initiated cycle begins (no burst active), guarded by
+    if(!_messageVirtualMeasurementBurstActive)."""
     idx = UI_JS.index("function _scheduleMessageVirtualMeasurementRefresh(windowMetrics)")
     end = UI_JS.index("function _markMessageVirtualMeasurementsSettled", idx)
     body = UI_JS[idx:end]
     # The key-change branch must only assign the key.
     assert "if(_messageVirtualMeasurementCycleKey!==cycleKey){" in body
-    assert "_messageVirtualMeasurementRetryCount=0;" not in body, (
+    key_branch_start = body.index("if(_messageVirtualMeasurementCycleKey!==cycleKey){")
+    key_branch_end = body.index("}", key_branch_start)
+    key_branch = body[key_branch_start:key_branch_end]
+    assert "_messageVirtualMeasurementRetryCount=0;" not in key_branch, (
         "cycle-key change must not reset the retry budget (issue #6654)"
     )
+    # The per-burst reset is allowed only at the start of a new external cycle.
+    assert "if(!_messageVirtualMeasurementBurstActive){" in body
+    guard_pos = body.index("if(!_messageVirtualMeasurementBurstActive){")
+    reset_pos = body.index("_messageVirtualMeasurementRetryCount=0;")
+    assert guard_pos < reset_pos, (
+        "the budget reset must be guarded by the burst-active check "
+        "(per-burst lifecycle, issue #6717)"
+    )
     # The budget guard and increment must still be present.
-    assert "if(_messageVirtualMeasurementRetryCount>=MESSAGE_VIRTUAL_MEASUREMENT_MAX_RERENDERS) return;" in body
+    assert "if(_messageVirtualMeasurementRetryCount>=MESSAGE_VIRTUAL_MEASUREMENT_MAX_RERENDERS){" in body
     assert "_messageVirtualMeasurementRetryCount++;" in body