Skip to content

fix(desktop): preserve expanded messaging pages across polling - #65527

Open
PINKIIILQWQ wants to merge 1 commit into
NousResearch:mainfrom
PINKIIILQWQ:fix/desktop-messaging-refresh-preserve-pages
Open

fix(desktop): preserve expanded messaging pages across polling#65527
PINKIIILQWQ wants to merge 1 commit into
NousResearch:mainfrom
PINKIIILQWQ:fix/desktop-messaging-refresh-preserve-pages

Conversation

@PINKIIILQWQ

Copy link
Copy Markdown
Contributor

What does this PR do?

Prevents Desktop messaging sections from shrinking after the user loads more conversations and the 10-second background poll runs.

Regression

The live messaging poll added by #57636 correctly keeps Telegram / WeChat / Feishu traffic fresh, but its combined seed is capped at 100 rows across all platforms. A per-platform load-more request can load 13 Feishu conversations while the next combined seed contains only 4 Feishu rows. The refresh previously replaced the entire messaging cache, so the section appeared to collapse from 13 back to 4.

This was discovered while manually validating #51407 and is intentionally kept separate from that scrollbar-only PR.

Changes

  • Reconcile capped messaging refreshes instead of clobbering the cache.
  • Preserve the already-loaded count independently for each messaging platform.
  • Let fresh seed rows replace older rows, so repeated polling does not grow the cache indefinitely.
  • Keep sub-cap refreshes authoritative so externally deleted or archived sessions can disappear.
  • Add store-level and hook-level behavioral regression coverage.

Reproduction covered by the test

  1. Load 13 Feishu conversations through the platform pager.
  2. Return a capped combined seed containing 4 Feishu rows and 96 Telegram rows.
  3. Run refreshMessagingSessions().
  4. Feishu remains at 13 rows instead of shrinking to 4.

The hook-level test failed on unmodified main with: expected 13, received 4.

Validation

  • npm run test:ui --workspace apps/desktop -- src/store/session.test.ts src/app/session/hooks/use-session-list-actions.test.tsx — 26 passed
  • npm run typecheck --workspace apps/desktop — passed
  • ESLint on all changed files — passed with no warnings
  • Prettier check on all changed files — passed
  • git diff --check — passed

Scope

Desktop renderer state reconciliation only. No gateway, database, API, CSS, or polling-interval changes.

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/desktop Electron desktop app (apps/desktop/*) sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Jul 16, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for isolating a real renderer-cache regression. Current main fetches the aggregate messaging seed at apps/desktop/src/app/session/hooks/use-session-list-actions.ts:100, directly replaces the cache at :108, and independently expands a platform at :120-133; the PR's per-source reconciliation addresses that mismatch.

Problems

  • apps/desktop/src/app/session/hooks/use-session-list-actions.ts:109 (PR right side) still publishes any completed refresh. The 10-second poll at apps/desktop/src/app/contrib/hooks/use-background-sync.ts:102-110 can overlap another refresh, but unlike refreshSessions (use-session-list-actions.ts:76,154-184) this path has no request-generation guard. An older sub-cap response would enter the new helper's authoritative !truncated branch (apps/desktop/src/store/session.ts:224-226, PR right side) and overwrite newer cached rows.

Suggested changes

  • Fence messaging refresh publications with a request ref/counter, including setMessagingTruncated, and add a reversed-resolution test for overlapping requests.

Automated hermes-sweeper review.

@@ -105,11 +106,16 @@ export function useSessionListActions({ profileScope }: UseSessionListActionsArg
// Drop any non-messaging source the broad exclude didn't catch (custom
// sources) — those stay in local recents, not a platform section.
const rows = result.sessions.filter(s => isMessagingSource(s.source))
const truncated = result.sessions.length >= MESSAGING_SECTION_LIMIT

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please fence this publication with a messaging-refresh request counter, as refreshSessions does. The visible 10-second poll can overlap a newer refresh or platform-page update; an older sub-cap response would make reconcileMessagingRefresh(..., false) return its stale incoming page and overwrite the newer cache.

@teknium1 teknium1 added the sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform label Jul 18, 2026
@PINKIIILQWQ
PINKIIILQWQ force-pushed the fix/desktop-messaging-refresh-preserve-pages branch from d6d3570 to aef4177 Compare July 19, 2026 09:03
@PINKIIILQWQ

PINKIIILQWQ commented Jul 19, 2026

Copy link
Copy Markdown
Contributor Author

Addressed all review feedback and rebased onto latest main (238 new commits including a test-infrastructure refactor):

  1. Reconciled refreshreconcileMessagingRefresh prevents the background seed from collapsing platform pages the user has expanded. A truncated seed replaces rows but can't reduce a platform's loaded count.
  2. Request-generation guardrefreshMessagingRequestRef (mirroring refreshSessionsRequestRef pattern) drops stale fetch results that resolve after a newer call.
  3. Tests — unit tests for reconcileMessagingRefresh (3 cases) + overlapping-request integration test for the guard. 34/34 messaging+session tests passing.

@PINKIIILQWQ
PINKIIILQWQ force-pushed the fix/desktop-messaging-refresh-preserve-pages branch from aef4177 to 839eb4c Compare July 19, 2026 09:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants