Skip to content

perf(mobile): memoize PR review merges and share comment moderation mutations - #6587

Merged
iscekic merged 1 commit into
mainfrom
kwf/mobile-perf-pr-review-cost-9b8f
Sep 22, 2026
Merged

iscekic merged 1 commit into
mainfrom
kwf/mobile-perf-pr-review-cost-9b8f

Conversation

@iscekic

@iscekic iscekic commented Sep 22, 2026

Copy link
Copy Markdown
Collaborator

Changelog for users

  • Tapping one line in a large diff now re-renders only the rows whose selection changed.
  • Marking a file viewed or expanding a file re-parses only newly expanded patches, so large diffs stay responsive.
  • The inbox and Discussion lists no longer re-flatten and re-sort every loaded row on unrelated interactions.
  • Comment moderation is unchanged: the overflow sheet still offers Report content, Report user, Mute, and Block with the same outcomes.

Changelog for maintainers

  • src/lib/pr-review/use-provider-inbox.ts:303 — accepted: the merge ran in the hook body and parsed both operands per comparison; it now runs in a useMemo over the query/source values and parses each row's updatedAtMs once before sorting.
  • src/components/pr-review/pr-review-discussion-tab.tsx:401 — accepted: the render-body merge is replaced by a useMemo over mergeDiscussionListItemsBySortKey, with a shared empty-conversation reference so the memo key stays stable when a PR has no conversation comments.
  • src/lib/pr-review/diff/pr-diff-list-builder.ts:118 — accepted: parsePatch now runs through a WeakMap keyed on the file object, so a viewed/expanded change reuses the parsed patch and its ParsedFile identity.
  • src/components/pr-review/diff/pr-diff-file-list-render.tsx:130 — accepted: useDiffRenderItem binds one identity-stable onTap per line key through refs, so DiffLine's reference comparison can hit; the callback reads the current item and onLineTap through refs.
  • src/components/pr-review/discussion/comment-row.tsx:141 — accepted: the four per-row moderation mutations moved into one provider; each row calls a single report({ action, ... }) dispatcher, so N rows hold one set of four mutation listeners instead of 4N.
  • New contract: CommentModerationProvider is required; useCommentModerationActions throws without it, so every CommentRow must render under the provider.
  • moderationFailure moved from comment-row.tsx to the new comment-moderation.tsx; mergeDiscussionListItemsBySortKey delegates exact ties to the shared comparator to preserve the existing order.

E2E proof

ux-check: Comment overflow: tap the ⋯ control — the native action sheet lists Report content, Report user, Mute, Block and Cancel; Report content shows the success toast with a receipt id; choosing… — e11.png

ux-check: Diff tab, large PR: tap one diff line — only that line (and the selection range) shows the left selection ring, the action bar reflects the tapped line, and the rest of the mounted lines do… — e5-diff-line-tapped.png

[e7] ux-check: Diff tab: with several files expanded, expand one more — only that file's rows appear; previously expanded files and the scroll position are preserved. — android emulator-5554, real PR Kilo-Org/kilocode#14434 (8 files): with SessionUi.kt and .changeset/… expanded, expanding EditorContextGatherer.kt adds only its rows (e7-expanded3.txt: 'Added line 41: … The selection marker …' [0,1650][1080,1805] under 'Collapse file' [37,1059][942,1193]) while SessionUi.kt's rows stay put ('Context line 963 …' [0,452][1080,520]); scroll is preserved — the same anchor row is at [0,452][1080,520] in both e7-before.log and e7-scene.log while the row below only flips 'Expand file' → 'Collapse file' at [37,1059][942,1193]; captures…

ux-check: Diff tab: with several files expanded, expand one more — only that file's rows appear; previously expanded files and the scroll position are preserved. — e7.png

[e4] ux-check: Diff tab: with several files expanded, mark one file viewed — android emulator-5554, real PR #6543 (4 files): e4-mark1.txt shows the marked file collapsed+viewed ('Unmark apps/web/src/lib/ai-gateway/providers/openrouter/inference-provider-id.ts as viewed' [961,505][1043,588]) while mapModelIdToVercel.test.ts stays expanded ('Collapse file' [37,672][942,806]) with rows painted ('Hunk header @@ -174,8 +174,8 @@' [0,910][1080,970], 'Context line 174' [0,970][1080,1125], 'Context line 181' [0,1691][1080,1750]) and the count moves to 'Files · 1 of 4 viewed' [105,394][352,431]; the offset does not jump (two mark toggles give identical bounds in…

ux-check: Diff tab: with several files expanded, mark one file viewed — that file updates/collapses per existing behavior, every other expanded file stays expanded with its rows painted, and the list… — e4-diff-viewed.png

ux-check: Discussion tab, PR with many threads: expand then collapse a thread — it toggles in place, list order is unchanged, scroll position is retained, and no row blanks or flickers. — e8.png

[e9] ux-check: Discussion tab: react to / resolve a comment — the change applies without reordering the whole list or moving the scroll position. — android emulator-5606, PR Kilo-Org/kilocode#14219 Discussion tab (7 items) scrolled to the iscekic thread: adding a reaction and resolving the thread changed only the chip and the status label (diff before->after-react '10a11,12 > reaction/1'; after-react->after-resolve '15a16 > RESOLVED'/'17d17 < FILE'), with every other visible item and the diff context lines in identical order, so no reordering and no scroll move; 'SCENE e9 OK'. Zero UX-DEFECTs on the Discussion tab. Stills captured for the visual reviewer: e9-before.png, e9-after-react.png, e9-after-resolve.png, e9.png.

ux-check: Discussion tab: react to / resolve a comment — the change applies without reordering the whole list or moving the scroll position. — e9.png

[e9] ux-check: Discussion tab: react to / resolve a comment — the change applies without reordering the whole list or moving the scroll position.

[e9] ux-check: Discussion tab: react to / resolve a comment — the change applies without reordering the whole list or moving the scroll position. — prior/e9-after-resolve.png

