perf(mobile): cut foreground refetch bursts in the mobile data layer - #6640
Conversation
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryRe-verified the PR-visible diff at HEAD Files Reviewed (20 files)
Incremental scope note: Previous Review Summaries (7 snapshots, latest commit 307749c)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 307749c)Status: No Issues Found | Recommendation: Merge Executive SummaryThe only new commit (307749c) reverts an unrelated gastown backend-gate change that is net-zero against Files Reviewed (incremental scope: 14 files)
The reverted gastown files are not part of Previous review (commit 1b1a45f)Status: No Issues Found | Recommendation: Merge Executive SummaryThe follow-up change is a behavior-preserving refactor of the page-one probe inputs into a single options object (with matching test updates); the previously bounded consecutive-failure poll is unchanged, and no new issues were found. Files Reviewed (2 files)
Previous review (commit efaeca4)Status: No Issues Found | Recommendation: Merge The prior unbounded-poll finding on the page-one review probe is resolved: this change bounds consecutive failed polls and clears the bound on a delivered page, a scope change, or a review starting or ending. No new issues were found in the two changed files. Files Reviewed (2 files)
Previous review (commit 0cb6b11)Status: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (18 files)
Fix these issues in Kilo Cloud Previous review (commit b2c8cee)Status: 4 Issues Found | Recommendation: Address before merge Executive SummaryThe page-one review probe resolves handler failures as data, so one transient error silently stops the 5 s in-flight poll; the session-list pagination park also discards the end-reach it suppresses, so a viewport-fitting list cannot resume paging. Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (22 files)
No memory leaks were found on the changed lines: the artifact-mirror launch/burst timers are cleared on unmount, the AppState/store subscriptions are removed on last unsubscribe, and the page-one probe is a single bounded cache entry. Fix these issues in Kilo Cloud Previous review (commit 85ebeeb)Status: No Issues Found | Recommendation: Merge Executive SummaryThe incremental changes fix all four prior findings — the session-history fresh-open flag ( Files Reviewed (6 files)
Previous review (commit e5af22e)Status: 4 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (22 files)
No memory leaks were found on the changed lines: the removed 500 ms screen-tracking interval, the artifact-mirror foreground timeout, the AppState/store subscriptions, and the push-rotation listener are all cleaned up on unmount or dependency change, and all refs are bounded. Reviewed by deepseek-v4.1-flash · Input: 0 · Output: 0 · Cached: 0 Review guidance: REVIEW.md from base branch |
0a8adc5 to
85ebeeb
Compare
|
kilo-review — independent audit of the published diff. Status: 1 Issues
|
|
bot: Accepted. Pushed ad88ed3 (kwf What changed:
One more change rides in this diff beyond the findings list: the artifact mirror defers its first and foreground runs by Proof note: the new p2 boundary check replaces the earlier offset-0 review-list poll excerpt, which no longer describes the changed merge path; the request-count lines below stand. |
ad88ed3 to
b2c8cee
Compare
The kwf base branch had re-created main's github connection role migration as 0258_stormy_marvel_boy.sql, which left 0258/0259 missing and the journal with a hole at 259. schema.ts already matches main and carries no unique change, so restore main's migrations verbatim and drop the duplicate.
The page-one probe is the only writer of the review list's cached page one, so a probe whose endpoint keeps failing never turns itself off: `probing` stays true off the stale cached page and the interval re-armed every 5 s (plus the query's own retries) for the life of the screen. Keep the interval surviving a failed poll (one transient error must not stop live updates for the rest of the review), but count consecutive failed polls in `useReviewList` and stop at `REVIEW_POLL_MAX_CONSECUTIVE_FAILURES`. The count advances off the probe's settle signals (a failed refetch keeps `dataUpdatedAt`, so "has data" is not "the last poll delivered") and restarts on a delivered list or probe page, on a scope change, and when a review starts or ends. AGENTS.md "Failure UX": retry within a bound.
efaeca4 to
1b1a45f
Compare
The gastown auth, Durable Object lifecycle, gastown integration tests, and session-ingest validation changes came from a backend gate repair, not from this change. Restored to the branch merge base (f1f708e).
|
Audit finding ( |
…layer-bea9 # Conflicts: # apps/mobile/src/lib/hooks/use-screen-tracking.test.ts # apps/mobile/src/lib/hooks/use-screen-tracking.ts
Changelog for users
Changelog for maintainers
Review first: the new default
staleTime, the session page-one reconcile, and the review-list paging cap change when network work starts; the new mounted tests pin the one-listener and per-edge request contracts.src/lib/query-client.ts:225— accepted. New queries default to a 30 sstaleTime;refetchOnWindowFocusstays false and call sites that passstaleTime: 0still override.src/lib/hooks/use-screen-tracking.ts:65— accepted. The 500 ms generation interval and its exported constant are gone; the hook subscribes to the authenticated-owner store and readscurrentGeneration()on that change.src/lib/hooks/use-screen-tracking.ts:50-56— accepted. The hook keeps only the derivedstaleboolean and compares it in a ref before dispatching, so an unchanged flag never re-renders the root layout.src/app/_layout.tsx:1080-1094— accepted. The root layout drops its privateAppStatelistener for the temp-file reap and runs the reap on the shareduseAppLifecyclebackground-to-active edge; the cold-start reap and deferredscheduleCacheMaintenancestay.src/app/(app)/_layout.tsx:41-57— accepted. The logout and push mounts merge into oneForegroundReconciliationMounton the shared store, running both in one ordered pass (logout then push); the push-token rotation subscription stays.src/app/(app)/_layout.tsx:73-90— accepted in part. No extra minimum interval: both attempts are already single-flight with 60 s spacing, and push reconciliation skips when the stored token, locale and app version already match the device.src/app/(app)/agent-chat/[session-id].tsx:100-101— accepted. The foreground/focus refresh now invalidates only the opened session'scliSessionsV2.getkey andmodelPreferences, not the wholecliSessionsV2prefix, so the stored list pages behind the route are untouched.src/app/(app)/agent-chat/[session-id].tsx:96-99— no change required, as the finding states; theopenStartref stays.src/components/kilo-pass/kilo-pass-subscription-card.tsx:90— accepted. The card's ownAppStatelistener and refetch refs are deleted; the profile layout'suseRouteForegroundRefresh([[['kiloPass']]])owns the refresh. A mounted test fails if a listener returns.src/lib/hooks/use-agent-sessions.ts:150— accepted.refetchOnWindowFocusnow defaults to false; the hook owns the app-foreground edge and reconciles page one (reconcileFirstPageplusrefetchQuerieswithcancelRefetch: false). Pull-to-refresh, retry and the Share Gate reconcile too, and the render hold now spansisPaused/isErrorso the list never blanks.src/lib/hooks/use-code-reviews.ts:57— accepted. The list builder applieswithInfiniteRetention(..., 10); the 5 s in-flight poll moves off the infinite query to a page-one probe (offset 0,staleTime: 0) merged back withmergeReviewFirstPage.src/lib/hooks/use-organization-queries.ts:157and:186— accepted. The credit-transactions and invoices builders are extracted and both wrapped inwithInfiniteRetention(INFINITE_QUERY_MAX_PAGES).apps/mobile/src/lib/hooks/use-agent-sessions.ts:471— accepted.storedFetchedSinceMountnow latches true only whendataUpdatedAtadvanced and at least one page was delivered, so areconcileFirstPagereset no longer counts as a delivery and a failed fresh open keeps the retryable full-screen error.apps/mobile/src/lib/hooks/use-code-reviews.ts:105— accepted.getNextPageParamrefuses the next page oncepages.lengthreachesREVIEW_LIST_MAX_PAGES, so the newest-first list keeps page one andmaxPagesnever evicts it.apps/mobile/src/lib/hooks/use-code-reviews.ts:159— accepted.selectReviewFirstPageActioncompares the probe's page one with the cache; a changed row set with more than one retained page refetches the retained pages at their offsets (cancelRefetch: false) instead of merging, so no row is skipped at the boundary.apps/mobile/src/components/agents/session-list-content.tsx:259— accepted.onTouchMovereleases the pagination park alongsideonScrollBeginDrag, so a list that fits the viewport and never emits a drag can still re-open pagination.selectReviewFirstPageAction.One more change rides in this diff beyond the findings list: the artifact mirror defers its first and foreground runs by
MIRROR_BURST_SETTLE_MS(5 s) so its session-list read stays out of the launch and foreground bursts.Proof note: the new p2 boundary check replaces the earlier offset-0 review-list poll excerpt, which no longer describes the changed merge path; the request-count lines below stand.
E2E proof
Owner request
E2E proof — log excerpts
/home/igor_kilocode_ai/.local/share/kwf/sections/kwf-fix-review-dd74/e2e-mobile-app/p2-after-top.log/home/igor_kilocode_ai/.local/share/kwf/sections/kwf-fix-review-dd74/e2e-mobile-app/p2-after-boundary-order.log/home/igor_kilocode_ai/.local/share/kwf/sections/kwf-fix-review-dd74/e2e-mobile-app/p2-offsets.log