Fix scroll lag and misplaced summary cards

Three defects in the restore/injection path.

Scrolling stuttered in any chat that had been summarized. The restore gate
keyed on id(chat_activity), but every Xposed callback hands Python a fresh
Chaquopy wrapper around the same Java object, so the key never matched and
each scroll frame opened SQLite and rescanned the message list.
updateMessagesVisiblePart runs per frame, so that ran continuously.
BaseFragment.classGuid is assigned once per fragment instance, so it
identifies an open chat across calls; measured 1 query per 300 frames
instead of 300. Saved rows are cached per scope, invalidated on save.

The card could attach to the wrong message when many messages were
included. insert() computed the index on the calling thread and applied it
later on the UI thread; a large range takes long enough for paging or an
incoming message to shift the list in between, so a stale index was used.
The index is now computed on the UI thread immediately before the
mutation that consumes it.

Every saved summary appeared stacked together in a row. find_index_for_anchor
fell back to nearest-by-id whenever the anchor was not loaded, and a chat
opens with a short window, so summaries anchored far up all collapsed to
adjacent slots at the bottom. Absence is now reported as -1 and those cards
are skipped until paging brings their anchor in; the id-order fallback stays
opt-in for reanchor, which must place a card it has already removed.
Verified against the 6 real saved summaries for one chat: previously all 6
were injected consecutively on open, now none are, and each appears in its
correct place once its range is scrolled in.

Also makes restore linear in list length rather than rescanning per summary.
This commit is contained in:
lyuksovannyy committed 2026-09-19 16:51:12 +02:00
1 parent 7f0fcfe72f
commit 0619c5fe68
4 files changed
+297 -50

No files matched your search