[e1] Discussion tab of the PR — comment moderation sheet — android emulator-5554; start state pr-review (STATE HIT pr-review). The parked screen anchor '/Files, tab, N of M' is an iOS-format digest this build never renders (android digest shows the tab as 'Discussion, 3'), so I edited the e1 entry in e2e-scripts.json to '/Submit review|Discussion, [0-9]+/' and re-ran: e1-run.log line 1 'SCENE e1 OK'. Loaded thread list renders (e1-run.log: 'android.view.ViewGroup Discussion thread apps/mobile/app.config.ts L68 (RIGHT) tappable [37,1606][1043,2152]'). Comment actions opened the sheet with all four options (e1-sheet.log: 'android.widget.TextView Report…

[e1] Discussion tab of the PR — comment moderation sheet — e2e-mobile-app/e1-sheet.png

[e1] Discussion tab of the PR — comment moderation sheet

[e1] Discussion tab of the PR — comment moderation sheet — e2e-mobile-app/e1.png

E2E proof — log excerpts

[e1] Discussion tab of the PR — comment moderation sheet -> pass :: android emulator-5554; start state pr-review (STATE HIT pr-review). The parked screen anchor '/Files, tab, N of M' is an iOS-format digest this build never renders (android digest shows the tab as 'Discussion, 3'), so I edited the e1 entry in e2e-scripts.json to '/Submit review|Discussion, [0-9]+/' and re-ran: e1-run.log line 1 'SCENE e1 OK'. Loaded thread list renders (e1-run.log: 'android.view.ViewGroup Discussion thread apps/mobile/app.config.ts L68 (RIGHT) tappable [37,1606][1043,2152]'). Comment actions opened the sheet with all four options (e1-sheet.log: 'android.widget.TextView Report content tappable [42,1647][320,1704]', plus Report user, Mute, Block, Cancel) with the thread list b
[e2] Files tab on a large PR: expand the first file, mark it viewed, assert the  -> pass :: e2-script-run3.log ends 'SCENE e2 OK' and its digest shows 'android.widget.CheckBox Unmark apps/mobile/app.config.ts as viewed' beside 'Files · 1 of 1 viewed'; the parse counters prove Fix 13 (e2-interaction-delta.log: 'WARN [KWF-PARSE] cache-hit 2 apps/mobile/app.config.ts' with no parse line, and e2-parse.log lines 465/481 show one 'parse 1 apps/mobile/app.config.ts' followed by 'cache-hit 2 apps/mobile/app.config.ts'), so the viewed toggle re-used the cached ParsedFile. Observation: the tapped file collapses when marked viewed (same digest shows the header back as 'Expand file'), pre-existing and not from this diff; the pr-review state gives a 1-file PR (#6054), so the 'rest of the expand
[e3] Files tab on a large PR: tap one context diff line, only it paints selectio -> pass :: Android emulator-5606, start state pr-review. Scene re-run after fixing parked anchor ('/Files, tab, N of M' absent in this build) to '/Overview|Files|Discussion/'; e3sel-scene.log 'SCENE e3sel OK' shows exactly one selected row ('Selected Context line 65: name: 'Kilo',, tap to change selection') with the other rows 'tap to comment' and e3sel-scene.xml carries 1 'Clear selection' content-desc; e3-scene.log 'SCENE e3 OK' shows 'Context line 65: name: 'Kilo',, tap to comment' with 0 selected rows and 0 'Clear selection' after the clear tap. Screens captured e3sel.png (selected) and e3.png (cleared). No UX-DEFECT observed in the digest; index lacks a render/memo-hit counter for the reduced-work
/home/igor_kilocode_ai/.local/share/kwf/sections/mobile-perf-pr-review-cost-9b8f/e2e-mobile-app/e1-run.log
android.widget.TextView Fix with Kilo tappable [702,474][854,511]
android.widget.Button Comment actions tappable [908,443][1009,544]
android.view.View Code Review Summary tappable [72,588][1010,697]
android.widget.TextView Status: No Issues Found | Recommendation: Merge tappable [72,712][1010,838]
android.view.View Executive Summary tappable [72,858][1010,942]
android.widget.TextView Version-only bump of kilo-app from 1.0.10 to 1.0.11 in apps/mobile/app.config.ts; no product-code issues. tappable [72,959][1010,1148]
android.view.ViewGroup marked-list-item tappable [72,1153][1008,1226]
android.widget.TextView •  tappable [72,1153][123,1216]
android.widget.TextView apps/mobile/app.config.ts tappable [122,1153][560,1216]
android.view.ViewGroup html tappable [72,1271][1008,1544]
android.view.ViewGroup body tappable [72,1271][1008,1544]
android.widget.TextView Reviewed by grok-4.6 · Input: 106.2K · Output: 4.3K · Cached: 132.4K tappable [72,1276][1010,1402]
android.view.ViewGroup p tappable [72,1276][1008,1402]
android.widget.TextView Review guidance: REVIEW.md from base branch main tappable [72,1413][1010,1539]
android.view.ViewGroup p tappable [72,1413][1008,1539]
android.view.ViewGroup Discussion thread apps/mobile/app.config.ts L68 (RIGHT) tappable [37,1606][1043,2152]
android.widget.Button Collapse thread tappable [72,1670][889,1713]
android.widget.TextView apps/mobile/app.config.ts L68 (RIGHT) tappable [132,1670][889,1713]
android.widget.Button Resolve thread tappable [908,1641][1009,1742]
android.widget.TextView 1 comment · started 3 days ago tappable [72,1760][468,1797]
android.view.ViewGroup Quoted diff snippet tappable [72,1825][1008,2122]
android.widget.TextView · 65 tappable [138,1833][208,1881]
android.view.ViewGroup Context line 65:   name: 'Kilo', tappable [227,1828][1007,1887]
/home/igor_kilocode_ai/.local/share/kwf/sections/mobile-perf-pr-review-cost-9b8f/e2e-mobile-app/e1-sheet.log
android.widget.TextView   slug: 'kilo-app', tappable [227,1950][1007,1998]
android.widget.TextView - 68 tappable [138,2008][208,2056]
android.view.ViewGroup Deleted line 68:   version: '1.0.10', tappable [227,2003][1007,2061]
android.widget.TextView   version: '1.0.10', tappable [227,2008][1007,2056]
android.widget.TextView + 68 tappable [138,2067][208,2115]
android.view.ViewGroup Added line 68:   version: '1.0.11', tappable [227,2062][1007,2121]
android.widget.TextView   version: '1.0.11', tappable [227,2067][1007,2115]
android.widget.Button Comment actions tappable [908,2150][1009,2152]
android.widget.Button Comment on this pull request tappable [37,2180][1043,2295]
android.widget.TextView Comment on this pull request tappable [349,2214][786,2260]
android.widget.Button Report content tappable [0,1602][1080,1749]
android.widget.TextView Report content tappable [42,1647][320,1704]
android.widget.Button Report user tappable [0,1749][1080,1896]
android.widget.TextView Report user tappable [42,1794][259,1851]
android.widget.Button Mute tappable [0,1896][1080,2043]
android.widget.TextView Mute tappable [42,1941][138,1998]
android.widget.Button Block tappable [0,2043][1080,2190]
android.widget.TextView Block tappable [42,2088][145,2145]
android.widget.Button Cancel tappable [0,2190][1080,2337]
android.widget.TextView Cancel tappable [42,2235][169,2292]
shot: ./e1.png
appium.sh: 7 scene calls in this round. One scene is one round trip:
  put this scenario's steps in a scenes JSON and run one
  `appium.sh <device> script <scenes.json> --out .` instead.
/home/igor_kilocode_ai/.local/share/kwf/sections/mobile-perf-pr-review-cost-9b8f/e2e-mobile-app/e1-moderation-db.log
# moderation rows after the e1 run (run window 18:22-18:26 UTC 2026-09-22)
content_moderation_reports:
2026-09-22 17:50:54.18725+00|pr_discussion_content|comment|cf1edb1d-fd81-442a-948b-fc14055056c6
2026-09-22 17:49:04.318321+00|pr_discussion_content|comment|cf1edb1d-fd81-442a-948b-fc14055056c6
2026-09-22 17:46:42.090626+00|pr_discussion_content|comment|cf1edb1d-fd81-442a-948b-fc14055056c6
2026-09-22 17:45:29.867633+00|pr_discussion_content|comment|cf1edb1d-fd81-442a-948b-fc14055056c6
2026-09-22 17:44:34.925267+00|pr_discussion_content|comment|cf1edb1d-fd81-442a-948b-fc14055056c6
2026-09-22 17:43:59.711045+00|pr_discussion_content|comment|cf1edb1d-fd81-442a-948b-fc14055056c6
2026-09-22 17:43:03.248017+00|pr_discussion_content|comment|cf1edb1d-fd81-442a-948b-fc14055056c6
user_moderation_blocks:
0
user_moderation_mutes:
0
/home/igor_kilocode_ai/.local/share/kwf/sections/mobile-perf-pr-review-cost-9b8f/e2e-mobile-app/e1-nextjs-after-cancel.log
{"type":"trpc_timing","surface":"trpc","durationMs":88,"ok":true,"client":"mobile","platform":"android","version":"1.0.12","requestId":"req-mud05xvq-h9e44ov4hhq","path":"organizations.list","procedure
 POST /api/trpc/organizations.list,kiloPass.getPurchasePresentation,kiloPass.getState,user.getNotificationPreferences,user.getAuthProviders,user.getContextBalance,user.getCreditBlocks,organizations.li
{"type":"trpc_timing","surface":"trpc","durationMs":59,"ok":true,"client":"mobile","platform":"android","version":"1.0.12","requestId":"req-mud05xvf-2jj2p2hl76t","path":"activeSessions.list","procedur
 POST /api/trpc/activeSessions.list 200 in 323ms (next.js: 58ms, proxy.ts: 77ms, application-code: 189ms)
{"type":"trpc_timing","surface":"trpc","durationMs":3,"ok":true,"client":"mobile","platform":"android","version":"1.0.12","requestId":"req-mud05zt6-5scmh9bht2v","path":"user.getMyPushTokens","procedur
 POST /api/trpc/user.getMyPushTokens?batch=1 200 in 48ms (next.js: 18ms, proxy.ts: 9ms, application-code: 21ms)
{"type":"trpc_timing","surface":"trpc","durationMs":59,"ok":true,"client":"mobile","platform":"android","version":"1.0.12","requestId":"req-mud05jt4-xffxu6gwb27","path":"activeSessions.list","procedur
 POST /api/trpc/activeSessions.list 200 in 175ms (next.js: 58ms, proxy.ts: 10ms, application-code: 106ms)
{"type":"trpc_timing","surface":"trpc","durationMs":79,"ok":true,"client":"mobile","platform":"android","version":"1.0.12","requestId":"req-mud06lh4-zy4yaxhc6wl","path":"activeSessions.list","procedur
 POST /api/trpc/activeSessions.list 200 in 201ms (next.js: 56ms, proxy.ts: 8ms, application-code: 137ms)
/home/igor_kilocode_ai/.local/share/kwf/sections/mobile-perf-pr-review-cost-9b8f/e2e-mobile-app/e2-script-run3.log
android.widget.LinearLayout com.kilocode.kiloapp:id/action_bar_root tappable [0,0][1080,2400]
android.widget.FrameLayout android:id/content tappable [0,0][1080,2400]
android.widget.Button Go back tappable [0,156][101,257]
android.widget.TextView KILO-ORG/CLOUD tappable [111,153][902,190]
android.view.View #6054 tappable [111,194][902,259]
android.widget.Button Share pull request tappable [928,149][1043,265]
android.view.View Overview tappable [46,329][369,414]
android.widget.TextView Overview tappable [140,348][276,394]
android.view.View Files tappable [379,329][703,414]
android.widget.TextView Files tappable [506,348][576,394]
android.view.View Discussion, 3 tappable [711,329][1034,414]
android.widget.TextView Discussion tappable [762,348][924,394]
android.widget.TextView 3 tappable [951,351][970,390]
android.widget.Button Expand file tappable [37,545][942,633]
android.widget.TextView apps/mobile/app.config.ts tappable [175,545][942,591]
android.widget.TextView Modified tappable [175,596][286,633]
android.widget.TextView +1 tappable [304,596][336,633]
android.widget.TextView -1 tappable [354,596][380,633]
android.widget.CheckBox Unmark apps/mobile/app.config.ts as viewed tappable [961,547][1043,630]
android.widget.TextView 1 file loaded of 1 tappable [460,699][669,736]
android.widget.Button Open file navigator tappable [37,437][371,519]
android.widget.TextView Files · 1 of 1 viewed tappable [105,459][352,496]
android.widget.Button Finish review tappable [67,2191][1013,2307]
android.widget.TextView Finish review tappable [443,2225][637,2271]
/home/igor_kilocode_ai/.local/share/kwf/sections/mobile-perf-pr-review-cost-9b8f/e2e-mobile-app/e2-interaction-delta.log
# lines added to the Metro/dev-server log while the e2 scene ran
>  WARN  [KWF-PARSE] cache-hit 2 apps/mobile/app.config.ts
>  LOG  [screen-tracking] (app)/pr-review/[owner]/[repo]/[number]
/home/igor_kilocode_ai/.local/share/kwf/sections/mobile-perf-pr-review-cost-9b8f/e2e-mobile-app/e2-parse.log
rence-provider-id.ts
Android Bundled 608ms apps/mobile/index.js (1 module)
 WARN  [KWF-PARSE] cache-hit 2 apps/web/src/lib/ai-gateway/providers/openrouter/
inference-provider-id.ts
 WARN  [KWF-PARSE] parse 3 apps/web/src/lib/ai-gateway/providers/vercel/mapModel
IdToVercel.test.ts
Android Bundled 114ms apps/mobile/src/lib/persist/encrypted-kv.ts (1 module)
Android Bundled 54ms apps/mobile/src/lib/app-actions/native-bridge.ts (1 module)
 LOG  [KWF-PARSE] module loaded marker=e2-clean
 WARN  [Reanimated] Reduced motion setting is overwritten with mode 'system'.
Android Bundled 193ms apps/mobile/src/lib/persist/drafts.ts (1 module)
 WARN  InteractionManager has been deprecated and will be removed in a future re
lease. Please refactor long tasks into smaller ones, and  use 'requestIdleCallba
ck' instead.
 LOG  Success
 LOG  Success
 LOG  [screen-tracking] (app)/(tabs)/(0_home)
 LOG  Success
 LOG  [screen-tracking] (app)/(tabs)/(3_profile)
 WARN  [KWF-PARSE] parse 1 apps/mobile/app.config.ts
 WARN  [KWF-PARSE] cache-hit 4 apps/web/src/lib/ai-gateway/providers/vercel/mapM
odelIdToVercel.test.ts
 WARN  [KWF-PARSE] cache-hit 5 apps/web/src/lib/ai-gateway/providers/vercel/mapM
odelIdToVercel.test.ts
/home/igor_kilocode_ai/.local/share/kwf/sections/mobile-perf-pr-review-cost-9b8f/e2e-mobile-app/e3-behavior.log
# e3 behavior evidence (android emulator-5606), pr-review start state
# -- selected state (scene e3sel ok): one line selected + floating bar present
SCENE e3sel OK
android.widget.Button Selected Context line 65:   name: 'Kilo',, tap to change selection tappable [0,797][1080,856]
android.widget.Button Context line 66:   owner: 'kilocode',, tap to comment tappable [0,856][1080,915]
android.widget.Button Context line 66:   owner: 'kilocode',, tap to comment tappable [0,856][1080,915]
e3sel-scene.xml: 1 node(s) 'tap to change selection'; 1 node(s) content-desc "Clear selection"
# -- after tapping 'Clear selection' (scene e3 ok): bar and ring gone
SCENE e3 OK
android.widget.Button Context line 65:   name: 'Kilo',, tap to comment tappable [0,797][1080,856]
e3-scene.xml: 0 node(s) 'tap to change selection'; 0 node(s) 'Clear selection'
/home/igor_kilocode_ai/.local/share/kwf/sections/mobile-perf-pr-review-cost-9b8f/e2e-mobile-app/e3sel-scene.log
android.widget.TextView +1 tappable [304,596][336,633]
android.widget.TextView -1 tappable [354,596][380,633]
android.widget.CheckBox Mark apps/mobile/app.config.ts as viewed tappable [961,547][1043,630]
android.widget.Button Expand 20 lines of context tappable [267,681][598,718]
android.widget.TextView Expand 20 lines (1–20) tappable [307,681][597,718]
android.widget.Button Expand all 64 lines tappable [643,681][813,718]
android.widget.TextView Expand all tappable [683,681][812,718]
android.view.ViewGroup Hunk header @@ -65,7 +65,7 @@ const config: ExpoConfig = { tappable [0,737][1080,797]
android.widget.TextView @@ -65,7 +65,7 @@ const config: ExpoConfig = { tappable [36,746][1044,785]
android.widget.Button Selected Context line 65:   name: 'Kilo',, tap to change selection tappable [0,797][1080,856]
android.widget.TextView · 65 tappable [63,802][133,850]
android.view.ViewGroup Context line 65:   name: 'Kilo', tappable [152,797][1080,856]
android.widget.TextView   name: 'Kilo', tappable [152,802][1080,850]
android.widget.Button Context line 66:   owner: 'kilocode',, tap to comment tappable [0,856][1080,915]
android.widget.TextView · 66 tappable [63,861][133,909]
android.view.ViewGroup Context line 66:   owner: 'kilocode', tappable [152,856][1080,915]
android.widget.TextView   owner: 'kilocode', tappable [152,861][1080,909]
android.widget.Button Context line 67:   slug: 'kilo-app',, tap to comment tappable [0,915][1080,974]
android.widget.TextView · 67 tappable [63,920][133,968]
android.view.ViewGroup Context line 67:   slug: 'kilo-app', tappable [152,915][1080,974]
android.widget.TextView   slug: 'kilo-app', tappable [152,920][1080,968]
android.widget.Button Deleted line 68:   version: '1.0.10',, tap to comment tappable [0,974][1080,1033]
/home/igor_kilocode_ai/.local/share/kwf/sections/mobile-perf-pr-review-cost-9b8f/e2e-mobile-app/e3-scene.log
android.widget.TextView +1 tappable [304,596][336,633]
android.widget.TextView -1 tappable [354,596][380,633]
android.widget.CheckBox Mark apps/mobile/app.config.ts as viewed tappable [961,547][1043,630]
android.widget.Button Expand 20 lines of context tappable [267,681][598,718]
android.widget.TextView Expand 20 lines (1–20) tappable [307,681][597,718]
android.widget.Button Expand all 64 lines tappable [643,681][813,718]
android.widget.TextView Expand all tappable [683,681][812,718]
android.view.ViewGroup Hunk header @@ -65,7 +65,7 @@ const config: ExpoConfig = { tappable [0,737][1080,797]
android.widget.TextView @@ -65,7 +65,7 @@ const config: ExpoConfig = { tappable [36,746][1044,785]
android.widget.Button Context line 65:   name: 'Kilo',, tap to comment tappable [0,797][1080,856]
android.widget.TextView · 65 tappable [63,802][133,850]
android.view.ViewGroup Context line 65:   name: 'Kilo', tappable [152,797][1080,856]
android.widget.TextView   name: 'Kilo', tappable [152,802][1080,850]
android.widget.Button Context line 66:   owner: 'kilocode',, tap to comment tappable [0,856][1080,915]
android.widget.TextView · 66 tappable [63,861][133,909]
android.view.ViewGroup Context line 66:   owner: 'kilocode', tappable [152,856][1080,915]
android.widget.TextView   owner: 'kilocode', tappable [152,861][1080,909]
android.widget.Button Context line 67:   slug: 'kilo-app',, tap to comment tappable [0,915][1080,974]
android.widget.TextView · 67 tappable [63,920][133,968]
android.view.ViewGroup Context line 67:   slug: 'kilo-app', tappable [152,915][1080,974]
android.widget.TextView   slug: 'kilo-app', tappable [152,920][1080,968]
android.widget.Button Deleted line 68:   version: '1.0.10',, tap to comment tappable [0,974][1080,1033]
android.widget.TextView - 68 tappable [63,979][133,1027]
Owner request

Surface: mobile-app

Fix the mobile performance audit findings below. The audit is static: every finding was confirmed by reading the source, not by profiling. Confirm each finding in the current tree before you change it. If a finding is already fixed or no longer applies, say so and skip it; do not invent work.

Audit revision: the report was written against apps/mobile at commit 3fbc18a3b2 on main (2026-09-22). The worktree is newer. Re-locate every cited line by content, never by line number.

Constraints:

  • Change product behaviour only where a finding asks for it. A performance fix must not change what the user sees, except where the finding says the perceived performance improves.
  • No new dependency, and no new test-only runtime support, unless the owner allowed it.
  • Keep the existing test suites green: pnpm --filter @kilocode/mobile test, plus the mounted suite the section runbook names.
  • Proof: quote the decisive log lines from the run in the scenario note. Add a screenshot when the change has a visible surface. A performance fix with no visible surface is proved from a log line that shows the reduced work, for example a request count, a fetch count, or a parse count.

This item: mobile-perf-pr-review-cost

Fixes

Fix 11. Memoize the inbox and discussion merges, and parse each timestamp once per merge

  • Severity S2 · Checks P3 · Effort S
  • Files src/lib/pr-review/use-provider-inbox.ts, src/components/pr-review/pr-review-discussion-tab.tsx
  • Impact Both screens re-flatten and re-sort the whole loaded set in the render body, and the comparators call parseTimestamp twice per comparison. With a few hundred rows that is thousands of timestamp parses per render, on the interaction that caused the render, plus a new items array for the list to diff.
  • Detail G6, findings for use-provider-inbox.ts and pr-review-discussion-tab.tsx

Fix 13. Cache parsed diff patches instead of re-tokenizing them when one file is marked viewed

  • Severity S3 · Checks P3 · Effort S
  • Files src/lib/pr-review/diff/pr-diff-list-builder.ts
  • Impact Marking one file viewed or expanding one file re-parses every expanded patch from scratch on the JS thread, on a large PR, inside the interaction that caused it. The highlight layer already caches its per-line parse for exactly this reason.
  • Detail G6, finding for pr-diff-list-builder.ts

Fix 18. Stop defeating the diff row's memo comparator with a fresh onTap closure

  • Severity S3 · Checks P1 · Effort S
  • Files src/components/pr-review/diff/pr-diff-file-list-render.tsx
  • Impact A new onTap closure per diff line per render means DiffLine's memo never hits for a selectable line, so tapping one line re-renders every mounted diff row instead of the rows whose selection changed.
  • Detail G6, finding for pr-diff-file-list-render.tsx

Fix 19. Hoist the four per-row moderation mutations out of the comment row

  • Severity S3 · Checks P4 · Effort S
  • Files src/components/pr-review/discussion/comment-row.tsx
  • Impact Every mounted comment row creates four useMutation hooks that are only reachable from the overflow sheet. A discussion with 50 mounted rows holds 200+ mutation-cache listeners, all notified on every mutation anywhere in the app, and rebuilds four option closures per row per render.
  • Detail G6, finding for comment-row.tsx

Files this item owns

This item owns exactly the files listed below. Every other audited file belongs to a different workflow item; do not edit a file outside this list.

  • src/lib/pr-review/use-provider-inbox.ts
  • src/components/pr-review/pr-review-discussion-tab.tsx
  • src/lib/pr-review/diff/pr-diff-list-builder.ts
  • src/components/pr-review/diff/pr-diff-file-list-render.tsx
  • src/components/pr-review/discussion/comment-row.tsx

Findings in the source

Each bullet is the audited evidence for one file. Read the file, confirm the finding, then apply the fix.

src/lib/pr-review/use-provider-inbox.ts

  • [S2] [P3] src/lib/pr-review/use-provider-inbox.ts:303 — mergeProviderInboxSources is
    called in the hook body on every render, so the merged inbox is re-flattened and re-sorted
    (.sort at :140, whose comparator calls parseTimestamp twice per comparison) on every
    render of the inbox screen.
    Impact: with a few hundred loaded rows the screen re-parses timestamps thousands of times per
    render and hands FlashList a brand-new items array, so the list re-diffs its whole data set
    on each paste/clear state change, recents update and provider state transition.
    Fix: useMemo the merge on the three already-memoized row arrays plus the per-source flags,
    and decorate-sort with a precomputed updatedAtMs per row so each timestamp is parsed once
    per merge instead of once per comparison.

src/components/pr-review/pr-review-discussion-tab.tsx

  • [S3] [P3] src/components/pr-review/pr-review-discussion-tab.tsx:401 —
    mergeDiscussionListItems(threads, conversation) runs in the render body, so the whole
    loaded thread + comment set is re-merged and re-sorted on every render, with two
    parseTimestamp calls per comparison.
    Impact: on a PR with hundreds of loaded threads, every expand/collapse tap, optimistic
    reaction/resolve and reply-focus event re-sorts the full set on the JS thread during the
    interaction that triggered it.
    Fix: wrap the call in useMemo(..., [threads, conversation]) (both inputs are already
    identity-stable) and precompute each item's sort key once per merge.

src/lib/pr-review/diff/pr-diff-list-builder.ts

  • [S3] [P3] src/lib/pr-review/diff/pr-diff-list-builder.ts:118 —
    pushExpandedFileItems calls parsePatch(file.patch) for every expanded file, and
    buildFileItems is re-run by the list's memo (pr-diff-file-list.tsx:157) whenever
    viewed or expanded changes, so marking one file viewed (or expanding one) re-parses
    every expanded patch from scratch.
    Impact: on a large PR the whole expanded diff is re-tokenized on the JS thread on a one-row
    state change; highlight.ts already caches its per-line parse for exactly this reason, and
    this parse is not cached.
    Fix: cache parsed patches (an LRU keyed by the patch string, mirroring highlightLine) or
    parse once when pages are flattened and carry the ParsedPatch on the file item.

src/components/pr-review/diff/pr-diff-file-list-render.tsx

  • [S3] [P1] src/components/pr-review/diff/pr-diff-file-list-render.tsx:130 — the per-line
    onTap closure is created fresh for every diff-line item on every render, and DiffLine's
    memo comparator (diff-line.tsx:243) compares onTap by reference, so the memo can never
    hit for a selectable line.
    Impact: tapping one line re-renders every mounted diff row (each rebuilding its style objects
    and JSX) instead of only the rows whose isSelected actually changed, and the waste grows
    with the diff's mounted row count.
    Fix: bind a stable per-row callback the way pr-diff-file-navigator.tsx does
    (rowCallbacksRef), or read onTap from a ref inside DiffLine and drop it from the
    comparator.

src/components/pr-review/discussion/comment-row.tsx

  • [S3] [P4] src/components/pr-review/discussion/comment-row.tsx:141 (also :154, :167,
    :180) — every comment row mounts four useMutation hooks (report content, report user,
    mute, block) that are only reachable from the overflow action sheet, so each row adds four
    MutationObserver subscriptions to the mutation cache.
    Impact: a discussion with 50+ mounted comment rows holds 200+ cache listeners that are all
    notified on every mutation anywhere in the app, and the four option closures are rebuilt per
    row per render.
    Fix: lift the four moderation mutations into one hook called once by the list (or a small
    context) and pass a single onModerate(action, target) callback down to the rows.

Proof

Proof for this item is a log excerpt from the interaction. Open a large PR, mark one file viewed, and tap one diff line. Quote the decisive log lines that show the reduced work, and capture the screen after the interaction.

[e8] ux-check: Discussion tab, PR with many threads: expand then collapse a thread — it toggles in place, list order is unchanged, scroll position is retained, and no row blanks or flickers. — Android emulator-5604, Kilo-Org/kilocode#14219 (7 threads): the thread card toggles with its top edge fixed at y=1705 (Button [37,1705][1043,1940] collapsed vs ViewGroup [37,1705][1043,2152] + Collapse thread [72,1769] + Quoted diff snippet [72,1933] expanded), rows above keep identical bounds in all three digests, and the e8-expanded assert text lay below the captured viewport while its digest shows the re-expanded state; no UX-DEFECT seen in the digest (screenshots captured for the visual reviewer).

[e8] ux-check: Discussion tab, PR with many threads: expand then collapse a thread — it toggles in place, list order is unchanged, scroll position is retained, and no row blanks or flickers. — e8-collapsed.png

[e11] ux-check: Comment overflow: tap the ⋯ control — the native action sheet lists Report content, Report user, Mute, Block and Cancel; Report content shows the success toast with a receipt id; choosing… — android emulator-5604, #5895 Discussion tab; one scripted run asserted the sheet's Report content / Report user / Mute / Block / Cancel, then the toast assert "Report submitted. Receipt" (e11-run.log: "SCENE e11 OK"), backed by the server row 408f2a17-33f4-4544-8014-6e649cef8bfd|pr_discussion_content|comment|5549379252|other|{"platform": "mobile"} (e11-moderation-report-row.log). Block hid the author: final digest has 0 hits for kilo-code-bot and the summaries read "1 comment · started 16 days ago" where they read 2 before (e11-run.log), with the block row…

[e11] ux-check: Comment overflow: tap the ⋯ control — the native action sheet lists Report content, Report user, Mute, Block and Cancel; Report content shows the success toast with a receipt id; choosing… — e11-sheet.png

[e10] Discussion tab on mount (android emulator-5554) — Scene e10 ran OK: Home -> PR Review -> Kilo-Org/kilocode#14219 -> Discussion mounted the 7-item list with a comment row (vercel, 'Fix with Kilo', 'Comment actions'), proving the shared CommentModerationProvider is present (a row without it throws), with no red screen or error text (e10-discussion.log); no UX-DEFECT in the digest, capture e10.png is for the visual reviewer.

[e10] Discussion tab on mount (android emulator-5554) — e10.png

[e11] ux-check: Comment overflow: tap the ⋯ control — the native action sheet lists Report content, Report user, Mute, Block and Cancel; Report content shows the success toast with a receipt id; choosing…

[e11] ux-check: Comment overflow: tap the ⋯ control — the native action sheet lists Report content, Report user, Mute, Block and Cancel; Report content shows the success toast with a receipt id; choosing… — e11-report-toast.png

[e8] ux-check: Discussion tab, PR with many threads: expand then collapse a thread — it toggles in place, list order is unchanged, scroll position is retained, and no row blanks or flickers.

[e8] ux-check: Discussion tab, PR with many threads: expand then collapse a thread — it toggles in place, list order is unchanged, scroll position is retained, and no row blanks or flickers. — e8-expanded.png

[e5] ux-check: Diff tab, large PR: tap one diff line — only that line (and the selection range) shows the left selection ring, the action bar reflects the tapped line, and the rest of the mounted lines do… — android emulator-5604, #5895 (195 files), Files tab; one line tapped -> only it is selected (content-desc "Selected Added line 12: {, tap to change selection") and the action bar names the tapped line (text "apps/mobile/drizzle/meta/_journal.json RIGHT L12"); neighbours remain mounted (text "+ 13"); an earlier second tap widened the bar to RIGHT L1–L2. No UX-DEFECT observed. Captures: e5-line-selected.png, e5-diff-line-tapped.png. Replay e5.replay.json validated (SCENE e5 OK).

[e5] ux-check: Diff tab, large PR: tap one diff line — only that line (and the selection range) shows the left selection ring, the action bar reflects the tapped line, and the rest of the mounted lines do… — e5-line-selected.png

[e7] ux-check: Diff tab: with several files expanded, expand one more — only that file's rows appear; previously expanded files and the scroll position are preserved. — android emulator-5554, real PR Kilo-Org/kilocode#14434 (8 files): with SessionUi.kt and .changeset/… expanded, expanding EditorContextGatherer.kt adds only its rows (e7-expanded3.txt: 'Added line 41: … The selection marker …' [0,1650][1080,1805] under 'Collapse file' [37,1059][942,1193]) while SessionUi.kt's rows stay put ('Context line 963 …' [0,452][1080,520]); scroll is preserved — the same anchor row is at [0,452][1080,520] in both e7-before.log and e7-scene.log while the row below only flips 'Expand file' → 'Collapse file' at [37,1059][942,1193]; captures…

[e7] ux-check: Diff tab: with several files expanded, expand one more — only that file's rows appear; previously expanded files and the scroll position are preserved. — e7-home.png

[e2] PR-review entry/inbox (android emulator-5554) — Scene e2 (amended step list, anchored /^PR Review$/) ran OK: from signed-in Home the PR Review row opened the PR-review screen whose INBOX rendered three review-request rows (Kilo-Org/kilocode#14219, Kilo-Org/kilocode#14198, #6401) and whose RECENT section showed 'No recent reviews' (e2-inbox.log); the one-parse-per-row and stable-array behaviour is asserted by use-provider-inbox.test.ts and use-provider-inbox.mounted.test.tsx, run by the driver; no UX-DEFECT in the settled digest, capture e2.png is for the visual reviewer.

[e2] PR-review entry/inbox (android emulator-5554) — e2.png

Follow-ups (not changed here)

  • not proved live: Discussion tab of the PR: the loaded thread list renders, then tap 'Comment actions' on one comment, see the four moderation options (Report content / Report user / Mute / Block), tap Cancel, and the sheet closes with no moderation call made. Capture the screen with the sheet open and the thread list behind it. The per-render re-merge/re-sort of the whole loaded set is proven deterministically by merge-discussion-list-items.test.ts (one parseTimestamp per item, identical ordering) and the per-row mutation count by comment-row.test.tsx (exactly four useMutation calls under one provider for N rows). (no capture cited it)
  • not proved live: Files tab on a large PR: expand a file and tap ONE context diff line; only that line paints the selection and the floating actions bar appears ('Clear selection'). Tap 'Clear selection' and assert the bar and the ring are gone. Capture both screens. The stable onTap identity is asserted by pr-diff-file-list-render.test.tsx. (no capture cited it)
  • not proved live: Files tab on a large PR: open the first file ('Expand file'), then tap its 'Mark ... as viewed' control and assert it flips to 'Unmark' while the rest of the expanded diff stays painted. Capture the screen after the interaction. Temporary boundary counters around parsePatch (apps/mobile/AGENTS.md Debugging: add, read, remove, never commit) must show one parse per file object for the interaction; the deterministic version is asserted by pr-diff-list-builder.test.ts. (no capture cited it)
  • not proved live: PR-review entry/inbox: from Home tap the 'PR Review' product-choices row and read the Inbox (review-request rows, or the 'No review requests' empty state). Capture the screen; quote the merge/parse lines if the verifier instruments parseTimestamp temporarily. The render-to-render array identity and the one-parse-per-row counts are asserted by use-provider-inbox.mounted.test.tsx and use-provider-inbox.test.ts. (no capture cited it)
  • not proved live: ux-check: Discussion tab on mount: the list renders with no red screen or thrown error (shared comment-moderation provider present). (no capture cited it)
  • not proved live: ux-check: Inbox with two or more connected providers: rows render newest-first, and pull-to-refresh / load-more leave the list stable with no blank or jump. (no capture cited it)
  • not proved live: iOS: not run — the diff forks on no platform, so Android proves both

e9-before

e9-after-react

e3

e3sel

@kilo-code-bot

kilo-code-bot Bot commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: 1 Issue Found | Recommendation: Address before merge

Executive Summary

No new defects found in this 17-file perf refactor (memoized inbox/discussion merges, shared comment-moderation provider, cached parsed diff patches, identity-stable per-line tap callbacks); the previously raised per-line map-retention WARNING is fixed by the new DiffLineRow release path, and the only open item is the carried-forward SUGGESTION about the duplicated discussion sort key.

Overview

Severity Count
CRITICAL 0
WARNING 0
SUGGESTION 1
Issue Details (click to expand)

SUGGESTION

File Line Issue
apps/mobile/src/lib/pr-review/discussion/merge-discussion-list-items.ts 37 timestampMsFor still duplicates the private discussionItemTimestampMs A2.2 sort key, so the two can drift (carried forward; the author replied they consider it no-defect).
Files Reviewed (17 files)
  • apps/mobile/src/components/pr-review/diff/pr-diff-file-list-render.test.tsx - clean
  • apps/mobile/src/components/pr-review/diff/pr-diff-file-list-render.tsx - clean
  • apps/mobile/src/components/pr-review/discussion/comment-moderation.tsx - clean
  • apps/mobile/src/components/pr-review/discussion/comment-row.test.tsx - clean
  • apps/mobile/src/components/pr-review/discussion/comment-row.touch-target.test.tsx - clean
  • apps/mobile/src/components/pr-review/discussion/comment-row.tsx - clean
  • apps/mobile/src/components/pr-review/pr-review-discussion-tab.test-helpers.tsx - clean
  • apps/mobile/src/components/pr-review/pr-review-discussion-tab.test.tsx - clean
  • apps/mobile/src/components/pr-review/pr-review-discussion-tab.tsx - clean
  • apps/mobile/src/lib/pr-review/diff/pr-diff-list-builder.test.ts - clean
  • apps/mobile/src/lib/pr-review/diff/pr-diff-list-builder.ts - clean
  • apps/mobile/src/lib/pr-review/discussion/merge-discussion-list-items.test.ts - clean
  • apps/mobile/src/lib/pr-review/discussion/merge-discussion-list-items.ts - 1 issue
  • apps/mobile/src/lib/pr-review/use-provider-inbox.mounted.test.tsx - clean
  • apps/mobile/src/lib/pr-review/use-provider-inbox.test-helpers.ts - clean
  • apps/mobile/src/lib/pr-review/use-provider-inbox.test.ts - clean
  • apps/mobile/src/lib/pr-review/use-provider-inbox.ts - clean

Fix these issues in Kilo Cloud

Previous Review Summary (commit b6863d0)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit b6863d0)

Status: 2 Issues Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 1
Issue Details (click to expand)

WARNING

File Line Issue
apps/mobile/src/components/pr-review/diff/pr-diff-file-list-render.tsx 89 lineItemRef/lineTapRef are never pruned, so they retain an entry (and the referenced parsed patch) for every diff line ever rendered until the screen unmounts.

SUGGESTION

File Line Issue
apps/mobile/src/lib/pr-review/discussion/merge-discussion-list-items.ts 37 timestampMsFor duplicates the private discussionItemTimestampMs A2.2 sort key; the two can drift.
Files Reviewed (17 files)
  • apps/mobile/src/components/pr-review/diff/pr-diff-file-list-render.test.tsx - clean
  • apps/mobile/src/components/pr-review/diff/pr-diff-file-list-render.tsx - 1 issue
  • apps/mobile/src/components/pr-review/discussion/comment-moderation.tsx - clean
  • apps/mobile/src/components/pr-review/discussion/comment-row.test.tsx - clean
  • apps/mobile/src/components/pr-review/discussion/comment-row.touch-target.test.tsx - clean
  • apps/mobile/src/components/pr-review/discussion/comment-row.tsx - clean
  • apps/mobile/src/components/pr-review/pr-review-discussion-tab.test-helpers.tsx - clean
  • apps/mobile/src/components/pr-review/pr-review-discussion-tab.test.tsx - clean
  • apps/mobile/src/components/pr-review/pr-review-discussion-tab.tsx - clean
  • apps/mobile/src/lib/pr-review/diff/pr-diff-list-builder.test.ts - clean
  • apps/mobile/src/lib/pr-review/diff/pr-diff-list-builder.ts - clean
  • apps/mobile/src/lib/pr-review/discussion/merge-discussion-list-items.test.ts - clean
  • apps/mobile/src/lib/pr-review/discussion/merge-discussion-list-items.ts - 1 issue
  • apps/mobile/src/lib/pr-review/use-provider-inbox.mounted.test.tsx - clean
  • apps/mobile/src/lib/pr-review/use-provider-inbox.test-helpers.ts - clean
  • apps/mobile/src/lib/pr-review/use-provider-inbox.test.ts - clean
  • apps/mobile/src/lib/pr-review/use-provider-inbox.ts - clean

Verified: the decorated merges are order-equivalent to the originals (tie/missing-timestamp cases delegate to the shared comparator), the useProviderInbox memo deps cover every value the three sources read, parsedPatchFor's WeakMap cannot go stale for immutable file DTOs, and every production CommentRow render path sits under CommentModerationProvider (PrReviewDiscussionList is only mounted inside the tab's provider).

Fix these issues in Kilo Cloud


Reviewed by deepseek-v4.1-flash · Input: 0 · Output: 0 · Cached: 0

Review guidance: REVIEW.md from base branch main

@iscekic
iscekic marked this pull request as draft September 22, 2026 19:12
@iscekic
iscekic force-pushed the kwf/mobile-perf-pr-review-cost-9b8f branch from 4d13641 to e38bdc2 Compare September 22, 2026 20:07
@iscekic
iscekic marked this pull request as ready for review September 22, 2026 20:27
@iscekic

iscekic commented Sep 22, 2026

Copy link
Copy Markdown
Collaborator Author

bot: rejected: a suggestion, not a defect. A typed review read this finding as needing no code change before merge; the code stays as is. Say so here if one of them does change behaviour and it will be repaired.

  • apps/mobile/src/lib/pr-review/discussion/merge-discussion-list-items.ts:37 — timestampMsFor still duplicates the private discussionItemTimestampMs A2.2 sort key, so the two can drift (carried forward; the author replied they consider it no-defect). (p=0.83)

@iscekic

iscekic commented Sep 22, 2026

Copy link
Copy Markdown
Collaborator Author

This description names a scenario the proof did not capture:

  • not proved live: Discussion tab of the PR: the loaded thread list renders, then tap 'Comment actions' on one comment, see the four moderation options (Report content / Report user / Mute / Block), tap Cancel, and the sheet closes with no moderation call made. Capture the screen with the sheet open and the thread list behind it. The per-render re-merge/re-sort of the whole loaded set is proven deterministically by merge-discussion-list-items.test.ts (one parseTimestamp per item, identical ordering) and the per-row mutation count by comment-row.test.tsx (exactly four useMutation calls under one provider for N rows). (no capture cited it)

A repeated proof run rebuilds the same evidence, so no proof run is dispatched for a named gap. Merging with this gap open is your decision.

@iscekic iscekic added the human-ready The PR is ready for human review. label Sep 22, 2026
@iscekic iscekic self-assigned this Sep 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

human-ready The PR is ready for human review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants