Uh oh!
There was an error while loading. Please reload this page.
fix(desktop): cache external-session catalog per selection so switching sources shows loaded rows instantly - #3905
Conversation
…ng sources shows loaded rows instantly Switching import source (or toggling the archived filter / search) blanked the list and flashed the full-width 'reading external conversations' spinner on every switch, including returning to a source already loaded, because loadCatalog unconditionally reset the catalog and loading flag with no per-selection caching. Cache the last loaded CatalogState per (adapterId, includeArchived, search): a cache hit renders instantly and refreshes in the background instead of blanking; misses keep the spinner. The poll now observes rather than claims the request generation so it can never strand an in-flight load's spinner. Adds source-switching tests (harness now supports multiple adapters). Generated-by: Claude Code
3d9b5c9 to
fbf120fCompare
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for addressing the repeated blank-loading state when switching external-session sources. Keeping the Host as the catalog authority while caching only the renderer projection is the right boundary, and the selection key plus request generation model is appropriately scoped.
I left one inline P2 for a recoverable stale-loading case. It does not risk catalog correctness or persisted data, so I am approving the overall direction, but clearing the obsolete loading state would make the cache behavior complete.
Reviewed with Codex and an independent @Reviewer agent. I verified the exact head, cache ownership, selection and request generations, switching and import-recovery paths, and passing CI.
中文对照
谢谢你处理切换外部 Session 来源时反复出现空白加载状态的问题。Host 仍然是 catalog authority，renderer 只缓存展示 projection，这个边界是正确的；selection key 和 request generation 的范围也比较合适。
我留了一条 P2 行内评论：切回缓存 selection 时，旧的 loading 状态可能没有被清理。它不会影响 catalog 正确性或持久化数据，因此我认可整体方向；不过清理过期 loading 状态可以让缓存行为真正闭环。
本次审查使用了 Codex 和一个独立的 @Reviewer 子代理；我核对了精确 head、缓存职责、selection 与 request generation、切换和导入恢复路径，以及 CI 结果。
Uh oh!
There was an error while loading. Please reload this page.
… hit Restoring a cached catalog only reset the rows and the recovery banner, so a search or pagination request still in flight from the previous selection kept its loading flag. That older generation can never reach its own `finally` reset once a newer request supersedes it, and this hit's background refresh may not have landed yet, so the full-page spinner or a disabled Load More could strand over otherwise-complete cached rows indefinitely. Clear both `catalogLoading` and `loadingMore` when publishing a non-append cache hit so the instant, no-spinner contract holds regardless of a pending prior request or a slow background refresh. Adds two source-switching tests: a pending search returning to a cached term (reachable via the search box, the only control not disabled during a load) and a pending Load More when switching to a cached source. Both fail without the change (19/21) and pass with it (21/21). Addresses the P2 review comment on apache#3905. Generated-by: Claude Code
…d selection Revisiting a cached selection restored its full CatalogState — which can be several pages deep after Load More — but the background refresh that followed requested only the first page (no cursor) and then replaced the cache and the view with that single page. So a multi-page source, revisited, flashed all its pages and then snapped back to page one once the refresh landed, silently dropping everything the user had paged in. Refresh a cache hit through the existing `readCatalogWindow`, seeded with the cached `sessions.length`, so it re-reads the entire loaded window instead of just page one. The uncached first-page load and the Load More append paths are unchanged. Adds a source-switching test: a two-page source, revisited, keeps both pages after the refresh (re-reading every page). It fails without the change (21/22) and passes with it (22/22). Addresses the P2 pagination-shrink review comment on apache#3905. Generated-by: Claude Code
…d imports Two review-driven follow-ups to the per-selection catalog cache: - Revisiting a selection whose cached rows still show an import in flight started a background readCatalogWindow *and* let the 1s import poll fire. Both ran under the same request generation, so a pre-import page read started by the revisit could land after a newer poll result and snap "Imported once" back to "Importing…". The revisit now defers to the poll — the single refresher for an importing selection — and issues no second read. - A successful unknown-outcome recovery only wrote the recovered rows back to the cache when its selection was still current. If the user had switched source or filter, the original selection kept its stale pre-import cache and, on return, showed an "Import" button instead of "Import again" until a later refresh — inviting a duplicate import. Recovery now always refreshes that selection's cache; only setCatalog and the generation bump stay gated on the selection still being current. Adds two source-switching tests (revisiting an importing source issues no second read; a recovered import is reflected in its cache after switching away). Both fail without their fix (23/24) and pass with it (24/24). Addresses the two P2 review comments on apache#3905. Generated-by: Claude Code
Recovery only pushed the recovered rows to the screen when the current catalog selection still had the exact generation captured at import time. Navigating away and back to the same source/filter/search (A→B→A) lands on the same selection with a newer generation, so the check refused to publish: the cache was refreshed but the visible rows stayed pre-import, and once activeImport cleared the row showed a clickable "Import" even though the success banner was up — persisting until a slow background refresh caught up. Match the current selection by its actual tuple (adapterId + includeArchived + search) instead of the import-time generation, so recovery publishes to the view — and bumps requestGeneration to retire any in-flight revisit/poll read — whenever the user is looking at that selection. The now-unused catalogSelectionGeneration field is dropped from ImportAttempt. Adds an A→B→A-then-recovery test; it fails on the generation check (24/25) and passes on the tuple match (25/25). Addresses the follow-up P2 review comment on apache#3905. Generated-by: Claude Code
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for completing the per-selection cache behavior. Reviewed at exact head abdc7038e84510f01f5ba772c1320f19ebd0aa4c: the earlier stale-loading case is closed, and the follow-up coverage now includes the loaded page window, import polling, recovery cache writes, and A→B→A publication without moving catalog authority out of the Runtime Host. The renderer remains a bounded projection cache rather than a second durable source of truth. Exact-head CI is green, the before/after behavior was manually accepted, and I found no remaining issue that should delay this fix.
Review analysis was assisted by Codex and independent reviewer agents. I verified the exact head, production cache/generation paths, prior finding, CI, and manual UI decision, and I own this review.
中文对照
感谢你把 per-selection cache 行为收完整。我审查了精确 head abdc7038e84510f01f5ba772c1320f19ebd0aa4c：之前的 stale-loading 问题已经闭合，后续覆盖也包含已加载分页窗口、import poll、recovery cache 写入和 A→B→A 的状态发布，同时 catalog authority 仍留在 Runtime Host。Renderer 只是有上限的展示缓存，没有变成第二个持久化事实源。Exact-head CI 已通过，前后交互也已人工确认，没有发现需要延迟合并的问题。
本次分析由 Codex 和独立 reviewer 子代理协助；我核验了精确 head、生产 cache/generation 路径、旧问题、CI 和人工 UI 结论，并对本次 Review 负责。
Uh oh!
There was an error while loading. Please reload this page.
Three renderer files tracked by the architecture ledger drifted after the ledger was last regenerated in apache#4249, each landing without a paired ledger update: - app-shell-chat-actions.ts nonTriviaTokens 4363 -> 4376 (apache#4246) - app-shell-session-events.ts nonTriviaTokens 2931 -> 3042 (apache#4232) - import-tasks-settings-page.tsx hookCalls useEffect 4->5, useRef 3->5 (apache#3905) CI first went red on apache#4246 and accumulated the other two, wedging the 'Check renderer architecture' job on main and every branch cut from it. Regenerate the snapshot (--write) to match the current source; no runtime code changes. Generated-by: Claude Code
…pache#3905) Cache the loaded external-session catalog per source, archive filter, and search selection so revisits render immediately while Runtime Host remains the catalog authority and background refreshes stay generation-safe.
Summary
Switching the import source — or toggling the archived filter / editing the search, and including returning to a source already loaded — blanked the list and flashed the full-width "Reading external conversations…" spinner on every switch, because
loadCatalogunconditionally reset the catalog and loading flag with no per-selection cache.This caches the last loaded
CatalogStateper(adapterId, includeArchived, search): a cache hit renders the rows immediately and refreshes in the background, while a miss keeps the existing spinner. The background import poll now observes the request generation instead of claiming a new one, so it can never strand an in-flight load'scatalogLoadingreset.Five follow-ups from review, all on the per-selection cache: (1) a cache hit now clears any stale
catalogLoading/loadingMoreleft by a superseded search or pagination request, so the spinner or a disabled Load More can't strand over otherwise-complete cached rows; (2) it refreshes throughreadCatalogWindowseeded with the cached page count, so a multi-page selection keeps every loaded page instead of snapping back to page one; (3) revisiting a selection whose cached rows are still importing defers to the 1s import poll instead of starting a second concurrent read, so a stale pre-import page can't land on top of a newer poll result; (4) a successful unknown-outcome recovery always refreshes its selection's cache, even after the user switches away, so returning shows "Import again" rather than a stale "Import" that invites a duplicate; and (5) recovery matches the current selection by its tuple (source + archived filter + search) rather than the import-time generation, so navigating away and back (A→B→A) still publishes the recovered rows to the screen instead of leaving a clickable "Import" until a slow refresh catches up.Fixes#3904
Verification
tscfortsconfig.preload.json,tsconfig.main.json,tsconfig.renderer.json— all clean.biome checkon both changed files — clean.node --test dist/main/__tests__/import-tasks-settings-page.test.js— 25/25 pass (15 existing + 10 source-switching cases).shows a previously-loaded source instantly with no spinner,does not let a stale background refresh overwrite a newer source selection), the stale-loading reset (clears the reading spinner when a pending search returns to a cached term,clears a pending Load More lock when switching back to a cached source), the full-window refresh (keeps every loaded page when a revisited multi-page source refreshes), the concurrent-refresh guard (does not start a second catalog read when revisiting a still-importing source), the recovery cache write (updates the cache for a recovered import even after switching away), and the recovery tuple match (publishes a recovered import to the current view after leaving and returning to its source); restoring each returns the suite to green (25/25).format:check/knip, fullnpm run build, and the cross-workspace suite.tsconfig.storybook.jsonreports one pre-existing, unrelated error (UsageRequestLog.sessionNameinstories/settings/settings-pages.stories.tsx) that exists on the base commit and this PR does not touch.Review focus (UI evidence)
Before/after of revisiting an already-loaded import source (switch to another source and back). The Electron window can't be screenshotted headlessly, so these are rendered from a temporary Storybook story of the real
ImportTasksSettingsPage(settings-pages.stories.tsx) driven with a two-source bridge whose revisit read is left pending — on the pre-fix component it blanks and shows the spinner; on this PR it serves the cached rows. The temporary story is not committed.main)Before: the list blanks and the full-width "Reading external conversations…" spinner shows on the revisit (the source/filter controls dim too). After: the previously-loaded rows appear instantly with no spinner. First-time loads still show the spinner. To see it live: run the desktop app, open Settings → Activity → Import Tasks with ≥2 sources, and switch back and forth.
AI use
Select exactly one:
Tool(s) and scope: Claude Code (Claude Opus) — root-cause investigation, the per-selection cache implementation, the five review-driven follow-ups (stale-loading reset, full-window refresh, concurrent-refresh guard, recovery cache write, and recovery tuple match), and the added tests. Reviewed and submitted by the human contributor of record; commit carries a
Generated-by: Claude Codetrailer.Checklist
Does this PR entail a change in behavior?