Skip to content

fix(tui): keep chat list sorted by updated.value across all mutations#449

Open
manuel-goepfi wants to merge 1 commit into
pieces-app:mainfrom
manuel-goepfi:fix/tui-chat-list-first-shot-order
Open

fix(tui): keep chat list sorted by updated.value across all mutations#449
manuel-goepfi wants to merge 1 commit into
pieces-app:mainfrom
manuel-goepfi:fix/tui-chat-list-first-shot-order

Conversation

@manuel-goepfi
Copy link
Copy Markdown

@manuel-goepfi manuel-goepfi commented Apr 28, 2026

Summary

The chat list panel held the newest-first invariant only at first-shot load. Three mutation paths could subsequently desync the visible order from chat.conversation.updated.value, leaving stale chats stuck at the top of the list.

Root causes

  • add_chat only sorted while ConversationsSnapshot.first_shot was True. After first-shot, a chat that appeared new to the widget map (because the panel had never seen it) jumped to position 0 regardless of timestamp.
  • add_chat_at_top blindly prepended every chat passed to it, even when the chat's updated.value placed it elsewhere in the order.
  • update_chat mutated title/summary in place but never re-positioned, so a chat whose updated.value just bumped (new message arrival) did not bubble to the top.

Fix

Centralise the ordering rule via two helpers (_chat_sort_key and its tuple variant) returning (rank, value) so chats with usable timestamps sort by datetime, while chats whose snapshot fails to load sink to the bottom under reverse=True without raising.

  • load_chats sorts the incoming list before mounting (newest-first ordering even when copilot.chats() returns identifiers in arrival order).
  • add_chat always sorts via _sort_items_by_updated. The existing _reorder_pending debounce moves into _schedule_reorder and continues to coalesce reorder passes during first-shot drain (avoids O(n²) when hundreds of WS-streamed chats arrive in one refresh cycle). Post-first-shot, events are sparse — reorder eagerly.
  • add_chat_at_top becomes a thin shim over add_chat. A genuinely new chat lands at the top via its newest timestamp; a chat that merely appeared new to the widget map falls into its correct slot.
  • update_chat resorts and reorders so a bumped updated.value bubbles the chat in the DOM to match the data invariant.
  • base_list_panel._reorder_item_widgets narrows its bare-Exception handler to NoMatches (the only legitimately swallowable exception here — Textual mount is async); other exceptions propagate.

Sort failures are logged with chat id and exception type so a future regression that breaks .value loading is observable rather than silent.

Test plan

  • Verified locally against 74 chats including a stale entry that previously stuck at position 0 — it now lands at the correct rank.
  • No NameError on import (_chat_sort_key_tuple referenced before definition was the prior crash mode).
  • First-shot drain still debounces reorders (_reorder_pending flag preserved).

@manuel-goepfi manuel-goepfi force-pushed the fix/tui-chat-list-first-shot-order branch 6 times, most recently from c3d05ca to 2d9b11b Compare April 28, 2026 17:13
The chat list panel held the newest-first invariant only at first-shot
load. Three mutation paths could subsequently desync the visible order
from chat.conversation.updated.value, leaving stale chats stuck at the
top:

* `add_chat` only sorted while ConversationsSnapshot.first_shot was
  True. After first-shot, a chat that *appeared* new to the widget map
  (because the panel had never seen it) jumped to position 0 regardless
  of timestamp.
* `add_chat_at_top` blindly prepended every chat passed to it, even
  when the chat's updated.value placed it elsewhere in the order.
* `update_chat` mutated title/summary in place but never re-positioned,
  so a chat whose updated.value just bumped (new message arrival) did
  not bubble to the top.

This commit centralises the ordering rule:

* Adds a module-level `_chat_sort_key` returning (rank, value) tuples
  so chats with a usable timestamp sort by datetime, while chats whose
  snapshot fails to load sink to the bottom under reverse=True without
  raising. `_chat_sort_key_tuple` is the (chat, title, subtitle) variant
  used by the items list.
* `load_chats` now sorts the incoming list before mounting, so the
  panel opens with newest-first ordering even when copilot.chats()
  returns identifiers in arrival order.
* `add_chat` always sorts, gated by `_sort_items_by_updated`. The
  existing `_reorder_pending` debounce moves into `_schedule_reorder`
  and continues to coalesce reorder passes during first-shot drain
  (avoids O(n^2) when hundreds of WS-streamed chats arrive in one
  refresh cycle). Post-first-shot, events are sparse — reorder eagerly.
* `add_chat_at_top` becomes a thin shim over `add_chat`. A genuinely
  new chat will already land at the top via its newest timestamp; a
  chat that merely appeared new to the widget map falls into its
  correct slot instead of jumping forward.
* `update_chat` resorts and reorders so a bumped updated.value bubbles
  the chat in the DOM to match the data invariant.
* `base_list_panel._reorder_item_widgets` now narrows its bare-Exception
  handler to `NoMatches`, the only legitimately swallowable exception
  here (Textual mount is async and the just-mounted sibling may not
  yet be in container.children); other exceptions propagate.

Sort failures are logged with chat id and exception type so a future
regression that breaks .value loading is observable rather than silent.

Verified locally against 74 chats including a stale entry that
previously stuck at position 0 — it now lands at the correct rank
under the new sort.
@manuel-goepfi manuel-goepfi force-pushed the fix/tui-chat-list-first-shot-order branch from 2d9b11b to a9742ca Compare April 29, 2026 06:51
@manuel-goepfi manuel-goepfi changed the title fix(tui): sort chat list by updated time during first-shot load fix(tui): keep chat list sorted by updated.value across all mutations Apr 29, 2026
Copy link
Copy Markdown
Member

@mark-at-pieces mark-at-pieces left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great change looking solid, might ass some basic unit tests;

but thinking updated is the way to go here!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants