fix(mobile): PR sheet chrome + session UX defect batch; sandbox CLI 7.3.63 for routed-model stamps - #4763
Merged
Merged
Conversation
…board-safe footer react-native-screens formSheet only honors a pinned header when the screen content's direct children are [header, ScrollView]; the old View → ScrollView + sticky-footer tree let the ScrollView go full-bleed and overpaint the header. The stray leading-edge ‹ was the parent PR screen's Go back showing through a transparent top band (opaque sheet chrome + close affordance fixes it). Footer CTAs move inside the ScrollView as trailing content with automaticallyAdjustKeyboardInsets + AppAwareKeyboardPaddingView, plus density passes so Merge/Submit/comment CTAs stay above the keyboard at both detents without scrolling.
The input placeholder already shows the example URL, so the Paste a link hint state is dead weight; invalid and clipboard-empty helper states are unchanged. Navigator rows get a min-w-0 chain with a truncating shrinkable directory segment, non-shrinking basename, and a non-shrinking viewed toggle with a guaranteed gap, so long paths no longer overlap the toggle.
Paperclip and voice button no longer flank the multiline input at mid-height. The input sits full-width at the top of the card and one bottom action row holds the paperclip (leading) and voice toggle (trailing); when voice input is unavailable the row shows the paperclip only. Attachment strip, voice status line, and ChatToolbar unchanged.
Render-only filter: a Models row is hidden iff providerID is kilo and modelID starts with kilo-auto/. Totals and the Subagents residual are still computed over all steps, and section visibility plus the Models (N) count derive from the filtered rows plus the residual, so an auto-only session with no residual hides the section entirely.
…amps The 7.3.54 pin predates the routed-model stamp feature (first released in v7.3.63), so step-finish parts for kilo-auto/* sessions arrived without a model stamp and the mobile Models list had nothing concrete to show. Bumped in lockstep across Dockerfiles, wrangler image_vars, and KILO_CLI_VERSION (devcontainer.test.ts asserts the lockstep). Re-proved live: a fresh kilo-auto/efficient session now stamps step-finish.model with the concrete routed id at the ingest boundary and in the app-facing payload. @kilocode/sdk stays at 7.3.54 (verified faithful in the ingest path). The slash-command SOURCE marker follows the pin; the catalog body regeneration is a follow-up (generator's kilo serve step hung locally).
…model label While the cloud agent is preparing, the fixed footer row above the composer duplicated the in-transcript PreparationGroup. The new shouldShowSessionFooterRow gate suppresses the footer only while the transcript carries a running (non-no-op) preparation item; a stale completed or failed group never suppresses it, and with no live group the footer keeps rendering so there is never a blank progress window. Also removes the per-message model label from the transcript (label, wiring, and computeMessageModelLabels); message info moves to a long-tap details sheet. resolveMessageDisplayModel is kept for that sheet.
Long-press on a user or assistant message now opens a details sheet (Modal pageSheet with SheetHeader chrome) instead of the copy ActionSheet. The sheet shows a Copy message action (immediate clipboard write through the shared performCopy extracted from use-message-copy, hidden when the message has no copyable text), title-cased Role, Sent time formatted from the epoch created timestamp, a Model row for assistant messages (routed-first via resolveMessageDisplayModel, friendly catalog name, hidden only when unresolvable), and a Cost & tokens block for assistant messages (hidden when cost and all five token values are zero). The VoiceOver/TalkBack rotor copy action keeps the existing useMessageCopy behavior, and compaction-only rows stay non-interactive.
Contributor
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryRe-verified against current HEAD ( Files Reviewed (38 files)
Previous Review Summary (commit e62f835)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit e62f835)Status: No Issues Found | Recommendation: Merge Executive SummaryReviewed the mobile PR-sheet chrome/UX defect batch and cloud-agent-next CLI version bump across 39 changed files; no high-confidence security, runtime, logic, or breaking-API issues were found in changed lines. Files Reviewed (39 files)
Reviewed by claude-sonnet-5 · Input: 18 · Output: 4.5K · Cached: 346.1K Review guidance: REVIEW.md from base branch |
Contributor
Author
|
(bot) @kilo-code-bot please review the latest head (57e0e8d) — the branch was updated with a clean merge of main. |
pandemicsyn
reviewed
Jul 25, 2026
pandemicsyn
left a comment
pandemicsyn
left a comment
Contributor
There was a problem hiding this comment.
lgtm, one nit inline from the robot.
| isDisabled, | ||
| /** Half-detent / keyboard-open: keep a single-line-ish field so footer CTAs fit. */ | ||
| compact = false, | ||
| }: Readonly<{ |
Contributor
There was a problem hiding this comment.
useFormSheetKeyboardVisible() is now called independently in both PrMergeSheet (for isHalfDetent) and CommitMessageField (for tight), each registering its own pair of Keyboard listeners. Functionally harmless (both derive the same boolean) but it duplicates listener setup/teardown on every mount and is easy to lose sync with if the two call sites ever diverge in show/hide event names.
Suggested fix: Thread the keyboard-visible boolean down from PrMergeSheet as a prop instead of re-subscribing inside CommitMessageField, matching the single-source pattern already used for isHalfDetent.
pandemicsyn
approved these changes
Jul 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Mobile UX defect batch (7 reproduced defects) + GitHub rate-limit investigation (report-only).
‹floated over content; with the keyboard open the CTA footer rendered fully behind the keyboard. Root cause (device-backed): react-native-screens formSheet only honors a pinned header when the screen content's direct children are[header, ScrollView]— the oldView → ScrollView + sticky footertree made the ScrollView full-bleed over the header; the stray‹was the parent PR screen's "Go back" showing through a transparent top band. Fix: shared pinnedPrFormSheetHeaderoutside the ScrollView, footer CTAs as trailing scroll content withautomaticallyAdjustKeyboardInsets+AppAwareKeyboardPaddingView, plus density passes so all CTAs sit above the keyboard at both detents without scrolling.min-w-0chain, truncating shrinkable directory segment, non-shrinking basename, non-shrinking toggle with a guaranteed gap.Importing session…footer row duplicated the in-transcript PreparationGroup. Fix:shouldShowSessionFooterRowgate — whilepreparing, suppress the footer only when the merged transcript carries a visible (non-no-op) preparation item; otherwise the footer keeps rendering (never a blank progress window).kilo/kilo-auto/*rows (render-only filter; totals/residual untouched; count derives from filtered rows). Data path — root cause was the sandbox-pinned CLI7.3.54predating the routed-model stamp feature (first released in7.3.63); bumped the pin in lockstep (Dockerfiles, wranglerimage_vars,KILO_CLI_VERSION) and re-proved live: freshkilo-auto/efficientsessions now stampstep-finish.modelwith the concrete routed id (e.g.qwen/qwen3.7-plus) at the ingest boundary and in the app-facing payload.R3 (file-navigator missing header) was dropped: could not be reproduced — already fixed on baseline.
GitHub rate-limit investigation (report only — no code changes)
expires_in+refresh_tokenand refresh usesgrant_type: 'refresh_token'— the GitHub App user-to-server pattern; each call builds an Octokit with the fetched per-user token; all 16 procedures wrapwithGitHubUserTokenRetry, so the rate-limit bucket is per user and scales with user count; no installation-token/shared-bucket path exists in this featureapps/web/src/lib/integrations/platforms/github/user-authorization.ts:13-18,72-93; refresh grantservices/git-token-service/src/github-user-authorization-service.ts:502-511; per-call fetch+Octokitapps/web/src/lib/github-pr-review/retry.ts:63-70+client.ts:14-19; 16 call sitesgithub-pr-review-router.ts:509-956Promise.allgithub-pr-review-router.ts:653-673+ loop399-429apps/mobile/src/lib/pr-review/diff/pr-review-file-list-state.ts:197-204;octokit.paginateinlistChecks(apps/web/src/routers/github-pr-review-router.ts:570-581);onEndReachedauto-pagination (apps/mobile/src/components/pr-review/diff/pr-diff-file-list.tsx:343-347)apps/mobile/src/components/pr-review/merge/pr-merge-section.tsx:49-50,131-142; fan-outapps/web/src/routers/github-pr-review-router.ts:515-537apps/mobile/src/lib/pr-review/use-pr-review-mutations.ts:34-41;apps/mobile/src/lib/pr-review/merge/use-pr-merge-mutations.ts:31-40;apps/mobile/src/lib/pr-review/discussion/use-review-discussion-mutations.ts:55-60TOO_MANY_REQUESTS2× against an exhausted bucketapps/mobile/src/lib/query-client.ts:7-26(permanent-code set omits it)FORBIDDENapps/web/src/lib/github-pr-review/errors.ts:49-53,163-171retryAfterEpochMsdropped at the tRPC boundaryapps/web/src/lib/github-pr-review/retry.ts:22-27apps/web/src/lib/github-pr-review/errors.ts:146; repo-wide grep confirms no If-None-Match in GitHub pathsgetFileLinesdownloads whole raw file per 500-line window, no cacheapps/web/src/routers/github-pr-review-router.ts:611-637;apps/mobile/src/lib/pr-review/diff/use-pr-diff-context-loader.ts:61-68Recommendations (no code in this PR): bound GraphQL follow-up concurrency + pace pagination; plumb
retryAfterto the client and suppress auto-retry on 429; classify headerless 403 secondary limits as retryableTOO_MANY_REQUESTS; consider ETag caching for overview/files.Notes
default-slash-commands.generated.ts: source marker bumped for the lockstep test; the catalog body was not regenerated (the generator'skilo servestep hung locally with 7.3.63) — content drift vs 7.3.63 is possible and tracked as follow-up.providerMetadata.kilocode.routedModelIDis not emitted by the 7.3.63 stamp path; consumers readpart.model({providerID, modelID}), whichcloud-agent-sdk/part-utils#getStepFinishRoutedModelalready does.