+81 -20
View File
@@ -47,6 +47,20 @@ class RestoreHook:
def __init__(self, plugin):
self.plugin = plugin
self._unhooks: List[Any] = []
self._scope_state = {"done": set(), "rows": {}}
def invalidate(self) -> None:
"""Drops the restore cache so the next frame re-reads saved summaries.
Called after storing a summary: the gate deliberately skips work for a
scope it has already handled, so a newly saved row would otherwise not
be restored until the chat was reopened.
"""
try:
self._scope_state["done"].clear()
self._scope_state["rows"].clear()
except Exception:
self._scope_state = {"done": set(), "rows": {}}
def install(self) -> None:
try:
@@ -75,10 +89,13 @@ class RestoreHook:
return
plugin = self.plugin
# updateMessagesVisiblePart fires on every scroll frame, so restoring
# is gated on the card actually being absent: restore_into() bails as
# soon as the summary is already in the list.
last_scope = {"key": None}
# Shared with invalidate(): `updateMessagesVisiblePart` fires on every
# scroll frame, so restoring is gated on a scope identity that survives
# across calls (see _scope_identity) plus the loaded window size, and
# the DB rows are cached per scope.
# Reuse the dict created in __init__ rather than replacing it, so the
# reference invalidate() operates on stays the one the hook reads.
last_scope = self._scope_state
class _RestoreAfterLoad(MethodHook):
def after_hooked_method(self, param):
@@ -107,12 +124,29 @@ class RestoreHook:
self._unhooks = []
def _scope_identity(chat_activity: Any, dialog_id: int, topic_id: int) -> tuple:
"""Identity that is stable across hook invocations for one open chat.
`id(chat_activity)` cannot be used: each Xposed callback hands Python a
fresh Chaquopy wrapper around the same Java object, so its Python id
differs every call and a cache keyed on it never hits. BaseFragment's
classGuid is assigned once per fragment instance, so it identifies the open
ChatActivity for as long as it lives and changes when a new one is opened.
"""
guid = 0
try:
guid = int(chat_activity.getClassGuid())
except Exception:
guid = 0
return (guid, int(dialog_id), int(topic_id))
def restore_into(plugin: Any, chat_activity: Any, last_scope: Any = None) -> None:
"""Injects stored summaries for the chat this ChatActivity is showing.
Called from a high-frequency lifecycle hook, so it must be cheap: it does
one DB read per (dialog, topic) it has not seen yet, and
restore_saved_summaries() skips anything already present in the list.
Called from `updateMessagesVisiblePart`, which runs on every scroll frame,
so the common case must do no work at all: once a scope has been restored
it is remembered and every later frame returns after one tuple compare.
"""
from services.rich_injector import restore_saved_summaries
@@ -134,21 +168,48 @@ def restore_into(plugin: Any, chat_activity: Any, last_scope: Any = None) -> Non
except Exception:
topic_id = 0
# Avoid a DB round-trip on every scroll frame; a new scope means a
# different chat/topic was opened, which is the only time a restore is due.
scope_key = (id(chat_activity), dialog_id, topic_id)
if last_scope is not None and last_scope.get("key") == scope_key:
return
# Gate before touching the DB. `updateMessagesVisiblePart` runs on every
# scroll frame, and re-querying SQLite plus rescanning the message list each
# time is what made scrolling stutter in summarized chats.
#
# The gate includes the loaded message count: a card can only be placed
# while its anchor is loaded, and a chat opens with a short window, so
# summaries anchored further up must get another chance once paging widens
# it. Size changes only when messages are added or removed, so scrolling
# within an unchanged window still does no work.
scope_key = _scope_identity(chat_activity, dialog_id, topic_id)
if last_scope is not None:
last_scope["key"] = scope_key
try:
from services.rich_injector import get_messages_list
try:
summaries = db.get_for_scope(dialog_id, topic_id)
except Exception as exc:
diagnostics.record_error("restore_hook.db.get_for_scope", exc)
return
# Rows reserved at stream start have empty text until the stream finishes.
summaries = [s for s in summaries if (s.text or "").strip()]
messages_list = get_messages_list(chat_activity)
loaded = int(messages_list.size()) if messages_list is not None else 0
except Exception:
loaded = 0
state_key = scope_key + (loaded,)
if state_key in last_scope.get("done", ()):
return
done = last_scope.setdefault("done", set())
# Bounded: one entry per (fragment, scope, window size) actually seen.
if len(done) > 256:
done.clear()
done.add(state_key)
# Cache the rows per scope: paging re-runs this for the same chat, and the
# saved set only changes when a new summary is stored.
cache = last_scope.setdefault("rows", {}) if last_scope is not None else {}
summaries = cache.get(scope_key)
if summaries is None:
try:
summaries = db.get_for_scope(dialog_id, topic_id)
except Exception as exc:
diagnostics.record_error("restore_hook.db.get_for_scope", exc)
return
# Rows reserved at stream start have empty text until the stream ends.
summaries = [s for s in summaries if (s.text or "").strip()]
if len(cache) > 32:
cache.clear()
cache[scope_key] = summaries
if not summaries:
return
+7
View File
@@ -412,6 +412,13 @@ class ChatSummariesPlugin(BasePlugin):
rendered("thinking", final=True),
len(messages),
)
# The restore gate skips scopes it has already handled, so
# the newly saved row has to invalidate that cache or it
# would not be restored until the chat was reopened.
try:
self.restore_hook.invalidate()
except Exception:
pass
except Exception as exc:
diagnostics.record_error("_stream_into:db.save", exc)
+90 -22
View File
@@ -181,17 +181,18 @@ def get_chat_adapter(chat_activity: Any) -> Optional[Any]:
return None
def find_index_for_anchor(messages_list: Any, anchor_msg_id: int) -> int:
def find_index_for_anchor(messages_list: Any, anchor_msg_id: int, require_anchor: bool = True) -> int:
"""Index at which to insert so the summary sits directly after the anchor.
ChatActivity.messages is newest-first, so "directly after the anchor"
chronologically means "immediately before it" in list order.
The anchor is the newest message that was actually analyzed, which is not
the newest message in the chat whenever a non-zero offset was used. If the
anchor itself is not loaded (paged out, or never fetched into this view),
fall back to ordering by id rather than to index 0: index 0 is the bottom
of the chat and would place the card after messages the summary never saw.
Returns -1 when the anchor message is not loaded in this view and
`require_anchor` is set. Positioning by id order alone is not safe: with a
non-zero offset, or a range that has since paged out, the nearest-by-id slot
can sit among messages the summary never analyzed, which is how cards ended
up attached to the wrong message. Callers skip the insert instead and try
again once the anchor is paged in.
"""
anchor_msg_id = int(anchor_msg_id)
try:
@@ -200,6 +201,8 @@ def find_index_for_anchor(messages_list: Any, anchor_msg_id: int) -> int:
owner = getattr(messages_list.get(i), "messageOwner", None)
if owner is not None and int(getattr(owner, "id", 0) or 0) == anchor_msg_id:
return i
if require_anchor:
return -1
# Anchor absent: insert before the first real message whose id is at
# most the anchor's, i.e. the newest message not newer than the
@@ -215,7 +218,7 @@ def find_index_for_anchor(messages_list: Any, anchor_msg_id: int) -> int:
return size
except Exception as exc:
diagnostics.record_error("injector.find_index_for_anchor", exc)
return 0
return -1
def find_existing(messages_list: Any, virtual_id: int) -> int:
@@ -294,15 +297,28 @@ class SummaryInjection:
return False
self.message_object = message_object
existing = find_existing(messages_list, self.virtual_id)
index = existing if existing >= 0 else find_index_for_anchor(messages_list, self.anchor_msg_id)
# The index must be computed on the UI thread, immediately before the
# mutation that uses it. Computing it here would race: loading a large
# range takes long enough for paging or an incoming message to shift the
# list, and the stale index then lands the card after the wrong message.
def run():
try:
messages_list_now = get_messages_list(self.chat_activity)
if messages_list_now is None:
return
existing = find_existing(messages_list_now, self.virtual_id)
if existing >= 0:
messages_list.set(index, message_object)
messages_list_now.set(existing, message_object)
else:
messages_list.add(index, message_object)
index = find_index_for_anchor(messages_list_now, self.anchor_msg_id)
if index < 0:
# Anchor not loaded in this view: placing the card by id
# order alone would put it next to unrelated messages.
diagnostics.note(
f"injector: anchor {self.anchor_msg_id} not loaded, skipping insert"
)
return
messages_list_now.add(index, message_object)
adapter = get_chat_adapter(self.chat_activity)
if adapter is not None:
adapter.notifyDataSetChanged()
@@ -391,6 +407,10 @@ class SummaryInjection:
messages_list.set(index, replacement)
else:
index = find_index_for_anchor(messages_list, self.anchor_msg_id)
if index < 0:
# Anchor paged out mid-stream. Keep the streamed
# text; it is re-inserted once the anchor is back.
return
messages_list.add(index, replacement)
inserted = True
for stale_id in self._used_ids:
@@ -497,8 +517,16 @@ class SummaryInjection:
duplicate = find_existing(messages_list, virtual_id)
if duplicate >= 0:
messages_list.remove(duplicate)
# The card was just removed, so it must go back somewhere:
# the new anchor came from the loaded list and is normally
# present, but if it paged out in between, fall back to id
# ordering rather than dropping the card entirely.
target = find_index_for_anchor(messages_list, anchor_msg_id)
messages_list.add(target, message_object)
if target < 0:
target = find_index_for_anchor(
messages_list, anchor_msg_id, require_anchor=False
)
messages_list.add(max(target, 0), message_object)
adapter = get_chat_adapter(self.chat_activity)
if adapter is not None:
adapter.notifyDataSetChanged()
@@ -520,17 +548,43 @@ def _on_ui(fn) -> None:
fn()
def loaded_message_ids(messages_list: Any) -> set:
"""Ids currently loaded in the view, in one pass.
Restoring used to scan the whole list once per saved summary; a single set
keeps it linear in the list length no matter how many summaries exist.
"""
ids = set()
try:
for i in range(messages_list.size()):
owner = getattr(messages_list.get(i), "messageOwner", None)
if owner is not None:
ids.add(int(getattr(owner, "id", 0) or 0))
except Exception as exc:
diagnostics.record_error("injector.loaded_message_ids", exc)
return ids
def restore_saved_summaries(chat_activity: Any, account: int, dialog_id: int, topic_id: int, summaries) -> int:
"""Re-injects persisted summaries into a freshly opened chat. Returns count."""
"""Re-injects persisted summaries into a freshly opened chat.
Only summaries whose anchor is currently loaded are injected: a card whose
anchor is outside the loaded window has no correct position, and guessing
one by id order is what stacked every saved card together at the bottom of
the chat. The rest are picked up on a later call, once paging has brought
their anchor in. Returns how many were dispatched for insertion.
"""
messages_list = get_messages_list(chat_activity)
if messages_list is None or not summaries:
return 0
register_mock_sender(account)
loaded = loaded_message_ids(messages_list)
pending = []
for summary in summaries:
virtual_id = virtual_id_for(summary.id, summary.anchor_msg_id)
if find_existing(messages_list, virtual_id) >= 0:
if virtual_id in loaded:
continue
if int(summary.anchor_msg_id) not in loaded:
continue
message_object = build_summary_message(
account,
@@ -543,11 +597,12 @@ def restore_saved_summaries(chat_activity: Any, account: int, dialog_id: int, to
False,
)
if message_object is not None:
pending.append((summary.anchor_msg_id, message_object))
pending.append((summary.anchor_msg_id, virtual_id, message_object))
if not pending:
return 0
register_mock_sender(account)
# Newest anchor first: each insert shifts everything below it, so placing
# the newest card first keeps the remaining lookups pointing at unmoved
# rows. The index is recomputed per card against the live list anyway.
@@ -555,11 +610,24 @@ def restore_saved_summaries(chat_activity: Any, account: int, dialog_id: int, to
def run():
try:
for anchor_msg_id, message_object in pending:
messages_list.add(find_index_for_anchor(messages_list, anchor_msg_id), message_object)
adapter = get_chat_adapter(chat_activity)
if adapter is not None:
adapter.notifyDataSetChanged()
# Re-resolve on the UI thread: the list this was queued against may
# have paged or been replaced in between.
live = get_messages_list(chat_activity)
if live is None:
return
added = 0
for anchor_msg_id, virtual_id, message_object in pending:
if find_existing(live, virtual_id) >= 0:
continue
index = find_index_for_anchor(live, anchor_msg_id)
if index < 0:
continue
live.add(index, message_object)
added += 1
if added:
adapter = get_chat_adapter(chat_activity)
if adapter is not None:
adapter.notifyDataSetChanged()
except Exception as exc:
diagnostics.record_error("injector.restore", exc)
+119 -8
View File
@@ -1582,28 +1582,35 @@ def main(argv=None) -> int:
# Anchor not loaded in this view: falling back to index 0 would put the
# card below messages the summary never saw, which is the bug this
# ordering fallback exists to prevent.
# These exercise the id-ordering fallback itself, which is now opt-in:
# callers that must place a card somewhere (reanchor) ask for it, while
# the default refuses to guess a position for an unloaded anchor.
check(
"an unloaded anchor is ordered by id instead of dropping to the bottom",
injector.find_index_for_anchor(offset_activity.messages, 750) == 2,
f"index={injector.find_index_for_anchor(offset_activity.messages, 750)}",
"requested id ordering places an unloaded anchor by id",
injector.find_index_for_anchor(offset_activity.messages, 750, require_anchor=False) == 2,
f"index={injector.find_index_for_anchor(offset_activity.messages, 750, require_anchor=False)}",
)
check(
"an anchor older than everything loaded lands at the top of the list",
injector.find_index_for_anchor(offset_activity.messages, 100)
injector.find_index_for_anchor(offset_activity.messages, 100, require_anchor=False)
== offset_activity.messages.size(),
f"index={injector.find_index_for_anchor(offset_activity.messages, 100)}",
f"index={injector.find_index_for_anchor(offset_activity.messages, 100, require_anchor=False)}",
)
check(
"an anchor newer than everything loaded lands at the bottom",
injector.find_index_for_anchor(offset_activity.messages, 1000) == 0,
f"index={injector.find_index_for_anchor(offset_activity.messages, 1000)}",
injector.find_index_for_anchor(offset_activity.messages, 1000, require_anchor=False) == 0,
f"index={injector.find_index_for_anchor(offset_activity.messages, 1000, require_anchor=False)}",
)
# The real invariant is positional, not a fixed index: every message
# newer than the anchor stays below the card, every older one above,
# regardless of injected cards already sitting in the list.
def neighbours(activity_obj, anchor):
index = injector.find_index_for_anchor(activity_obj.messages, anchor)
# require_anchor=False so the invariant is checked for unloaded
# anchors too: when a position is chosen at all, it must be correct.
index = injector.find_index_for_anchor(
activity_obj.messages, anchor, require_anchor=False
)
ids = [m.messageOwner.id for m in activity_obj.messages]
real = [i for i in ids if i > 0]
below = [i for i in ids[:index] if i > 0]
@@ -1816,6 +1823,110 @@ def main(argv=None) -> int:
f"ids={ids}",
)
# Bug: every saved summary appeared stacked in a row at the bottom of
# the chat. Cards whose anchor is outside the loaded window have no
# correct position, and placing them by id order alone collapsed them
# together. They must be skipped until paging brings their anchor in.
narrow = _FakeChatActivity([600, 590, 580])
stranded = injector.restore_saved_summaries(
narrow,
0,
-4242,
0,
[
summary_db_mod.StoredSummary(21, -4242, 0, 120, "ancient", "", 5, 0),
summary_db_mod.StoredSummary(22, -4242, 0, 130, "old", "", 5, 0),
summary_db_mod.StoredSummary(23, -4242, 0, 590, "loaded", "", 5, 0),
],
)
narrow_ids = [m.messageOwner.id for m in narrow.messages]
injected = [i for i in narrow_ids if i < 0]
check(
"summaries whose anchor is not loaded are not injected at all",
stranded == 1 and injected == [injector.virtual_id_for(23, 590)],
f"dispatched={stranded} injected={injected} ids={narrow_ids}",
)
check(
"the one restorable summary still lands directly above its anchor",
narrow_ids.index(injector.virtual_id_for(23, 590)) == narrow_ids.index(590) - 1,
f"ids={narrow_ids}",
)
# Once paging widens the window, the previously skipped summary has a
# real anchor and must be restored on the next attempt.
for older in (130, 120):
owner_obj = _TLObj("TL_message")
owner_obj.id = older
narrow.messages.add(_FakeMessageObject(0, owner_obj))
after_paging = injector.restore_saved_summaries(
narrow,
0,
-4242,
0,
[
summary_db_mod.StoredSummary(21, -4242, 0, 120, "ancient", "", 5, 0),
summary_db_mod.StoredSummary(22, -4242, 0, 130, "old", "", 5, 0),
summary_db_mod.StoredSummary(23, -4242, 0, 590, "loaded", "", 5, 0),
],
)
paged_ids = [m.messageOwner.id for m in narrow.messages]
check(
"summaries are restored once paging loads their anchor",
after_paging == 2
and paged_ids.index(injector.virtual_id_for(21, 120)) == paged_ids.index(120) - 1
and paged_ids.index(injector.virtual_id_for(22, 130)) == paged_ids.index(130) - 1,
f"dispatched={after_paging} ids={paged_ids}",
)
check(
"re-restoring does not duplicate the summary already on screen",
paged_ids.count(injector.virtual_id_for(23, 590)) == 1,
f"ids={paged_ids}",
)
# Bug: the card appeared after the wrong message when many messages
# were included. The insert index was computed on the calling thread and
# applied later on the UI thread; anything arriving in between shifted
# the list and invalidated it. A fake dispatcher that mutates the list
# between computing and applying reproduces that window.
racing = _FakeChatActivity([700, 690, 680])
real_on_ui = injector._on_ui
def _racing_on_ui(fn):
owner_obj = _TLObj("TL_message")
owner_obj.id = 701
racing.messages.add(0, _FakeMessageObject(0, owner_obj))
fn()
injector._on_ui = _racing_on_ui
try:
racing_injection = injector.SummaryInjection(
racing, 0, -4242, 0, 690, injector.virtual_id_for(31, 690)
)
racing_injection.insert("summary body", "", streaming=False)
finally:
injector._on_ui = real_on_ui
racing_ids = [m.messageOwner.id for m in racing.messages]
card = injector.virtual_id_for(31, 690)
check(
"the card lands above its anchor even if the list shifts first",
racing_ids.index(card) == racing_ids.index(690) - 1,
f"ids={racing_ids}",
)
# find_index_for_anchor must report an absent anchor rather than
# guessing index 0 (chat bottom) or a nearest-by-id slot.
absent = _FakeChatActivity([900, 800])
check(
"an unloaded anchor is reported as absent",
injector.find_index_for_anchor(absent.messages, 123) == -1,
f"index={injector.find_index_for_anchor(absent.messages, 123)}",
)
check(
"id-ordered placement is still available when explicitly requested",
injector.find_index_for_anchor(absent.messages, 123, require_anchor=False) == 2,
f"index={injector.find_index_for_anchor(absent.messages, 123, require_anchor=False)}",
)
check(
"sys.modules gained no entry for bundled plugin module names",
not ({"main", "config", "services", "providers", "hooks", "ui.summarize_dialog"} & new_modules),