feat(mobile): GitHub review threads, viewed sync and line comments on the diff screen (#37073)

* feat(mobile): GitHub threads, viewed sync and line comments on the diff screen

Committed files of a GitHub pull request now show its review threads under
their lines with Reply and Resolve, take their viewed state from GitHub and
sync the checkbox and Viewed · next file back through setPRFileViewed. A tap
or a long-press drag picks lines and opens a sheet that sends the note to
the workspace's agent through the existing send path, or posts it as a
review comment. Threads and GitHub comments appear only while the local
branch is at the PR head, the same contents desktop shows them on;
otherwise the file says to push. openDiffForFile takes an optional line for
a thread's Open in diff.

* test(mobile): use a type import for the line-mutations recording adapter

* fix(mobile): name the staged or unstaged side in the jump sheet's file labels
This commit is contained in:
Kelvin Amoaba
2026-11-01 18:38:23 +00:00
committed by GitHub
parent 24e7a0ba69
commit a3efdcb21b
45 changed files with 2105 additions and 288 deletions
+12 -12
View File
@@ -72,18 +72,18 @@ export const MOBILE_WEB_APP_BUNDLE_SCRIPT_SWEEP = [
['./h/[hostId]/accounts.tsx', 12],
['./h/[hostId]/agent-history/[worktreeId].tsx', 17],
['./h/[hostId]/automations.tsx', 24],
['./h/[hostId]/edit.tsx', 30],
['./h/[hostId]/files/[worktreeId].tsx', 35],
['./h/[hostId]/files/preview/[worktreeId].tsx', 42],
['./h/[hostId]/history/[worktreeId].tsx', 45],
['./h/[hostId]/index.tsx', 53],
['./h/[hostId]/new-workspace.tsx', 63],
['./h/[hostId]/pr/[worktreeId].tsx', 64],
['./h/[hostId]/review/[worktreeId].tsx', 77],
['./h/[hostId]/session/[worktreeId].tsx', 92],
['./h/[hostId]/source-control/[worktreeId].tsx', 100],
['./h/[hostId]/tasks.tsx', 114],
['./h/_layout.tsx', 118]
['./h/[hostId]/edit.tsx', 29],
['./h/[hostId]/files/[worktreeId].tsx', 34],
['./h/[hostId]/files/preview/[worktreeId].tsx', 44],
['./h/[hostId]/history/[worktreeId].tsx', 47],
['./h/[hostId]/index.tsx', 55],
['./h/[hostId]/new-workspace.tsx', 66],
['./h/[hostId]/pr/[worktreeId].tsx', 67],
['./h/[hostId]/review/[worktreeId].tsx', 80],
['./h/[hostId]/session/[worktreeId].tsx', 95],
['./h/[hostId]/source-control/[worktreeId].tsx', 104],
['./h/[hostId]/tasks.tsx', 119],
['./h/_layout.tsx', 123]
]
const sweptScripts = MOBILE_WEB_APP_BUNDLE_SCRIPT_SWEEP.map(([, scripts]) => scripts)
@@ -23,6 +23,7 @@ export default withWorkspaceRoute(function MobileDiffReviewScreen() {
file?: string | string[]
area?: string | string[]
commit?: string | string[]
line?: string | string[]
}>()
const hostId = firstReviewParam(params.hostId)
const worktreeId = firstReviewParam(params.worktreeId)
@@ -31,7 +32,7 @@ export default withWorkspaceRoute(function MobileDiffReviewScreen() {
// Carry screen query params unchanged, omitting empty values: screen normalizers distinguish absence from empty scope/file/area.
const routeParams: Record<string, string> = {
...Object.fromEntries(
(['name', 'scope', 'file', 'area', 'commit'] as const)
(['name', 'scope', 'file', 'area', 'commit', 'line'] as const)
.map((key) => [key, firstReviewParam(params[key])] as const)
.filter(([, value]) => value !== '')
),
@@ -0,0 +1,219 @@
{
"goldenFormatVersion": 6,
"operation": "session.pr-mutations",
"family": "github.pr-comment-mutation",
"namedDeltas": [],
"values": {
"00e36cc2e1a3": {
"file-viewed": {
"ok": true
},
"line-comment": {
"ok": true
}
},
"0632a06c33cb": {
"name": "github.addPRReviewComment#1",
"ordinal": 2,
"json": "{\"id\":\"frame-1\",\"deviceToken\":\"recording-device\",\"method\":\"github.addPRReviewComment\",\"params\":{\"repo\":\"id:repo-9\",\"prNumber\":12,\"commitId\":\"head-sha-1\",\"path\":\"src/app.ts\",\"line\":5,\"body\":\"recorded line comment\",\"startLine\":3}}"
},
"1da2b9026f18": {
"name": "github.setPRFileViewed#2",
"ordinal": 5,
"args": [
{
"name": "method",
"value": "github.setPRFileViewed"
},
{
"name": "params",
"value": {
"path": "src/app.ts",
"pullRequestId": "PR_kwDO",
"repo": "id:repo-9",
"viewed": true
}
},
{
"name": "options",
"value": {
"$rpc": "absent"
}
}
],
"settlement": {
"status": "fulfilled",
"startedAt": 0,
"settledAt": 0,
"value": {
"id": "frame-3",
"ok": true,
"result": false
}
}
},
"29a4002b51d7": {
"file-viewed": {
"error": "Failed to update viewed state.",
"ok": false
},
"line-comment": {
"ok": true
}
},
"5af21c5f4fe0": {
"name": "github.setPRFileViewed#1",
"ordinal": 3,
"args": [
{
"name": "method",
"value": "github.setPRFileViewed"
},
{
"name": "params",
"value": {
"path": "src/app.ts",
"pullRequestId": "PR_kwDO",
"repo": "id:repo-9",
"viewed": true
}
},
{
"name": "options",
"value": {
"$rpc": "absent"
}
}
],
"settlement": {
"status": "fulfilled",
"startedAt": 0,
"settledAt": 0,
"value": {
"id": "frame-2",
"ok": true,
"result": true
}
}
},
"65a217bed712": {
"line-comment": {
"ok": true
}
},
"94ed6c684a96": {
"name": "github.setPRFileViewed#1",
"ordinal": 4,
"json": "{\"id\":\"frame-2\",\"deviceToken\":\"recording-device\",\"method\":\"github.setPRFileViewed\",\"params\":{\"repo\":\"id:repo-9\",\"pullRequestId\":\"PR_kwDO\",\"path\":\"src/app.ts\",\"viewed\":true}}"
},
"afe0045df4a7": {
"status": "fulfilled",
"startedAt": 0,
"settledAt": 0,
"value": {
"error": "Failed to update viewed state.",
"ok": false
}
},
"c655a2fc8c3a": {
"name": "github.addPRReviewComment#1",
"ordinal": 1,
"args": [
{
"name": "method",
"value": "github.addPRReviewComment"
},
{
"name": "params",
"value": {
"body": "recorded line comment",
"commitId": "head-sha-1",
"line": 5,
"path": "src/app.ts",
"prNumber": 12,
"repo": "id:repo-9",
"startLine": 3
}
},
{
"name": "options",
"value": {
"$rpc": "absent"
}
}
],
"settlement": {
"status": "fulfilled",
"startedAt": 0,
"settledAt": 0,
"value": {
"id": "frame-1",
"ok": true,
"result": {
"comment": {
"id": 57
},
"ok": true
}
}
}
},
"de607d31ed8c": {
"name": "github.setPRFileViewed#2",
"ordinal": 6,
"json": "{\"id\":\"frame-3\",\"deviceToken\":\"recording-device\",\"method\":\"github.setPRFileViewed\",\"params\":{\"repo\":\"id:repo-9\",\"pullRequestId\":\"PR_kwDO\",\"path\":\"src/app.ts\",\"viewed\":true}}"
},
"fbc958e4d46e": {
"status": "fulfilled",
"startedAt": 0,
"settledAt": 0,
"value": {
"ok": true
}
}
},
"recording": {
"scenario": "pr-line-comment-and-file-viewed",
"checkpoints": [
{
"id": "line-comment",
"observation": {
"sender": ["c655a2fc8c3a"],
"payloads": ["0632a06c33cb"],
"settlements": {
"line-comment": "fbc958e4d46e"
},
"state": "65a217bed712",
"effects": []
}
},
{
"id": "viewed",
"observation": {
"sender": ["c655a2fc8c3a", "5af21c5f4fe0"],
"payloads": ["0632a06c33cb", "94ed6c684a96"],
"settlements": {
"line-comment": "fbc958e4d46e",
"viewed": "fbc958e4d46e"
},
"state": "00e36cc2e1a3",
"effects": []
}
},
{
"id": "refused",
"observation": {
"sender": ["c655a2fc8c3a", "5af21c5f4fe0", "1da2b9026f18"],
"payloads": ["0632a06c33cb", "94ed6c684a96", "de607d31ed8c"],
"settlements": {
"line-comment": "fbc958e4d46e",
"viewed": "fbc958e4d46e",
"refused": "afe0045df4a7"
},
"state": "29a4002b51d7",
"effects": []
}
}
]
}
}
@@ -8928,6 +8928,83 @@
}
]
},
{
"id": "pr-line-comment-and-file-viewed",
"operation": "session.pr-mutations",
"family": "github.pr-comment-mutation",
"sites": ["mobile/src/session/github-pr-review-line-mutations.ts"],
"schedules": [],
"steps": [
{
"action": "line-comment",
"id": "line-comment"
},
{
"complete": "github.addPRReviewComment#1",
"params": {
"repo": "id:repo-9",
"prNumber": 12,
"commitId": "head-sha-1",
"path": "src/app.ts",
"line": 5,
"body": "recorded line comment",
"startLine": 3
},
"reply": {
"ok": true,
"result": {
"ok": true,
"comment": {
"id": 57
}
}
}
},
{
"checkpoint": "line-comment"
},
{
"action": "file-viewed",
"id": "viewed"
},
{
"complete": "github.setPRFileViewed#1",
"params": {
"repo": "id:repo-9",
"pullRequestId": "PR_kwDO",
"path": "src/app.ts",
"viewed": true
},
"reply": {
"ok": true,
"result": true
}
},
{
"checkpoint": "viewed"
},
{
"action": "file-viewed",
"id": "refused"
},
{
"complete": "github.setPRFileViewed#2",
"params": {
"repo": "id:repo-9",
"pullRequestId": "PR_kwDO",
"path": "src/app.ts",
"viewed": true
},
"reply": {
"ok": true,
"result": false
}
},
{
"checkpoint": "refused"
}
]
},
{
"id": "pr-comment-resolve-unconfirmed",
"operation": "session.pr-mutations",
+24 -102
View File
@@ -1,26 +1,16 @@
import { useMemo, useState } from 'react'
import { FlatList, Pressable, Text } from 'react-native'
import { ChevronsUpDown, FileText, ListChecks } from 'lucide-react-native'
import { FileText, ListChecks } from 'lucide-react-native'
import type { DiffComment } from '../../../src/shared/diff-comment-types'
import { MobileLoadErrorState, MobileOfflineState, MobileStateView } from './MobileStateView'
import { useMobileColorScheme } from '../theme/use-mobile-color-scheme'
import { useMobileTranslation } from '../i18n/use-mobile-translation'
import { MobileDiffReviewLine } from './MobileDiffReviewLine'
import type {
ReviewDiffLine,
ReviewDiffState,
ReviewScreenState
} from '../session/mobile-diff-review-screen-model'
import {
buildMobileDiffReviewRows,
type MobileDiffReviewRow
} from '../session/mobile-diff-review-rows'
import { colors } from '../theme/mobile-theme'
import { MobileDiffReviewLines, type LineRange } from './MobileDiffReviewLines'
import type { ReviewDiffState, ReviewScreenState } from '../session/mobile-diff-review-screen-model'
import type {
MobileDiffReviewQueueFilter,
MobileDiffReviewQueueItem
} from '../session/mobile-diff-review-queue'
import { mobileDiffReviewStyles as styles } from './mobile-diff-review-screen-styles'
import type { MobileDiffReviewThread } from '../session/mobile-diff-review-github'
import type { PRCommentCardActions } from './pr-sidebar/PRCommentCard'
type Props = {
commentsByLine: ReadonlyMap<number, DiffComment[]>
@@ -29,7 +19,13 @@ type Props = {
filteredCount: number
screenState: ReviewScreenState
staleCommentIds: ReadonlySet<string>
onAddNote: (lineNumber: number) => void
threadsByLine: ReadonlyMap<number, readonly MobileDiffReviewThread[]>
threadActions: PRCommentCardActions | undefined
selectedRange: LineRange | null
onPickLines: (range: LineRange) => void
/** Scroll to this line once, then report it done. */
scrollToLine: number | null
onScrolledToLine: () => void
onEditNote: (comment: DiffComment) => void
onRetry: (() => void) | undefined
/** The host is unreachable, so a failed load is a wait rather than an error. */
@@ -46,7 +42,12 @@ export function MobileDiffReviewBody({
filteredCount,
screenState,
staleCommentIds,
onAddNote,
threadsByLine,
threadActions,
selectedRange,
onPickLines,
scrollToLine,
onScrolledToLine,
onEditNote,
onRetry,
offline,
@@ -121,96 +122,17 @@ export function MobileDiffReviewBody({
truncated={diffState.truncated}
commentsByLine={commentsByLine}
staleCommentIds={staleCommentIds}
{...(currentItem?.scope === 'commit' ? {} : { onAddNote })}
threadsByLine={threadsByLine}
threadActions={threadActions}
selectedRange={selectedRange}
scrollToLine={scrollToLine}
onScrolledToLine={onScrolledToLine}
{...(currentItem?.scope === 'commit' ? {} : { onPickLines })}
onEditNote={onEditNote}
/>
)
}
const NO_OPENED_LINES: ReadonlySet<number> = new Set()
// Keyed by file, so folds the reader opened close again on the next file.
function MobileDiffReviewLines({
lines,
truncated,
commentsByLine,
staleCommentIds,
onAddNote,
onEditNote
}: {
lines: ReviewDiffLine[]
truncated: boolean
commentsByLine: ReadonlyMap<number, DiffComment[]>
staleCommentIds: ReadonlySet<string>
onAddNote?: (lineNumber: number) => void
onEditNote: (comment: DiffComment) => void
}) {
useMobileColorScheme()
const t = useMobileTranslation()
// Opened folds are line indexes into this load; a reload with new lines starts folded again.
const [opened, setOpened] = useState<{ lines: ReviewDiffLine[]; indexes: ReadonlySet<number> }>(
() => ({ lines, indexes: new Set() })
)
const openedLines = opened.lines === lines ? opened.indexes : NO_OPENED_LINES
const rows = useMemo(
() => buildMobileDiffReviewRows(lines, openedLines, new Set(commentsByLine.keys())),
[commentsByLine, lines, openedLines]
)
return (
<FlatList<MobileDiffReviewRow<ReviewDiffLine>>
data={rows}
keyExtractor={(row) => row.key}
renderItem={({ item: row }) =>
row.kind === 'fold' ? (
<Pressable
style={({ pressed }) => [styles.foldRow, pressed && styles.buttonPressed]}
onPress={() => {
const next = new Set(openedLines)
for (let index = row.startIndex; index < row.endIndex; index += 1) {
next.add(index)
}
setOpened({ lines, indexes: next })
}}
accessibilityRole="button"
accessibilityLabel={t(
'components.diffReviewBody.showUnchangedLines',
'Show {{count}} unchanged lines',
{ count: row.hiddenCount }
)}
>
<ChevronsUpDown size={13} color={colors.textMuted} strokeWidth={2.2} />
<Text style={styles.foldText}>
{row.hiddenCount === 1
? t('components.diffReviewBody.unchangedLines_one', '{{count}} unchanged line', {
count: row.hiddenCount
})
: t('components.diffReviewBody.unchangedLines_other', '{{count}} unchanged lines', {
count: row.hiddenCount
})}
</Text>
</Pressable>
) : (
<MobileDiffReviewLine
line={row.line}
comments={commentsByLine.get(row.line.newLineNumber ?? -1) ?? []}
staleCommentIds={staleCommentIds}
onAddNote={onAddNote}
onEditNote={onEditNote}
/>
)
}
contentContainerStyle={styles.diffList}
ListFooterComponent={
truncated ? (
<Text style={styles.truncatedText}>
{t('components.diffReviewBody.diffTruncated', 'Diff truncated for mobile preview.')}
</Text>
) : null
}
/>
)
}
/** Binary, oversize and deleted diffs: nothing to retry, so no action. */
function DiffUnavailableState({ diffState }: { diffState: ReviewDiffState }) {
const t = useMobileTranslation()
@@ -19,6 +19,7 @@ import { MessageBox } from './composer-input/MessageBox'
import { composerInputStyles } from './composer-input/composer-input-styles'
import { hostOs } from '../platform/host-os'
import { MobileDiffReviewFilesSheet } from './MobileDiffReviewFilesSheet'
import { MobileDiffReviewRangeSheet } from './MobileDiffReviewRangeSheet'
import { mobileDiffReviewFilesSubtitle } from '../session/mobile-diff-review-screen-model'
type Props = {
@@ -51,7 +52,7 @@ function ReviewSheetContent({ controller, sheet }: Props & { sheet: ReviewSheet
const t = useMobileTranslation()
switch (sheet.kind) {
case 'send':
return <SendNotesContent controller={controller} load={sheet.load} />
return <SendNotesContent controller={controller} load={sheet.load} only={sheet.only} />
case 'discard':
return (
<ConfirmContent
@@ -74,6 +75,8 @@ function ReviewSheetContent({ controller, sheet }: Props & { sheet: ReviewSheet
return <NoteComposerContent controller={controller} composer={sheet.composer} />
case 'completion':
return <CompletionContent controller={controller} />
case 'range':
return <MobileDiffReviewRangeSheet controller={controller} range={sheet} />
case 'files':
return (
<MobileDiffReviewFilesSheet
@@ -95,9 +98,13 @@ function ReviewSheetContent({ controller, sheet }: Props & { sheet: ReviewSheet
}
}
function SendNotesContent({ controller, load }: Props & { load: SendSheetState }) {
function SendNotesContent({
controller,
load,
only
}: Props & { load: SendSheetState; only?: readonly string[] }) {
const t = useMobileTranslation()
const sendActions = useSendActions(controller, load)
const sendActions = useSendActions(controller, load, only)
return (
<ActionSheetContent
title={t('components.diffReviewDrawers.sendNotes', 'Send Notes')}
@@ -110,11 +117,14 @@ function SendNotesContent({ controller, load }: Props & { load: SendSheetState }
function useSendActions(
controller: ReturnType<typeof useMobileDiffReviewController>,
load: SendSheetState
load: SendSheetState,
only: readonly string[] | undefined
) {
const t = useMobileTranslation()
return useMemo<ActionSheetAction[]>(() => {
const comments = controller.unsentComments
const comments = only
? controller.unsentComments.filter((comment) => only.includes(comment.id))
: controller.unsentComments
const terminalActions =
load.kind === 'ready' || load.kind === 'error'
? load.terminals.map((terminal) => ({
@@ -155,12 +165,14 @@ function useSendActions(
id: 'copyNotes',
label: t('components.diffReviewDrawers.copyNotes', 'Copy Notes'),
icon: Copy,
disabled:
controller.screenState.kind !== 'ready' || controller.screenState.comments.length === 0,
onPress: () => void controller.copyNotes()
// A sheet for one new note copies that note, not every note in the workspace.
disabled: only
? comments.length === 0
: controller.screenState.kind !== 'ready' || controller.screenState.comments.length === 0,
onPress: () => void controller.copyNotes(only ? comments : undefined)
}
]
}, [controller, load, t])
}, [controller, load, only, t])
}
function NoteComposerContent({ controller, composer }: Props & { composer: ComposerState }) {
@@ -15,6 +15,8 @@ type Props = {
staleCommentIds: ReadonlySet<string>
onEditNote: (comment: DiffComment) => void
onToggleViewed: (item: MobileDiffReviewQueueItem) => void
/** GitHub threads on this file that can't sit under their lines until the branch matches the PR. */
hiddenThreadCount: number
}
/** Uncommitted files are staged rather than viewed, and a past commit's files are read-only. */
@@ -36,7 +38,8 @@ export function MobileDiffReviewFileSummary({
item,
staleCommentIds,
onEditNote,
onToggleViewed
onToggleViewed,
hiddenThreadCount
}: Props) {
useMobileColorScheme()
const t = useMobileTranslation()
@@ -62,6 +65,21 @@ export function MobileDiffReviewFileSummary({
/>
) : null}
</View>
{hiddenThreadCount > 0 ? (
<Text style={styles.meta} numberOfLines={2}>
{hiddenThreadCount === 1
? t(
'components.diffReviewFileSummary.threadsWhenInSync_one',
'Its GitHub comment shows here once this branch matches the pull request.',
{ count: hiddenThreadCount }
)
: t(
'components.diffReviewFileSummary.threadsWhenInSync_other',
'Its {{count}} GitHub comments show here once this branch matches the pull request.',
{ count: hiddenThreadCount }
)}
</Text>
) : null}
{item.oldPath ? (
<Text style={styles.meta} numberOfLines={1}>
{t('components.diffReviewFileSummary.renamedFrom', 'Renamed from {{oldPath}}', {
@@ -178,7 +178,8 @@ function FileRow({
<Pressable
onPress={onPress}
accessibilityRole="button"
accessibilityLabel={item.filePath}
// One path can be both staged and unstaged, so the label names which one this row opens.
accessibilityLabel={`${item.filePath}, ${item.subtitle}`}
accessibilityState={{ selected: current }}
style={({ pressed }) => [styles.fileMain, pressed && pressedStyle]}
>
+18 -3
View File
@@ -1,3 +1,4 @@
import { memo } from 'react'
import { Pressable, StyleSheet, Text, View } from 'react-native'
import { MessageSquare } from 'lucide-react-native'
import type { Translate } from '../i18n/mobile-i18n'
@@ -11,10 +12,15 @@ import { themedStyleSheet } from '../theme/themed-styles'
import { useMobileColorScheme } from '../theme/use-mobile-color-scheme'
import { MobileSyntaxSegments } from './MobileSyntaxSegments'
// Each pickable line row carries this id, so a drag finds the row under the finger in the page itself.
export const DIFF_LINE_ID_PREFIX = 'orca-review-diff-line-'
type Props = {
line: MobileHighlightedDiffLine<MobileDiffLine>
comments: readonly DiffComment[]
staleCommentIds: ReadonlySet<string>
/** Part of the lines picked for a note. */
selected?: boolean
/** Absent where lines take no notes, e.g. a past commit's files. */
onAddNote?: (lineNumber: number) => void
onEditNote: (comment: DiffComment) => void
@@ -41,10 +47,11 @@ function canCommentOnLine(line: MobileDiffLine): boolean {
return line.kind !== 'delete' && line.newLineNumber !== undefined
}
export function MobileDiffReviewLine({
export const MobileDiffReviewLine = memo(function MobileDiffReviewLine({
line,
comments,
staleCommentIds,
selected = false,
onAddNote,
onEditNote
}: Props) {
@@ -58,10 +65,14 @@ export function MobileDiffReviewLine({
style={[
styles.row,
line.kind === 'add' && styles.addedRow,
line.kind === 'delete' && styles.deletedRow
line.kind === 'delete' && styles.deletedRow,
selected && styles.selectedRow
]}
accessible
accessibilityLabel={accessibilityLabelForLine(line, t)}
{...(canComment && line.newLineNumber !== undefined
? { nativeID: `${DIFF_LINE_ID_PREFIX}${line.newLineNumber}` }
: {})}
>
<Text style={styles.prefix}>{mobileDiffLinePrefix(line.kind)}</Text>
<Text style={styles.lineNumber}>{lineNumber ? String(lineNumber) : ''}</Text>
@@ -113,7 +124,7 @@ export function MobileDiffReviewLine({
) : null}
</View>
)
}
})
// Use 18px lineHeight alone without padding/minHeight to match desktop diff density (STA-1239).
const styles = themedStyleSheet(() => ({
@@ -129,6 +140,10 @@ const styles = themedStyleSheet(() => ({
deletedRow: {
backgroundColor: colors.diffDeletedBg
},
// The same blue tint the composer gives a picked skill.
selectedRow: {
backgroundColor: colors.skillTokenSurface
},
prefix: {
width: 18,
textAlign: 'center',
@@ -0,0 +1,283 @@
import { useCallback, useEffect, useMemo, useRef, useState } from 'react'
import { FlatList, Platform, Pressable, Text, View } from 'react-native'
import { Gesture, GestureDetector } from 'react-native-gesture-handler'
import { ChevronsUpDown } from 'lucide-react-native'
import type { DiffComment } from '../../../src/shared/diff-comment-types'
import { useMobileColorScheme } from '../theme/use-mobile-color-scheme'
import { useMobileTranslation } from '../i18n/use-mobile-translation'
import { DIFF_LINE_ID_PREFIX, MobileDiffReviewLine } from './MobileDiffReviewLine'
import type { ReviewDiffLine } from '../session/mobile-diff-review-screen-model'
import {
buildMobileDiffReviewRows,
type MobileDiffReviewRow
} from '../session/mobile-diff-review-rows'
import { colors } from '../theme/mobile-theme'
import { mobileDiffReviewStyles as styles } from './mobile-diff-review-screen-styles'
import type { MobileDiffReviewThread } from '../session/mobile-diff-review-github'
import type { PRCommentCardActions } from './pr-sidebar/PRCommentCard'
import { CommentGroupView } from './pr-sidebar/PRCommentsSection'
import { useNow } from '../hooks/use-now'
import { triggerSelection } from '../platform/haptics'
const NO_OPENED_LINES: ReadonlySet<number> = new Set()
const RANGE_PICK_LONG_PRESS_MS = 350
const MAX_SCROLL_ATTEMPTS = 10
const NO_LINE_NOTES: readonly DiffComment[] = []
const SCROLL_RETRY_MS = 100
export type LineRange = { startLine: number; line: number }
// Keyed by file, so folds the reader opened close again on the next file.
export function MobileDiffReviewLines({
lines,
truncated,
commentsByLine,
staleCommentIds,
threadsByLine,
threadActions,
selectedRange,
scrollToLine,
onScrolledToLine,
onPickLines,
onEditNote
}: {
lines: ReviewDiffLine[]
truncated: boolean
commentsByLine: ReadonlyMap<number, DiffComment[]>
staleCommentIds: ReadonlySet<string>
threadsByLine: ReadonlyMap<number, readonly MobileDiffReviewThread[]>
threadActions: PRCommentCardActions | undefined
/** The lines the open note sheet is about, kept highlighted under it. */
selectedRange: LineRange | null
scrollToLine: number | null
onScrolledToLine: () => void
/** Absent where lines take no notes, e.g. a past commit's files. */
onPickLines?: (range: LineRange) => void
onEditNote: (comment: DiffComment) => void
}) {
useMobileColorScheme()
const t = useMobileTranslation()
const now = useNow(60_000, threadsByLine.size > 0)
// Opened folds are line indexes into this load; a reload with new lines starts folded again.
const [opened, setOpened] = useState<{ lines: ReviewDiffLine[]; indexes: ReadonlySet<number> }>(
() => ({ lines, indexes: new Set() })
)
const openedLines = opened.lines === lines ? opened.indexes : NO_OPENED_LINES
const rows = useMemo(
() =>
buildMobileDiffReviewRows(lines, openedLines, new Set(commentsByLine.keys()), threadsByLine),
[commentsByLine, lines, openedLines, threadsByLine]
)
const openFold = (startIndex: number, endIndex: number) => {
const next = new Set(openedLines)
for (let index = startIndex; index < endIndex; index += 1) {
next.add(index)
}
setOpened({ lines, indexes: next })
}
const listRef = useRef<FlatList<MobileDiffReviewRow<ReviewDiffLine>>>(null)
const scrollFailedRef = useRef(false)
const [scrollAttempt, setScrollAttempt] = useState(0)
useEffect(() => {
if (scrollToLine === null) {
return
}
const index = rows.findIndex(
(row) =>
row.kind === 'line' && row.line.kind !== 'delete' && row.line.newLineNumber === scrollToLine
)
if (index !== -1) {
// An unmeasured row can't be scrolled to yet: jump near it, let it lay out, and try again.
scrollFailedRef.current = false
listRef.current?.scrollToIndex({ index, animated: false, viewPosition: 0.3 })
if (!scrollFailedRef.current || scrollAttempt >= MAX_SCROLL_ATTEMPTS) {
onScrolledToLine()
return
}
const retry = setTimeout(() => setScrollAttempt((attempt) => attempt + 1), SCROLL_RETRY_MS)
return () => clearTimeout(retry)
}
// A target inside folded context opens its fold first; the next rows then have it.
const lineIndex = lines.findIndex(
(line) => line.kind !== 'delete' && line.newLineNumber === scrollToLine
)
const fold = rows.find(
(row) => row.kind === 'fold' && lineIndex >= row.startIndex && lineIndex < row.endIndex
)
if (fold?.kind === 'fold') {
openFold(fold.startIndex, fold.endIndex)
} else {
onScrolledToLine()
}
}, [lines, onScrolledToLine, rows, scrollAttempt, scrollToLine])
// Long-press then drag picks lines. The screen only renders as the page (the native route mounts the web shell),
// so the row under the finger comes from the document: no cached layout can go stale as folds and threads move rows.
const [dragRange, setDragRange] = useState<{ anchor: number; line: number } | null>(null)
// The gesture's own copy, read when it ends; the state only paints the highlight.
const dragRangeRef = useRef<{ anchor: number; line: number } | null>(null)
// The release that ends a pick also reaches the starting row as a tap; that tap must not replace the range.
const pickConsumedTapRef = useRef(false)
// Browsers decide on the first touchmove whether to scroll; once a pick is live, claim it so the drag isn't cancelled.
const frameRef = useRef<View>(null)
useEffect(() => {
const node: unknown = frameRef.current
if (Platform.OS !== 'web' || !(node instanceof EventTarget)) {
return
}
const holdTouch = (event: Event) => {
if (dragRangeRef.current) {
event.preventDefault()
}
}
node.addEventListener('touchmove', holdTouch, { passive: false })
return () => node.removeEventListener('touchmove', holdTouch)
}, [])
const updateDragRange = (next: { anchor: number; line: number } | null) => {
dragRangeRef.current = next
setDragRange(next)
}
const lineAt = (x: number, y: number): number | null => {
if (typeof document === 'undefined') {
return null
}
const row = document.elementFromPoint(x, y)?.closest(`[id^="${DIFF_LINE_ID_PREFIX}"]`)
const line = row ? Number(row.id.slice(DIFF_LINE_ID_PREFIX.length)) : Number.NaN
return Number.isInteger(line) ? line : null
}
const pick = Gesture.Pan()
.enabled(onPickLines !== undefined)
.activateAfterLongPress(RANGE_PICK_LONG_PRESS_MS)
.shouldCancelWhenOutside(false)
.runOnJS(true)
// A new touch starts fresh, so a pick whose release sent no tap can't swallow the next real one.
.onBegin(() => {
pickConsumedTapRef.current = false
})
.onStart((event) => {
const line = lineAt(event.absoluteX, event.absoluteY)
if (line !== null) {
triggerSelection()
updateDragRange({ anchor: line, line })
}
})
.onUpdate((event) => {
const line = lineAt(event.absoluteX, event.absoluteY)
const prev = dragRangeRef.current
if (line !== null && prev && prev.line !== line) {
updateDragRange({ ...prev, line })
}
})
.onFinalize((_event, success) => {
const picked = dragRangeRef.current
updateDragRange(null)
if (picked) {
pickConsumedTapRef.current = true
}
// A cancelled drag (the browser took the touch, say) picks nothing.
if (picked && success) {
onPickLines?.({
startLine: Math.min(picked.anchor, picked.line),
line: Math.max(picked.anchor, picked.line)
})
}
})
// Stable, so a drag step re-renders only the rows whose highlight changed.
const pickOneLine = useCallback(
(line: number) => {
if (pickConsumedTapRef.current) {
pickConsumedTapRef.current = false
return
}
onPickLines?.({ startLine: line, line })
},
[onPickLines]
)
const highlighted: LineRange | null = dragRange
? {
startLine: Math.min(dragRange.anchor, dragRange.line),
line: Math.max(dragRange.anchor, dragRange.line)
}
: selectedRange
return (
<GestureDetector gesture={pick}>
<View ref={frameRef} style={styles.diffListFrame}>
<FlatList<MobileDiffReviewRow<ReviewDiffLine>>
ref={listRef}
data={rows}
// The list holds still while lines are being picked, so the drag selects instead of scrolling.
scrollEnabled={dragRange === null}
onScrollToIndexFailed={(info) => {
scrollFailedRef.current = true
listRef.current?.scrollToOffset({
offset: Math.max(0, info.averageItemLength * info.index),
animated: false
})
}}
keyExtractor={(row) => row.key}
renderItem={({ item: row }) =>
row.kind === 'fold' ? (
<Pressable
style={({ pressed }) => [styles.foldRow, pressed && styles.buttonPressed]}
onPress={() => openFold(row.startIndex, row.endIndex)}
accessibilityRole="button"
accessibilityLabel={t(
'components.diffReviewBody.showUnchangedLines',
'Show {{count}} unchanged lines',
{ count: row.hiddenCount }
)}
>
<ChevronsUpDown size={13} color={colors.textMuted} strokeWidth={2.2} />
<Text style={styles.foldText}>
{row.hiddenCount === 1
? t(
'components.diffReviewBody.unchangedLines_one',
'{{count}} unchanged line',
{
count: row.hiddenCount
}
)
: t(
'components.diffReviewBody.unchangedLines_other',
'{{count}} unchanged lines',
{ count: row.hiddenCount }
)}
</Text>
</Pressable>
) : row.kind === 'thread' ? (
<View style={styles.threadRow}>
<CommentGroupView group={row.thread} actions={threadActions} now={now} />
</View>
) : (
<MobileDiffReviewLine
line={row.line}
comments={commentsByLine.get(row.line.newLineNumber ?? -1) ?? NO_LINE_NOTES}
staleCommentIds={staleCommentIds}
selected={
highlighted !== null &&
row.line.kind !== 'delete' &&
row.line.newLineNumber !== undefined &&
row.line.newLineNumber >= highlighted.startLine &&
row.line.newLineNumber <= highlighted.line
}
{...(onPickLines ? { onAddNote: pickOneLine } : {})}
onEditNote={onEditNote}
/>
)
}
contentContainerStyle={styles.diffList}
ListFooterComponent={
truncated ? (
<Text style={styles.truncatedText}>
{t('components.diffReviewBody.diffTruncated', 'Diff truncated for mobile preview.')}
</Text>
) : null
}
/>
</View>
</GestureDetector>
)
}
@@ -0,0 +1,152 @@
import { useEffect, useState } from 'react'
import { KeyboardAvoidingView, Pressable, Text, View } from 'react-native'
import { Sparkles } from 'lucide-react-native'
import { getRuntimePathBasename } from '../../../src/shared/cross-platform-path'
import { tuiAgentDisplayName } from '../../../src/shared/tui-agent-display-names'
import { useMobileTranslation } from '../i18n/use-mobile-translation'
import { hostOs } from '../platform/host-os'
import { canCommentOnGitHub } from '../session/mobile-diff-review-github'
import type { useMobileDiffReviewController } from '../session/use-mobile-diff-review-controller'
import { colors } from '../theme/mobile-theme'
import { useMobileColorScheme } from '../theme/use-mobile-color-scheme'
import { TextField } from './composer-input/TextField'
import { mobileDiffReviewStyles as styles } from './mobile-diff-review-screen-styles'
import { SheetHeader } from './nav-bar/SheetHeader'
import type { ReviewSheet } from '../session/mobile-diff-review-sheets'
import type { MobileReviewTerminalTab } from '../session/review-terminal-reply-schema'
type Props = {
controller: ReturnType<typeof useMobileDiffReviewController>
}
// Lines picked in the diff: a note for the workspace's agent, or a comment straight on GitHub.
export function MobileDiffReviewRangeSheet({
controller,
range
}: Props & { range: Extract<ReviewSheet, { kind: 'range' }> }) {
useMobileColorScheme()
const t = useMobileTranslation()
const item = controller.currentItem
const { agentName, terminals } = useAgentTerminals(controller)
const [busy, setBusy] = useState<'github' | 'agent' | null>(null)
const [postError, setPostError] = useState<string | null>(null)
const body = controller.composerBody.trim()
const onGitHub = canCommentOnGitHub(controller.github, item, range)
const title =
range.startLine === range.line
? t('components.diffReviewDrawers.rangeLine', 'Line {{line}}', { line: range.line })
: t('components.diffReviewDrawers.rangeLines', 'Lines {{startLine}}–{{line}}', {
startLine: range.startLine,
line: range.line
})
const sendLabel = agentName
? t('components.diffReviewDrawers.sendToAgent', 'Send to {{agent}}', { agent: agentName })
: t('components.diffReviewDrawers.sendToAnAgent', 'Send to agent')
return (
<KeyboardAvoidingView behavior={hostOs() === 'ios' ? 'padding' : undefined}>
<SheetHeader
title={title}
{...(item ? { subtitle: getRuntimePathBasename(item.filePath) } : {})}
trailing={{
label: t('common.cancel', 'Cancel'),
onPress: () => controller.closeSheet('range', range)
}}
/>
<TextField
value={controller.composerBody}
onChangeText={controller.setComposerBody}
autoFocus
multiline
placeholder={t('components.diffReviewDrawers.rangePlaceholder', 'What should change here?')}
accessibilityLabel={title}
style={styles.rangeField}
/>
{postError ? <Text style={styles.rangeError}>{postError}</Text> : null}
<View style={styles.drawerButtonRow}>
{onGitHub && item ? (
<Pressable
style={({ pressed }) => [
styles.secondaryButton,
styles.sheetSecondaryButton,
(busy !== null || !body) && styles.buttonDisabled,
pressed && styles.buttonPressed
]}
disabled={busy !== null || !body}
onPress={() => {
setBusy('github')
setPostError(null)
void controller
.commentOnGitHub({ item, startLine: range.startLine, line: range.line, body })
.then((posted) => {
if (posted.ok) {
// Only this sheet; one opened since keeps its own draft.
controller.closeSheet('range', range)
} else {
setPostError(posted.error)
}
})
.finally(() => setBusy(null))
}}
accessibilityRole="button"
accessibilityLabel={t(
'components.diffReviewDrawers.commentOnGitHub',
'Comment on GitHub'
)}
>
<Text style={styles.secondaryButtonText}>
{t('components.diffReviewDrawers.commentOnGitHub', 'Comment on GitHub')}
</Text>
</Pressable>
) : null}
<Pressable
style={({ pressed }) => [
styles.primaryButton,
(busy !== null || !body) && styles.buttonDisabled,
pressed && styles.buttonPressed
]}
disabled={busy !== null || !body}
onPress={() => {
setBusy('agent')
setPostError(null)
void controller
.sendRange(range, terminals ?? undefined)
.then(setPostError)
.finally(() => setBusy(null))
}}
accessibilityRole="button"
accessibilityLabel={sendLabel}
>
<Sparkles size={16} color={colors.bgBase} strokeWidth={2.2} />
<Text style={styles.primaryButtonText} numberOfLines={1}>
{sendLabel}
</Text>
</Pressable>
</View>
</KeyboardAvoidingView>
)
}
// The workspace's terminals, listed once per sheet: Send reuses them, and names the button after the one agent.
function useAgentTerminals(controller: ReturnType<typeof useMobileDiffReviewController>) {
const [terminals, setTerminals] = useState<readonly MobileReviewTerminalTab[] | null>(null)
const loadTerminals = controller.loadTerminals
useEffect(() => {
let active = true
void loadTerminals()
.then((listed) => {
if (active) {
setTerminals(listed)
}
})
// The button still works without a name; sending lists terminals again and reports its own failure.
.catch(() => {})
return () => {
active = false
}
}, [loadTerminals])
const agents = terminals?.filter((terminal) => terminal.agent !== undefined) ?? []
const [only] = agents
const agentName =
agents.length === 1 && only?.agent ? (tuiAgentDisplayName(only.agent) ?? only.title) : null
return { agentName, terminals }
}
@@ -4,6 +4,8 @@ import { Text, View, type LayoutChangeEvent } from 'react-native'
import type { useMobileDiffReviewController } from '../session/use-mobile-diff-review-controller'
import { useResponsiveLayout } from '../layout/responsive-layout'
import { MobileDiffReviewBody } from './MobileDiffReviewBody'
import { useMobilePrCommentActions } from '../session/use-mobile-pr-comment-actions'
import { usePRCommentCardActions } from './pr-sidebar/PRCommentsSection'
import { MobileDiffReviewDrawers } from './MobileDiffReviewDrawers'
import { MobileDiffReviewFileSummary } from './MobileDiffReviewFileSummary'
import { MobileDiffReviewFooter } from './MobileDiffReviewFooter'
@@ -47,6 +49,24 @@ export function MobileDiffReviewScreenView({ controller, onBack }: Props) {
})
})
const item = controller.currentItem
const github = controller.github
const refetchPR = controller.refetchPRSidebar
const refetchThreads = useCallback(() => refetchPR({ includeDetails: true }), [refetchPR])
const threadCommentActions = useMobilePrCommentActions({
client: controller.client,
connState: controller.connState,
worktreeId: controller.worktreeId,
prNumber: github?.prNumber ?? 0,
prRepo: github?.prRepo ?? null,
refetch: refetchThreads
})
const threadActions = usePRCommentCardActions(
github ? threadCommentActions : undefined,
github?.prRepo
)
const sheet = controller.sheet
const selectedRange =
sheet?.kind === 'range' ? { startLine: sheet.startLine, line: sheet.line } : null
// Dock GitHub sidebar only when wide; otherwise showPRSidebar toggles RightDrawer overlay.
const showInlineDock = presentationMode === 'inline' && controller.prSidebarIsGithubRepo
const gitStatus = controller.screenState.kind === 'ready' ? controller.screenState.status : null
@@ -93,8 +113,15 @@ export function MobileDiffReviewScreenView({ controller, onBack }: Props) {
staleCommentIds={controller.staleCommentIds}
onEditNote={controller.openEditComposer}
onToggleViewed={(target) => void controller.toggleItemViewed(target)}
hiddenThreadCount={controller.hiddenThreadCount}
/>
) : null}
{/* Each source keeps its own line, so a newer thread failure is never hidden behind older feedback. */}
{threadCommentActions.error ? (
<View style={styles.actionError}>
<Text style={styles.actionErrorText}>{threadCommentActions.error}</Text>
</View>
) : null}
{controller.actionError ? (
<View style={styles.actionError}>
<Text style={styles.actionErrorText}>{controller.actionError}</Text>
@@ -107,7 +134,15 @@ export function MobileDiffReviewScreenView({ controller, onBack }: Props) {
filteredCount={controller.filteredQueue.length}
screenState={controller.screenState}
staleCommentIds={controller.staleCommentIds}
onAddNote={controller.openComposer}
threadsByLine={controller.threadsByLine}
threadActions={threadActions}
selectedRange={selectedRange}
onPickLines={(range) => {
controller.setComposerBody('')
controller.openSheet({ kind: 'range', ...range })
}}
scrollToLine={controller.scrollToLine}
onScrolledToLine={controller.clearScrollToLine}
onEditNote={controller.openEditComposer}
onRetry={controller.retryAction ?? undefined}
offline={controller.connState !== 'connected'}
@@ -92,6 +92,15 @@ export const mobileDiffReviewControlStyles = themedStyleSheet(() => ({
fontSize: typography.metaSize,
marginTop: 2
},
rangeField: {
minHeight: 96,
alignItems: 'flex-start'
},
rangeError: {
marginTop: spacing.sm,
color: colors.statusRed,
fontSize: typography.metaSize
},
drawerButtonRow: {
flexDirection: 'row',
gap: spacing.sm,
@@ -68,6 +68,14 @@ export const mobileDiffReviewLayoutStyles = themedStyleSheet(() => ({
color: colors.textMuted,
fontSize: typography.metaSize
},
diffListFrame: {
flex: 1
},
threadRow: {
paddingHorizontal: spacing.md,
paddingVertical: spacing.sm,
backgroundColor: colors.bgBase
},
diffList: {
paddingBottom: 140,
backgroundColor: colors.editorSurface
@@ -52,7 +52,12 @@ vi.mock('../session/mobile-diff-review-loaders', () => ({
loadMobileDiffReviewDiff: vi.fn().mockResolvedValue({ kind: 'idle' })
}))
vi.mock('../session/use-mobile-pr-sidebar-controller', () => ({
useMobilePrSidebarController: () => ({})
useMobilePrSidebarController: () => ({
prSidebarState: { kind: 'hidden' },
prSidebarIsGithubRepo: false,
refetchPRSidebar: async () => {},
ensurePrSidebarDetails: async () => {}
})
}))
const { MobileDiffReviewDrawers } = await import('./MobileDiffReviewDrawers')
@@ -86,24 +86,7 @@ export function PRCommentsSection({
)
const isPr = details != null && !detailsFailed && details.item.type === 'pr'
// Memoize stable per-card callback bundles so unrelated timeline updates skip cards.
const cardActions = useMemo<PRCommentCardActions | undefined>(
() =>
actions && isPr
? {
reply: actions.reply,
toggleResolve: actions.toggleResolve,
editComment: actions.editComment,
deleteComment: actions.deleteComment,
isReplyBusy: actions.isReplyBusy,
isResolveBusy: actions.isResolveBusy,
isEditBusy: actions.isEditBusy,
isDeleteBusy: actions.isDeleteBusy,
prRepo: prRepo ?? null
}
: undefined,
[actions, isPr, prRepo]
)
const cardActions = usePRCommentCardActions(isPr ? actions : undefined, prRepo)
const canComment = isPr && actions !== undefined && canAddRootComment(prState)
const [filter, setFilter] = useState<PRCommentAudienceFilter>('all')
@@ -266,7 +249,32 @@ export function PRCommentsSection({
)
}
function CommentGroupView({
// Memoize stable per-card callback bundles so unrelated timeline updates skip cards.
export function usePRCommentCardActions(
actions: MobilePrCommentActions | undefined,
prRepo: GitHubPrRepoSlug | null | undefined
): PRCommentCardActions | undefined {
return useMemo<PRCommentCardActions | undefined>(
() =>
actions
? {
reply: actions.reply,
toggleResolve: actions.toggleResolve,
editComment: actions.editComment,
deleteComment: actions.deleteComment,
isReplyBusy: actions.isReplyBusy,
isResolveBusy: actions.isResolveBusy,
isEditBusy: actions.isEditBusy,
isDeleteBusy: actions.isDeleteBusy,
prRepo: prRepo ?? null
}
: undefined,
[actions, prRepo]
)
}
/** One standalone comment or thread; resolved threads collapse. The diff screen shows threads under their line with it. */
export function CommentGroupView({
group,
actions,
now
+4
View File
@@ -140,6 +140,10 @@
"viewedFiles_one": "{{count}} file viewed",
"viewedFiles_other": "{{count}} files viewed"
},
"diffReviewFileSummary": {
"threadsWhenInSync_one": "Its GitHub comment shows here once this branch matches the pull request.",
"threadsWhenInSync_other": "Its {{count}} GitHub comments show here once this branch matches the pull request."
},
"worktreeAgentList": {
"hideChildAgents_one": "Hide {{count}} child agent",
"hideChildAgents_other": "Hide {{count}} child agents",
+15 -3
View File
@@ -993,7 +993,13 @@
"unstageFile": "Unstage",
"addFileNote": "Add File Note",
"pullRequest": "Pull Request",
"discardChanges": "Discard Changes"
"discardChanges": "Discard Changes",
"rangeLine": "Line {{line}}",
"rangeLines": "Lines {{startLine}}–{{line}}",
"sendToAgent": "Send to {{agent}}",
"sendToAnAgent": "Send to agent",
"rangePlaceholder": "What should change here?",
"commentOnGitHub": "Comment on GitHub"
},
"diffReviewFileSummary": {
"changed": "Changed",
@@ -1002,7 +1008,9 @@
"untracked": "Untracked",
"notStaged": "Not staged",
"viewed": "Viewed",
"renamedFrom": "Renamed from {{oldPath}}"
"renamedFrom": "Renamed from {{oldPath}}",
"threadsWhenInSync_one": "Its GitHub comment shows here once this branch matches the pull request.",
"threadsWhenInSync_other": "Its {{count}} GitHub comments show here once this branch matches the pull request."
},
"diffReviewFooter": {
"previousFile": "Previous file",
@@ -1983,7 +1991,8 @@
},
"githubPrMutations": {
"titleUpdateFailed": "Failed to update title.",
"reviewThreadUpdateFailed": "Failed to update review thread."
"reviewThreadUpdateFailed": "Failed to update review thread.",
"fileViewedFailed": "Failed to update viewed state."
},
"githubPrRpc": {
"requestFailed": "Request failed: {{method}}"
@@ -2615,6 +2624,9 @@
"diffFailed": "Couldn't load diff preview",
"fileFailed": "Couldn't load file preview",
"outsideWorkspace": "This file is outside the workspace. Open it on desktop to view it."
},
"useDiffReviewGitHub": {
"waitingForDesktop": "Waiting for desktop..."
}
},
"chat": {
@@ -18,6 +18,7 @@ export function MobileDiffReviewRouteScreen() {
file?: string | string[]
area?: string | string[]
commit?: string | string[]
line?: string | string[]
}>()
const hostId = firstReviewParam(params.hostId)
const worktreeId = firstReviewParam(params.worktreeId)
@@ -26,6 +27,8 @@ export function MobileDiffReviewRouteScreen() {
const initialArea = normalizeReviewAreaParam(firstReviewParam(params.area))
const initialFilter = initialArea ? scopeForTargetArea(initialArea) : 'all'
const commitOid = firstReviewParam(params.commit) || null
const lineParam = Number.parseInt(firstReviewParam(params.line), 10)
const initialLine = Number.isInteger(lineParam) && lineParam > 0 ? lineParam : null
const initialTarget = useMemo(
() => (initialFile && initialArea ? { filePath: initialFile, area: initialArea } : null),
[initialArea, initialFile]
@@ -55,6 +58,7 @@ export function MobileDiffReviewRouteScreen() {
initialFilter,
initialTarget,
commitOid,
initialLine,
onOpenSession: openSession,
onReconnect: forceReconnect
})
@@ -1,8 +1,9 @@
import { z } from 'zod'
import { salvagedOptional, salvagingArray } from '../../../src/shared/zod-salvage'
import { openEnum, salvagedOptional, salvagingArray } from '../../../src/shared/zod-salvage'
import type { GitHubReaction, PRComment } from '../../../src/shared/github/comment-types'
import type {
GitHubAssignableUser,
GitHubPRFile,
GitHubPRMergeMethodSettings,
GitHubPRReviewSummary,
GitHubRepositoryIdentity,
@@ -239,3 +240,43 @@ export const prChecksSchema = salvagingArray(checkDetailSchema)
export function prCheckList(name: string) {
return salvagedOptional(name, prChecksSchema)
}
const PR_FILE_STATUS = [
'added',
'modified',
'removed',
'renamed',
'copied',
'changed',
'unchanged'
] as const
// The diff screen reads viewed state and commentable lines per path; a malformed file drops alone.
export const prFileSchema = z
.looseObject({
path: z.string().min(1),
oldPath: salvagedOptional('oldPath', z.string()),
status: openEnum(PR_FILE_STATUS, 'modified').catch('modified'),
additions: z.number().catch(0),
deletions: z.number().catch(0),
isBinary: z.boolean().catch(false),
reviewCommentLineNumbers: salvagedOptional(
'reviewCommentLineNumbers',
z.array(z.number().int().positive())
),
viewerViewedState: openEnum(['VIEWED', 'UNVIEWED', 'DISMISSED'] as const, undefined)
.optional()
.catch(undefined)
})
.transform((file): GitHubPRFile => ({
path: file.path,
...(file.oldPath ? { oldPath: file.oldPath } : {}),
status: file.status,
additions: file.additions,
deletions: file.deletions,
isBinary: file.isBinary,
...(file.reviewCommentLineNumbers
? { reviewCommentLineNumbers: file.reviewCommentLineNumbers }
: {}),
...(file.viewerViewedState ? { viewerViewedState: file.viewerViewedState } : {})
}))
@@ -69,6 +69,11 @@ export const githubPrReviewCommentReplyAdd = mutationStatusOperation(
'github.addPRReviewCommentReply'
)
export const githubPrReviewCommentAdd = mutationStatusOperation(
'github.add-pr-review-comment',
'github.addPRReviewComment'
)
export const githubPrIssueCommentAdd = mutationStatusOperation(
'github.add-issue-comment',
'github.addIssueComment'
@@ -104,3 +109,14 @@ export const githubPrReviewThreadResolve = bindDeferredRpcOperation(
read: rpcResultVariant('pr-mutation-confirmation', githubPrMutationConfirmationSchema)
})
)
// GitHub's viewed mark answers a bare boolean; false means it was not applied.
export const githubPrFileViewedSet = bindDeferredRpcOperation(
defineRpcOperation({
name: 'github.set-pr-file-viewed',
method: 'github.setPRFileViewed',
acceptance: 'require-result-or-throw-message',
barrier: 'after-caller-barrier',
read: rpcResultVariant('pr-mutation-confirmation', githubPrMutationConfirmationSchema)
})
)
@@ -12,6 +12,7 @@ import {
PR_STATE,
REVIEW_DECISION,
prCheckList,
prFileSchema,
prCheckSummary,
prCommentList,
prCount,
@@ -261,7 +262,9 @@ export const githubWorkItemDetailsSchema = z
checks: prCheckList('checks'),
participants: prUserList('participants'),
// Absent assignees means unreported, distinct from a confirmed empty list.
assignees: salvagedOptional('assignees', salvagingArray(z.string()))
assignees: salvagedOptional('assignees', salvagingArray(z.string())),
// Older hosts send no files; the diff screen then keeps viewed state on the phone.
files: salvagedOptional('files', salvagingArray(prFileSchema))
})
.transform((details): GitHubWorkItemDetails | null =>
details.item === null
@@ -275,7 +278,8 @@ export const githubWorkItemDetailsSchema = z
pullRequestId: details.pullRequestId,
checks: details.checks ?? [],
participants: details.participants ?? [],
assignees: details.assignees
assignees: details.assignees,
...(details.files ? { files: details.files } : {})
}
)
.nullable()
@@ -0,0 +1,71 @@
import { githubPrFileViewedSet, githubPrReviewCommentAdd } from './github-pr-mutation-operations'
import {
settleGithubPrConfirmation,
settleGithubPrMutation,
type GitHubPrMutationOutcome
} from './github-pr-mutation-outcome'
import { githubPrRequestParams, type GitHubPrRepoSlug } from './github-pr-repo-slug'
import type { RpcOperationSender } from '../transport/rpc-operation-sender'
import { translate } from '../i18n/mobile-i18n'
// The diff screen's writes on a PR file: a new line comment and its viewed mark.
// A new line comment on the PR's head commit; GitHub only accepts lines inside the file's diff hunks.
export function fetchAddPRReviewComment(
client: RpcOperationSender,
worktreeId: string,
args: {
prNumber: number
commitId: string
path: string
line: number
startLine?: number
body: string
prRepo?: GitHubPrRepoSlug | null
}
): Promise<GitHubPrMutationOutcome> {
const params: Record<string, unknown> = {
prNumber: args.prNumber,
commitId: args.commitId,
path: args.path,
line: args.line,
body: args.body
}
if (args.startLine !== undefined && args.startLine !== args.line) {
params.startLine = args.startLine
}
return settleGithubPrMutation(githubPrReviewCommentAdd, () =>
githubPrReviewCommentAdd.request(
client,
githubPrRequestParams(githubPrReviewCommentAdd.operation.method, worktreeId, params, {
prRepo: args.prRepo
})
)
)
}
export function fetchSetPRFileViewed(
client: RpcOperationSender,
worktreeId: string,
args: {
pullRequestId: string
path: string
viewed: boolean
prRepo?: GitHubPrRepoSlug | null
}
): Promise<GitHubPrMutationOutcome> {
return settleGithubPrConfirmation(
githubPrFileViewedSet,
() =>
githubPrFileViewedSet.request(
client,
githubPrRequestParams(
githubPrFileViewedSet.operation.method,
worktreeId,
{ pullRequestId: args.pullRequestId, path: args.path, viewed: args.viewed },
{ prRepo: args.prRepo }
)
),
translate('session.githubPrMutations.fileViewedFailed', 'Failed to update viewed state.')
)
}
@@ -1,9 +1,15 @@
import type { DiffComment, DiffReviewScope } from '../../../src/shared/diff-comment-types'
import {
mobileDiffReviewCommentMatchesItem,
type MobileDiffReviewQueueItem
} from './mobile-diff-review-queue'
export type CreateMobileDiffCommentInput = {
worktreeId: string
filePath: string
oldPath?: string
/** Inclusive range start, below lineNumber; absent for a single line. */
startLine?: number
lineNumber: number
body: string
id: string
@@ -71,6 +77,9 @@ export function createMobileDiffComment(input: CreateMobileDiffCommentInput): Di
filePath: input.filePath,
oldPath: input.oldPath,
source: 'diff',
...(input.startLine !== undefined && input.startLine < input.lineNumber
? { startLine: input.startLine }
: {}),
lineNumber: input.lineNumber,
body,
createdAt: input.createdAt,
@@ -133,3 +142,28 @@ export function sendableMobileDiffComments(
): DiffComment[] {
return comments.filter((comment) => !comment.sentAt && !sendingIds.has(comment.id))
}
/** The open file's notes by line, and which were written against an older version of its diff. */
export function mobileDiffReviewNotesForItem(
comments: readonly DiffComment[],
item: MobileDiffReviewQueueItem | null
): { commentsByLine: Map<number, DiffComment[]>; staleCommentIds: Set<string> } {
const commentsByLine = new Map<number, DiffComment[]>()
const staleCommentIds = new Set<string>()
if (!item) {
return { commentsByLine, staleCommentIds }
}
for (const comment of comments) {
if (!mobileDiffReviewCommentMatchesItem(comment, item)) {
continue
}
commentsByLine.set(comment.lineNumber, [
...(commentsByLine.get(comment.lineNumber) ?? []),
comment
])
if (comment.diffIdentity !== undefined && comment.diffIdentity !== item.diffIdentity) {
staleCommentIds.add(comment.id)
}
}
return { commentsByLine, staleCommentIds }
}
@@ -0,0 +1,130 @@
import { describe, expect, it } from 'vitest'
import type { PRComment } from '../../../src/shared/github/comment-types'
import type { GitHubWorkItemDetails } from '../../../src/shared/github/work-item-types'
import { buildMobileDiffReviewQueue } from './mobile-diff-review-queue'
import {
applyMobileDiffReviewGitHubViewed,
buildMobileDiffReviewGitHub,
canCommentOnGitHub
} from './mobile-diff-review-github'
function comment(overrides: Partial<PRComment>): PRComment {
return {
id: 1,
author: 'nwparker',
authorAvatarUrl: '',
body: 'Use radii.surface here.',
createdAt: '2026-11-01T00:00:00Z',
url: 'https://github.com/o/r/pull/1#discussion_r1',
...overrides
}
}
function details(overrides: Partial<GitHubWorkItemDetails>): GitHubWorkItemDetails {
return {
item: {
id: 'pr-1',
type: 'pr',
number: 1,
title: 'PR',
state: 'open',
url: 'https://github.com/o/r/pull/1',
labels: [],
updatedAt: '2026-11-01T00:00:00Z',
author: 'kelvin'
},
body: '',
comments: [],
headSha: 'head',
pullRequestId: 'PR_node',
files: [
{
path: 'src/a.ts',
status: 'modified',
additions: 2,
deletions: 1,
isBinary: false,
reviewCommentLineNumbers: [10, 11, 12],
viewerViewedState: 'DISMISSED'
}
],
...overrides
}
}
const branchItem = buildMobileDiffReviewQueue({
worktreeId: 'wt',
statusEntries: [],
branchEntries: [{ path: 'src/a.ts', status: 'modified', added: 2, removed: 1 }],
comments: [],
reviewState: { version: 1, files: {} }
})
describe('the diff screen’s GitHub pull request', () => {
it('puts each current thread under its end line and leaves outdated ones out', () => {
const github = buildMobileDiffReviewGitHub({
prNumber: 1,
prRepo: null,
localHeadOid: 'head',
details: details({
comments: [
comment({ id: 1, path: 'src/a.ts', line: 12, startLine: 10, threadId: 'T1' }),
comment({ id: 2, path: 'src/a.ts', line: 12, threadId: 'T1' }),
comment({ id: 3, path: 'src/a.ts', line: 4, threadId: 'T2', isOutdated: true })
]
})
})
const threads = github.threadsByPath.get('src/a.ts')
expect([...(threads?.keys() ?? [])]).toEqual([12])
expect(threads?.get(12)?.[0]?.replies.map((reply) => reply.id)).toEqual([2])
})
it('takes viewed state for the PR’s files from GitHub', () => {
const github = buildMobileDiffReviewGitHub({
prNumber: 1,
prRepo: null,
localHeadOid: 'head',
details: details({})
})
expect(applyMobileDiffReviewGitHubViewed(branchItem, github)[0]).toMatchObject({
isReviewed: false,
changedSinceReview: true
})
})
it('leaves viewed state on the phone when GitHub sent none for the file', () => {
const github = buildMobileDiffReviewGitHub({
prNumber: 1,
prRepo: null,
localHeadOid: 'head',
details: details({
pullRequestId: undefined,
files: [
{ path: 'src/a.ts', status: 'modified', additions: 2, deletions: 1, isBinary: false }
]
})
})
const viewedLocally = branchItem.map((item) => ({ ...item, isReviewed: true }))
expect(applyMobileDiffReviewGitHubViewed(viewedLocally, github)[0]?.isReviewed).toBe(true)
})
it('offers Comment on GitHub only on commentable lines while the branch is at the PR head', () => {
const atHead = buildMobileDiffReviewGitHub({
prNumber: 1,
prRepo: null,
localHeadOid: 'head',
details: details({})
})
const item = branchItem[0] ?? null
expect(canCommentOnGitHub(atHead, item, { startLine: 10, line: 12 })).toBe(true)
expect(canCommentOnGitHub(atHead, item, { startLine: 12, line: 13 })).toBe(false)
const ahead = buildMobileDiffReviewGitHub({
prNumber: 1,
prRepo: null,
localHeadOid: 'unpushed',
details: details({})
})
expect(ahead.linesMatch).toBe(false)
expect(canCommentOnGitHub(ahead, item, { startLine: 10, line: 10 })).toBe(false)
})
})
@@ -0,0 +1,136 @@
import type { GitHubPRFile } from '../../../src/shared/github/pull-request-types'
import type { GitHubWorkItemDetails } from '../../../src/shared/github/work-item-types'
import { getPRInlineCommentPlacement } from '../../../src/shared/github/pr-inline-comment-placement'
import { groupPRComments, type PRCommentGroup } from '../../../src/shared/pr-comment-groups'
import type { GitHubPrRepoSlug } from './github-pr-rpc'
import type { MobileDiffReviewQueueItem } from './mobile-diff-review-queue'
export type MobileDiffReviewThread = Extract<PRCommentGroup, { kind: 'thread' }>
/** What the diff screen knows about the branch's GitHub pull request. */
export type MobileDiffReviewGitHub = {
prNumber: number
prRepo: GitHubPrRepoSlug | null
pullRequestId: string | null
headSha: string | null
/**
* The local branch is at the PR's head, so local line numbers are GitHub's. Desktop shows threads
* only on the PR's own contents; the local diff is those contents only when the heads match.
*/
linesMatch: boolean
/** Null on hosts that don't send PR files; viewed state then stays on the phone. */
filesByPath: ReadonlyMap<string, GitHubPRFile> | null
threadsByPath: ReadonlyMap<string, ReadonlyMap<number, MobileDiffReviewThread[]>>
}
export function buildMobileDiffReviewGitHub(input: {
prNumber: number
prRepo: GitHubPrRepoSlug | null
details: GitHubWorkItemDetails
localHeadOid: string | null
}): MobileDiffReviewGitHub {
const { details } = input
const headSha = details.headSha ?? null
const threadsByPath = new Map<string, Map<number, MobileDiffReviewThread[]>>()
for (const group of groupPRComments(details.comments)) {
if (group.kind !== 'thread') {
continue
}
// Desktop's rule: outdated threads and file-level comments don't sit under a line.
const placement = getPRInlineCommentPlacement(group.root)
if (!placement) {
continue
}
const byLine = threadsByPath.get(placement.path) ?? new Map<number, MobileDiffReviewThread[]>()
byLine.set(placement.line, [...(byLine.get(placement.line) ?? []), group])
threadsByPath.set(placement.path, byLine)
}
return {
prNumber: input.prNumber,
prRepo: input.prRepo,
pullRequestId: details.pullRequestId ?? null,
headSha,
linesMatch: headSha !== null && headSha === input.localHeadOid,
filesByPath: details.files ? new Map(details.files.map((file) => [file.path, file])) : null,
threadsByPath
}
}
/**
* The PR file whose viewed state GitHub owns: GitHub reported it, the PR can be marked, and the
* local diff is the PR's version (the threads rule); otherwise the phone keeps its own state.
*/
export function gitHubViewedFile(
github: MobileDiffReviewGitHub | null,
item: MobileDiffReviewQueueItem
): GitHubPRFile | null {
if (!github?.pullRequestId || !github.linesMatch || item.scope !== 'branch') {
return null
}
const file = github.filesByPath?.get(item.filePath)
return file?.viewerViewedState !== undefined ? file : null
}
/** GitHub owns viewed state for the PR's files; DISMISSED means it changed after it was viewed. */
export function applyMobileDiffReviewGitHubViewed(
queue: readonly MobileDiffReviewQueueItem[],
github: MobileDiffReviewGitHub | null
): MobileDiffReviewQueueItem[] {
return queue.map((item) => {
const file = gitHubViewedFile(github, item)
return file
? {
...item,
isReviewed: file.viewerViewedState === 'VIEWED',
changedSinceReview: file.viewerViewedState === 'DISMISSED'
}
: item
})
}
/** Whether GitHub takes a review comment on every line of the range: only lines inside the file's diff hunks. */
export function canCommentOnGitHub(
github: MobileDiffReviewGitHub | null,
item: MobileDiffReviewQueueItem | null,
range: { startLine: number; line: number }
): boolean {
if (!github?.linesMatch || !github.headSha || !item || item.scope !== 'branch') {
return false
}
const commentable = github.filesByPath?.get(item.filePath)?.reviewCommentLineNumbers
if (!commentable) {
return false
}
const lines = new Set(commentable)
for (let line = range.startLine; line <= range.line; line += 1) {
if (!lines.has(line)) {
return false
}
}
return true
}
const NO_THREADS: ReadonlyMap<number, readonly MobileDiffReviewThread[]> = new Map()
/** The open file's threads by line, or how many are waiting until the branch matches the PR head. */
export function mobileDiffReviewThreadsForItem(
github: MobileDiffReviewGitHub | null,
item: MobileDiffReviewQueueItem | null
): {
threadsByLine: ReadonlyMap<number, readonly MobileDiffReviewThread[]>
hiddenThreadCount: number
} {
const byLine =
github && item?.scope === 'branch' ? github.threadsByPath.get(item.filePath) : undefined
if (!byLine) {
return { threadsByLine: NO_THREADS, hiddenThreadCount: 0 }
}
if (github?.linesMatch) {
return { threadsByLine: byLine, hiddenThreadCount: 0 }
}
let hiddenThreadCount = 0
for (const threads of byLine.values()) {
hiddenThreadCount += threads.length
}
return { threadsByLine: NO_THREADS, hiddenThreadCount }
}
@@ -17,6 +17,7 @@ vi.mock('react-native', () => ({
View: 'View'
}))
vi.mock('lucide-react-native', () => ({
ChevronsUpDown: 'Icon',
CircleAlert: 'Icon',
FileText: 'Icon',
ListChecks: 'Icon',
@@ -24,6 +25,8 @@ vi.mock('lucide-react-native', () => ({
WifiOff: 'Icon'
}))
vi.mock('../components/MobileDiffReviewLine', () => ({ MobileDiffReviewLine: () => null }))
vi.mock('react-native-gesture-handler', () => ({ Gesture: {}, GestureDetector: () => null }))
vi.mock('../components/pr-sidebar/PRCommentsSection', () => ({ CommentGroupView: () => null }))
vi.mock('expo-haptics', () => ({
impactAsync: vi.fn(),
notificationAsync: vi.fn(),
@@ -79,7 +82,12 @@ function ReviewWhileUnreachable({
filteredCount: controller.filteredQueue.length,
screenState: controller.screenState,
staleCommentIds: controller.staleCommentIds,
onAddNote: controller.openComposer,
threadsByLine: new Map(),
threadActions: undefined,
selectedRange: null,
onPickLines: () => {},
scrollToLine: null,
onScrolledToLine: () => {},
onEditNote: controller.openEditComposer,
onRetry: controller.retryAction ?? undefined,
offline: connState !== 'connected',
@@ -12,7 +12,11 @@ function lines(pattern: string): MobileDiffLine[] {
function shape(pattern: string, opened: number[] = []): string[] {
return buildMobileDiffReviewRows(lines(pattern), new Set(opened)).map((row) =>
row.kind === 'fold' ? `fold ${row.hiddenCount}` : row.line.kind.charAt(0)
row.kind === 'fold'
? `fold ${row.hiddenCount}`
: row.kind === 'line'
? row.line.kind.charAt(0)
: 'thread'
)
}
@@ -60,4 +64,38 @@ describe('buildMobileDiffReviewRows', () => {
'fold 7'
])
})
it('puts a thread under its line and keeps that line out of folds', () => {
const thread = {
kind: 'thread' as const,
threadId: 'T1',
root: {
id: 1,
author: 'nwparker',
authorAvatarUrl: '',
body: 'Use radii.surface here.',
createdAt: '2026-11-01T00:00:00Z',
url: 'https://github.com/o/r/pull/1'
},
replies: []
}
const rows = buildMobileDiffReviewRows(
lines('.'.repeat(20)),
new Set(),
new Set(),
new Map([[10, [thread]]])
)
expect(rows.map((row) => (row.kind === 'line' ? row.line.newLineNumber : row.kind))).toEqual([
'fold',
7,
8,
9,
10,
'thread',
11,
12,
13,
'fold'
])
})
})
+18 -4
View File
@@ -1,4 +1,5 @@
import type { MobileDiffLine } from './mobile-diff-lines'
import type { MobileDiffReviewThread } from './mobile-diff-review-github'
// Monaco's hideUnchangedRegions defaults, which desktop's diff uses: 3 lines of context each side, fold 3 or more.
const CONTEXT_LINES = 3
@@ -7,6 +8,7 @@ const MINIMUM_FOLDED_LINES = 3
export type MobileDiffReviewRow<Line extends MobileDiffLine = MobileDiffLine> =
| { kind: 'line'; key: string; line: Line; lineIndex: number }
| { kind: 'fold'; key: string; hiddenCount: number; startIndex: number; endIndex: number }
| { kind: 'thread'; key: string; thread: MobileDiffReviewThread }
/**
* Unchanged runs fold into one row each. Lines the reader opened stay open even when a later
@@ -15,7 +17,9 @@ export type MobileDiffReviewRow<Line extends MobileDiffLine = MobileDiffLine> =
export function buildMobileDiffReviewRows<Line extends MobileDiffLine>(
lines: readonly Line[],
openedLineIndexes: ReadonlySet<number>,
annotatedLineNumbers: ReadonlySet<number> = new Set()
annotatedLineNumbers: ReadonlySet<number> = new Set(),
/** GitHub threads by the new-side line they end on; each renders under that line. */
threadsByLine: ReadonlyMap<number, readonly MobileDiffReviewThread[]> = new Map()
): MobileDiffReviewRow<Line>[] {
// A note on an unchanged line anchors it like a change, so the note is never hidden in a fold.
const foldable = (lineIndex: number) => {
@@ -23,14 +27,24 @@ export function buildMobileDiffReviewRows<Line extends MobileDiffLine>(
return (
line?.kind === 'context' &&
!openedLineIndexes.has(lineIndex) &&
(line.newLineNumber === undefined || !annotatedLineNumbers.has(line.newLineNumber))
(line.newLineNumber === undefined ||
(!annotatedLineNumbers.has(line.newLineNumber) && !threadsByLine.has(line.newLineNumber)))
)
}
const rows: MobileDiffReviewRow<Line>[] = []
const pushLine = (lineIndex: number) => {
const line = lines[lineIndex]
if (line) {
rows.push({ kind: 'line', key: `line:${lineIndex}`, line, lineIndex })
if (!line) {
return
}
rows.push({ kind: 'line', key: `line:${lineIndex}`, line, lineIndex })
// Threads sit on the new side, so a deleted line never carries one.
const threads =
line.kind !== 'delete' && line.newLineNumber !== undefined
? threadsByLine.get(line.newLineNumber)
: undefined
for (const thread of threads ?? []) {
rows.push({ kind: 'thread', key: `thread:${thread.threadId}`, thread })
}
}
let index = 0
@@ -2,11 +2,14 @@ import type { MobileDiffReviewQueueItem } from './mobile-diff-review-queue'
import type { ComposerState, SendSheetState } from './mobile-diff-review-screen-model'
export type ReviewSheet =
| { kind: 'send'; load: SendSheetState }
/** `only` limits the sheet to these notes, e.g. the one just written. */
| { kind: 'send'; load: SendSheetState; only?: readonly string[] }
| { kind: 'discard'; target: MobileDiffReviewQueueItem }
| { kind: 'composer'; composer: ComposerState }
| { kind: 'completion' }
| { kind: 'files' }
/** Lines picked in the diff, inclusive, waiting for a note to send or post. */
| { kind: 'range'; startLine: number; line: number }
export type ReviewSheetKind = ReviewSheet['kind']
@@ -19,7 +22,8 @@ export type ReviewSheetsState = {
export type ReviewSheetsAction =
| { type: 'open'; sheet: ReviewSheet }
| { type: 'openWhenIdle'; sheet: ReviewSheet }
| { type: 'close'; kind: ReviewSheetKind }
/** With `sheet`, closes only that very presentation, so a slow completion can't close a newer one. */
| { type: 'close'; kind: ReviewSheetKind; sheet?: ReviewSheet }
| { type: 'updateSend'; load: SendSheetState }
export const NO_REVIEW_SHEETS: ReviewSheetsState = { requested: null, deferred: null }
@@ -46,6 +50,9 @@ export function reduceReviewSheets(
}
return { requested, deferred: action.sheet }
case 'close':
if (action.sheet && requested !== action.sheet && deferred !== action.sheet) {
return state
}
if (requested?.kind === action.kind) {
return { requested: deferred, deferred: null }
}
@@ -58,7 +65,7 @@ export function reduceReviewSheets(
if (requested?.kind !== 'send') {
return state
}
return { requested: { kind: 'send', load: action.load }, deferred }
return { requested: { ...requested, load: action.load }, deferred }
}
}
@@ -71,7 +78,8 @@ export function reviewSheetIntents(dispatch: (action: ReviewSheetsAction) => voi
openSheet: (sheet: ReviewSheet) => dispatch({ type: 'open', sheet }),
/** For async openers: waits for the user's sheet to close instead of closing it. */
openSheetWhenIdle: (sheet: ReviewSheet) => dispatch({ type: 'openWhenIdle', sheet }),
closeSheet: (kind: ReviewSheetKind) => dispatch({ type: 'close', kind }),
closeSheet: (kind: ReviewSheetKind, sheet?: ReviewSheet) =>
dispatch({ type: 'close', kind, ...(sheet ? { sheet } : {}) }),
updateSendSheet: (load: SendSheetState) => dispatch({ type: 'updateSend', load })
}
}
@@ -1,5 +1,11 @@
import { z } from 'zod'
import { salvagedOptional, salvagingArray } from '../../../src/shared/zod-salvage'
import { AGENT_STATUS_STATES } from '../../../src/shared/agent-status-types'
import { isTuiAgent } from '../../../src/shared/tui-agent-config'
import {
isMobileTerminalTabRunningAgent,
resolveMobileTerminalTabOwnedAgentId
} from './mobile-terminal-tab-agent'
// Require addressable terminal rows; salvage/drop other tab kinds rather than inventing send targets.
const reviewTerminalTabSchema = z
@@ -7,9 +13,36 @@ const reviewTerminalTabSchema = z
type: z.literal('terminal'),
id: z.string().min(1),
terminal: z.string().min(1),
title: salvagedOptional('title', z.string())
title: salvagedOptional('title', z.string()),
// Which agent runs in it, so a note can go straight to the only one; older hosts send neither.
launchAgent: salvagedOptional('launchAgent', z.string()),
agentStatus: salvagedOptional(
'agentStatus',
z
.looseObject({
agentType: z.string().optional().catch(undefined),
state: z.enum(AGENT_STATUS_STATES).optional().catch(undefined)
})
.nullable()
)
})
.transform((tab) => {
// The session strip's rule: an agent that finished and left a shell behind no longer counts.
const identity = {
title: tab.title ?? '',
agentStatus: tab.agentStatus ?? null,
launchAgent: tab.launchAgent && isTuiAgent(tab.launchAgent) ? tab.launchAgent : null
}
const agent = isMobileTerminalTabRunningAgent(identity)
? resolveMobileTerminalTabOwnedAgentId(identity)
: null
return {
id: tab.id,
terminal: tab.terminal,
title: tab.title ?? 'Terminal',
...(agent ? { agent } : {})
}
})
.transform((tab) => ({ id: tab.id, terminal: tab.terminal, title: tab.title ?? 'Terminal' }))
// Require an inventory array to distinguish malformed replies from no sessions; salvage valid rows independently.
export const reviewTerminalTabsSchema = z
@@ -121,6 +121,25 @@ describe('required members', () => {
expect(refuses(reviewTerminalTabsSchema, { tabs: 'none' })).toBe(true)
})
it('keeps which agent a terminal runs, so a note can go straight to the only one', () => {
const tabs = reads(reviewTerminalTabsSchema, {
tabs: [
{ id: 'tab-1', type: 'terminal', terminal: 't-1', launchAgent: 'claude' },
{ id: 'tab-2', type: 'terminal', terminal: 't-2', agentStatus: { agentType: 'codex' } },
{ id: 'tab-3', type: 'terminal', terminal: 't-3', agentStatus: null },
{
id: 'tab-4',
type: 'terminal',
terminal: 't-4',
title: 'zsh',
launchAgent: 'claude',
agentStatus: { agentType: 'claude', state: 'done' }
}
]
})
expect(tabs?.map((tab) => tab.agent)).toEqual(['claude', 'codex', undefined, undefined])
})
it('requires the three compare members every summary line reads', () => {
const summary = { baseRef: 'origin/main', compareRef: 'feature', changedFiles: 2 }
expect(reads(branchCompareProjectionSchema, { summary, entries: [] }).summary.baseRef).toBe(
@@ -363,6 +382,40 @@ describe('closed arm sets drop rather than default', () => {
})
expect(details?.comments[0]?.reactions).toEqual([{ content: '+1', count: 2 }])
})
it('keeps the PR files the diff screen reads viewed state and commentable lines from', () => {
const details = reads(githubWorkItemDetailsSchema, {
item: { id: 'i-1', number: 7, type: 'pr', state: 'open' },
files: [
{
path: 'src/a.ts',
status: 'modified',
additions: 2,
deletions: 1,
isBinary: false,
reviewCommentLineNumbers: [3, 4],
viewerViewedState: 'DISMISSED'
},
{ path: 'src/b.ts', status: 'moved', viewerViewedState: 'SOMETIMES' },
{ status: 'added' }
]
})
expect(details?.files).toEqual([
{
path: 'src/a.ts',
status: 'modified',
additions: 2,
deletions: 1,
isBinary: false,
reviewCommentLineNumbers: [3, 4],
viewerViewedState: 'DISMISSED'
},
{ path: 'src/b.ts', status: 'modified', additions: 0, deletions: 0, isBinary: false }
])
expect(
reads(githubWorkItemDetailsSchema, { item: { id: 'i-1', number: 7, type: 'pr' } })?.files
).toBeUndefined()
})
})
// Preserve null as explicit no; undefined means the host omitted the flag.
@@ -40,6 +40,8 @@ type CommentActionsInput = {
selectIndex: (index: number) => void
/** The reader's navigation count; a save that outlasts a newer pick leaves that pick alone. */
navigationRef: { readonly current: number }
/** GitHub's viewed mark for a PR file, or null when the file's viewed state lives on the phone. */
remoteViewed: (item: MobileDiffReviewQueueItem, viewed: boolean) => Promise<boolean> | null
setComposerBody: Dispatch<SetStateAction<string>>
setActionError: Dispatch<SetStateAction<string | null>>
sheets: Pick<ReviewSheetIntents, 'openSheet' | 'openSheetWhenIdle' | 'closeSheet'>
@@ -60,6 +62,7 @@ export function useMobileDiffReviewCommentActions(input: CommentActionsInput) {
setScreenState,
selectIndex,
navigationRef,
remoteViewed,
setComposerBody,
setActionError,
sheets
@@ -219,24 +222,47 @@ export function useMobileDiffReviewCommentActions(input: CommentActionsInput) {
closeComposer()
}, [closeComposer, composer, saveCommentsAndReviewState, screenState])
// Resolves whether the file is now viewed: on GitHub for a PR file, otherwise in the phone's review state.
const setItemViewed = useCallback(
async (item: MobileDiffReviewQueueItem, viewed: boolean): Promise<boolean> => {
const remote = remoteViewed(item, viewed)
if (remote) {
return remote
}
const descriptor = reviewDescriptorFromItem(item)
if (!descriptor || screenState.kind !== 'ready') {
return false
}
const now = Date.now()
let nextReviewState = viewed
? markMobileDiffReviewFileReviewed(screenState.reviewState, descriptor, now)
: {
...clearMobileDiffReviewFileReviewed(screenState.reviewState, item.key, now),
completedAt: undefined
}
if (
viewed &&
queue.every((candidate) => candidate.key === item.key || candidate.isReviewed)
) {
nextReviewState = completeMobileDiffReviewState(nextReviewState, now)
}
try {
await saveCommentsAndReviewState(screenState.comments, nextReviewState)
return true
} catch {
// The failed save already rolled back and showed its error.
return false
}
},
[queue, remoteViewed, saveCommentsAndReviewState, screenState]
)
const markReviewed = useCallback(async () => {
const descriptor = currentItem ? reviewDescriptorFromItem(currentItem) : null
if (!currentItem || !descriptor || screenState.kind !== 'ready') {
if (!currentItem) {
return
}
const now = Date.now()
let nextReviewState = markMobileDiffReviewFileReviewed(screenState.reviewState, descriptor, now)
if (queue.every((item) => item.key === currentItem.key || item.isReviewed)) {
nextReviewState = completeMobileDiffReviewState(nextReviewState, now)
}
const navigation = navigationRef.current
try {
await saveCommentsAndReviewState(screenState.comments, nextReviewState)
} catch {
// The failed save already rolled back and showed its error.
return
}
if (navigation !== navigationRef.current) {
if (!(await setItemViewed(currentItem, true)) || navigation !== navigationRef.current) {
return
}
const nextIndex = nextReviewIndexAfterMarkReviewed({
@@ -256,29 +282,15 @@ export function useMobileDiffReviewCommentActions(input: CommentActionsInput) {
filteredQueue,
navigationRef,
openSheetWhenIdle,
queue,
saveCommentsAndReviewState,
screenState,
selectIndex
selectIndex,
setItemViewed
])
const toggleItemViewed = useCallback(
async (item: MobileDiffReviewQueueItem) => {
const descriptor = reviewDescriptorFromItem(item)
if (!descriptor || screenState.kind !== 'ready') {
return
}
const now = Date.now()
const nextReviewState = item.isReviewed
? {
...clearMobileDiffReviewFileReviewed(screenState.reviewState, item.key, now),
completedAt: undefined
}
: markMobileDiffReviewFileReviewed(screenState.reviewState, descriptor, now)
// A failed save already rolled back and showed its error.
await saveCommentsAndReviewState(screenState.comments, nextReviewState).catch(() => {})
await setItemViewed(item, !item.isReviewed)
},
[saveCommentsAndReviewState, screenState]
[setItemViewed]
)
return {
@@ -10,7 +10,6 @@ import { getUnsentMobileDiffComments } from './mobile-diff-comment-edit'
import {
buildMobileDiffReviewQueue,
filterMobileDiffReviewQueue,
mobileDiffReviewCommentMatchesItem,
type MobileDiffReviewQueueFilter
} from './mobile-diff-review-queue'
import { useMobileDiffReviewSelection } from './use-mobile-diff-review-selection'
@@ -29,6 +28,11 @@ import {
import { useMobileDiffReviewInteractions } from './use-mobile-diff-review-interactions'
import { resolveMobileAgentLaunchAvailability } from './mobile-agent-launch-availability'
import { useMobilePrSidebarController } from './use-mobile-pr-sidebar-controller'
import { useMobileDiffReviewGitHub } from './use-mobile-diff-review-github'
import { mobileDiffReviewThreadsForItem } from './mobile-diff-review-github'
import { mobileDiffReviewNotesForItem } from './mobile-diff-comments'
const NO_COMMENTS: readonly DiffComment[] = []
type ControllerInput = {
client: RpcClient | null
@@ -43,6 +47,8 @@ type ControllerInput = {
initialTarget: MobileDiffReviewInitialTarget | null
/** Shows this commit's files instead of the worktree's changes. */
commitOid: string | null
/** A line of the tapped file to scroll to once, e.g. from a review thread's "Open in diff". */
initialLine?: number | null
onOpenSession: () => void
/** Null on the page, where the shell owns the connection. */
onReconnect: ((hostId: string) => void | Promise<void>) | null
@@ -62,6 +68,7 @@ export function useMobileDiffReviewController(input: ControllerInput) {
initialFilter,
initialTarget,
commitOid,
initialLine = null,
onOpenSession,
onReconnect
} = input
@@ -70,6 +77,8 @@ export function useMobileDiffReviewController(input: ControllerInput) {
// Bumped by every snapshot load (open, retry, after a git action), so the open file's diff reloads with it.
const [snapshotVersion, setSnapshotVersion] = useState(0)
const filter = initialFilter
const [pendingLine, setPendingLine] = useState<number | null>(initialLine)
const clearScrollToLine = useCallback(() => setPendingLine(null), [])
const [sheets, dispatchSheets] = useReducer(reduceReviewSheets, NO_REVIEW_SHEETS)
const sheetIntents = useMemo(() => reviewSheetIntents(dispatchSheets), [])
const composer = reviewComposer(sheets)
@@ -147,6 +156,30 @@ export function useMobileDiffReviewController(input: ControllerInput) {
void loadReviewData()
}, [loadReviewData])
// Use git.status for PR head identity; branchCompare’s base ref is not the head.
const prSidebarBranch = screenState.kind === 'ready' ? (screenState.status.branch ?? null) : null
const prSidebarHeadSha =
screenState.kind === 'ready'
? (screenState.status.head ?? screenState.branchCompare?.summary.headOid ?? null)
: null
const prSidebar = useMobilePrSidebarController({
client,
connState,
worktreeId,
branch: prSidebarBranch,
headSha: prSidebarHeadSha
})
const { github, applyViewed, setRemoteViewed, commentOnGitHub } = useMobileDiffReviewGitHub({
client,
connState,
worktreeId,
filter,
screenState,
prSidebar,
setActionError
})
const queue = useMemo(() => {
if (screenState.kind !== 'ready') {
return []
@@ -155,7 +188,7 @@ export function useMobileDiffReviewController(input: ControllerInput) {
screenState.branchCompare && canOpenMobileBranchCompareDiff(screenState.branchCompare.summary)
? (screenState.branchCompare.entries ?? [])
: []
return buildMobileDiffReviewQueue({
const built = buildMobileDiffReviewQueue({
worktreeId,
statusEntries: screenState.status.entries,
branchEntries,
@@ -165,7 +198,8 @@ export function useMobileDiffReviewController(input: ControllerInput) {
comments: screenState.comments,
reviewState: screenState.reviewState
})
}, [screenState, worktreeId])
return applyViewed(built)
}, [applyViewed, screenState, worktreeId])
const filteredQueue = useMemo(() => filterMobileDiffReviewQueue(queue, filter), [filter, queue])
const { currentIndex, currentItem, selectIndex, selectKey, moveSelection } =
@@ -186,54 +220,12 @@ export function useMobileDiffReviewController(input: ControllerInput) {
snapshotVersion
})
const commentsForCurrentItem = useMemo(() => {
if (!currentItem || screenState.kind !== 'ready') {
return []
}
return screenState.comments.filter((comment) =>
mobileDiffReviewCommentMatchesItem(comment, currentItem)
)
}, [currentItem, screenState])
const staleCommentIds = useMemo(
() =>
new Set(
commentsForCurrentItem
.filter(
(comment) =>
currentItem &&
comment.diffIdentity !== undefined &&
comment.diffIdentity !== currentItem.diffIdentity
)
.map((comment) => comment.id)
),
[commentsForCurrentItem, currentItem]
const comments = screenState.kind === 'ready' ? screenState.comments : NO_COMMENTS
const { commentsByLine, staleCommentIds } = useMemo(
() => mobileDiffReviewNotesForItem(comments, currentItem),
[comments, currentItem]
)
const commentsByLine = useMemo(() => {
const map = new Map<number, DiffComment[]>()
for (const comment of commentsForCurrentItem) {
const list = map.get(comment.lineNumber) ?? []
list.push(comment)
map.set(comment.lineNumber, list)
}
return map
}, [commentsForCurrentItem])
// Use git.status for PR head identity; branchCompare’s base ref is not the head.
const prSidebarBranch = screenState.kind === 'ready' ? (screenState.status.branch ?? null) : null
const prSidebarHeadSha =
screenState.kind === 'ready'
? (screenState.status.head ?? screenState.branchCompare?.summary.headOid ?? null)
: null
const prSidebar = useMobilePrSidebarController({
client,
connState,
worktreeId,
branch: prSidebarBranch,
headSha: prSidebarHeadSha
})
const interactions = useMobileDiffReviewInteractions({
client,
connState,
@@ -251,6 +243,7 @@ export function useMobileDiffReviewController(input: ControllerInput) {
selectIndex,
selectKey,
moveSelection,
remoteViewed: setRemoteViewed,
setComposerBody,
setActionError,
setBusyAction,
@@ -276,6 +269,18 @@ export function useMobileDiffReviewController(input: ControllerInput) {
worktreeId,
prSidebarBranch,
prSidebarHeadSha,
github,
// Threads only line up when the local branch is at the PR head; otherwise the file says so.
...mobileDiffReviewThreadsForItem(github, currentItem),
commentOnGitHub,
// Only the tapped file scrolls to the line, and only until it has.
scrollToLine:
pendingLine !== null &&
initialTarget !== null &&
currentItem?.filePath === initialTarget.filePath
? pendingLine
: null,
clearScrollToLine,
actionError,
busyAction,
commentsByLine,
@@ -0,0 +1,193 @@
import { useCallback, useEffect, useMemo, useState } from 'react'
import { translate } from '../i18n/mobile-i18n'
import type { ConnectionState } from '../transport/types'
import type { RpcClient } from '../transport/rpc-client'
import { triggerError, triggerSuccess } from '../platform/haptics'
import { fetchAddPRReviewComment, fetchSetPRFileViewed } from './github-pr-review-line-mutations'
import {
applyMobileDiffReviewGitHubViewed,
buildMobileDiffReviewGitHub,
canCommentOnGitHub,
gitHubViewedFile,
type MobileDiffReviewGitHub
} from './mobile-diff-review-github'
import type {
MobileDiffReviewQueueFilter,
MobileDiffReviewQueueItem
} from './mobile-diff-review-queue'
import type { ReviewScreenState } from './mobile-diff-review-screen-model'
import { isPrSidebarDetailsPlaceholder, prSidebarDetailsNeedFetch } from './mobile-pr-sidebar-state'
import type { MobilePrSidebarController } from './use-mobile-pr-sidebar-controller'
type Input = {
client: RpcClient | null
connState: ConnectionState
worktreeId: string
filter: MobileDiffReviewQueueFilter
screenState: ReviewScreenState
prSidebar: MobilePrSidebarController
setActionError: (message: string | null) => void
}
/** The branch's GitHub pull request as the diff screen uses it: viewed state, threads and line comments. */
export function useMobileDiffReviewGitHub(input: Input) {
const { client, connState, worktreeId, filter, screenState, prSidebar, setActionError } = input
const {
prSidebarState,
prSidebarIsGithubRepo,
refetchPRSidebar,
ensurePrSidebarDetails,
patchPrFileViewed
} = prSidebar
const wantsPullRequest = filter === 'branch' && prSidebarIsGithubRepo
// Committed files are the PR's files, so load its details (threads, viewed state) with the screen.
const sidebarKind = prSidebarState.kind
const needsDetails =
prSidebarState.kind === 'ready' && prSidebarDetailsNeedFetch(prSidebarState.data.details)
useEffect(() => {
if (!wantsPullRequest) {
return
}
if (sidebarKind === 'hidden') {
void refetchPRSidebar({ includeDetails: true })
} else if (needsDetails) {
void ensurePrSidebarDetails()
}
}, [ensurePrSidebarDetails, needsDetails, refetchPRSidebar, sidebarKind, wantsPullRequest])
const localHeadOid =
screenState.kind === 'ready'
? (screenState.branchCompare?.summary.headOid ?? screenState.status.head ?? null)
: null
const data = prSidebarState.kind === 'ready' ? prSidebarState.data : null
const details =
data?.details && !isPrSidebarDetailsPlaceholder(data.details) ? data.details : null
const github = useMemo<MobileDiffReviewGitHub | null>(
() =>
wantsPullRequest && data && details
? buildMobileDiffReviewGitHub({
prNumber: data.pr.number,
prRepo: data.pr.prRepo ?? null,
details,
localHeadOid
})
: null,
[data, details, localHeadOid, wantsPullRequest]
)
// Like desktop, a file whose viewed write is still in flight takes no second tick until it settles.
const [pendingViewed, setPendingViewed] = useState<ReadonlySet<string>>(() => new Set())
const applyViewed = useCallback(
(queue: readonly MobileDiffReviewQueueItem[]) =>
applyMobileDiffReviewGitHubViewed(queue, github),
[github]
)
/**
* Marks a PR file viewed on GitHub. Null when this file's viewed state lives on the phone; false
* while the PR's details are still loading, so a tick is never saved where GitHub will replace it.
*/
const setRemoteViewed = useCallback(
(item: MobileDiffReviewQueueItem, nextViewed: boolean): Promise<boolean> | null => {
// A failed details load leaves a placeholder; only a load still in flight waits.
const detailsLoading =
prSidebarState.kind === 'loading' ||
(prSidebarState.kind === 'ready' && prSidebarState.data.details === null)
if (wantsPullRequest && item.scope === 'branch' && detailsLoading) {
return Promise.resolve(false)
}
const pullRequestId = github?.pullRequestId
const file = gitHubViewedFile(github, item)
if (!github || !pullRequestId || !file) {
return null
}
if (pendingViewed.has(item.filePath)) {
return Promise.resolve(false)
}
if (!client || connState !== 'connected') {
setActionError(
translate('session.useDiffReviewGitHub.waitingForDesktop', 'Waiting for desktop...')
)
return Promise.resolve(false)
}
const path = item.filePath
const previous = file.viewerViewedState
const setPending = (pending: boolean) =>
setPendingViewed((prev) => {
const next = new Set(prev)
if (pending) {
next.add(path)
} else {
next.delete(path)
}
return next
})
setPending(true)
patchPrFileViewed(pullRequestId, path, nextViewed ? 'VIEWED' : 'UNVIEWED')
return fetchSetPRFileViewed(client, worktreeId, {
pullRequestId,
path,
viewed: nextViewed,
prRepo: github.prRepo
})
.then((outcome) => {
if (outcome.ok) {
return true
}
if (previous) {
patchPrFileViewed(pullRequestId, path, previous)
}
triggerError()
setActionError(outcome.error)
return false
})
.finally(() => setPending(false))
},
[
client,
connState,
github,
patchPrFileViewed,
pendingViewed,
prSidebarState,
setActionError,
wantsPullRequest,
worktreeId
]
)
const commentOnGitHub = useCallback(
async (args: {
item: MobileDiffReviewQueueItem
startLine: number
line: number
body: string
}): Promise<{ ok: true } | { ok: false; error: string | null }> => {
if (!client || !github?.headSha || !canCommentOnGitHub(github, args.item, args)) {
return { ok: false, error: null }
}
const outcome = await fetchAddPRReviewComment(client, worktreeId, {
prNumber: github.prNumber,
commitId: github.headSha,
path: args.item.filePath,
line: args.line,
startLine: args.startLine,
body: args.body,
prRepo: github.prRepo
})
if (!outcome.ok) {
// The sheet is still open over the screen's banner, so the sheet shows this.
triggerError()
return { ok: false, error: outcome.error }
}
triggerSuccess()
// Show the new thread under its line.
void refetchPRSidebar({ includeDetails: true })
return { ok: true }
},
[client, github, refetchPRSidebar, worktreeId]
)
return { github, applyViewed, setRemoteViewed, commentOnGitHub }
}
@@ -4,12 +4,14 @@ import type { ConnectionState } from '../transport/types'
import type { RpcClient } from '../transport/rpc-client'
import type { MobileDiffReviewQueueItem } from './mobile-diff-review-queue'
import type { ComposerState, ReviewScreenState } from './mobile-diff-review-screen-model'
import type { ReviewSheetIntents } from './mobile-diff-review-sheets'
import type { ReviewSheet, ReviewSheetIntents } from './mobile-diff-review-sheets'
import type { MobileReviewTerminalTab } from './review-terminal-reply-schema'
import { sourceFileDiffOpenRun } from '../source-control/mobile-source-file-open-operations'
import { refusedRpcMessageOrFallback } from '../transport/rpc-refusal-message'
import { useMobileDiffReviewCommentActions } from './use-mobile-diff-review-comment-actions'
import { useMobileDiffReviewGitActions } from './use-mobile-diff-review-git-actions'
import { useMobileDiffReviewSendActions } from './use-mobile-diff-review-send-actions'
import { useMobileDiffReviewRangeNote } from './use-mobile-diff-review-range-note'
import { connectionRetryAction } from '../transport/connection-retry-action'
type InteractionInput = {
@@ -30,6 +32,7 @@ type InteractionInput = {
/** Keeps a file open by key, through the reload that follows a mutation. */
selectKey: (key: string) => void
moveSelection: (direction: 'next' | 'previous') => void
remoteViewed: (item: MobileDiffReviewQueueItem, viewed: boolean) => Promise<boolean> | null
setComposerBody: Dispatch<SetStateAction<string>>
setActionError: Dispatch<SetStateAction<string | null>>
setBusyAction: Dispatch<SetStateAction<string | null>>
@@ -57,6 +60,7 @@ export function useMobileDiffReviewInteractions(input: InteractionInput) {
selectIndex,
selectKey,
moveSelection,
remoteViewed,
setComposerBody,
setActionError,
setBusyAction,
@@ -92,11 +96,20 @@ export function useMobileDiffReviewInteractions(input: InteractionInput) {
setScreenState,
selectIndex,
navigationRef,
remoteViewed,
setComposerBody,
setActionError,
sheets
})
const saveRangeNote = useMobileDiffReviewRangeNote({
worktreeId,
screenState,
currentItem,
composerBody,
saveCommentsAndReviewState
})
const { runGitMutation } = useMobileDiffReviewGitActions({
client,
connState,
@@ -106,17 +119,24 @@ export function useMobileDiffReviewInteractions(input: InteractionInput) {
loadReviewData
})
const { clearSentNotes, copyNotes, createTerminalAndSend, openSendSheet, sendPromptToTerminal } =
useMobileDiffReviewSendActions({
client,
connState,
hostCapabilities,
worktreeId,
screenState,
setActionError,
sheets,
saveCommentsAndReviewState
})
const {
clearSentNotes,
copyNotes,
createTerminalAndSend,
loadTerminals,
openSendSheet,
sendNoteNow,
sendPromptToTerminal
} = useMobileDiffReviewSendActions({
client,
connState,
hostCapabilities,
worktreeId,
screenState,
setActionError,
sheets,
saveCommentsAndReviewState
})
return {
clearSentNotes,
@@ -172,6 +192,20 @@ export function useMobileDiffReviewInteractions(input: InteractionInput) {
runGitMutation,
saveComposer,
sendPromptToTerminal,
loadTerminals,
// The sheet's own lines and the terminals it already listed; the draft clears when the next sheet opens.
sendRange: async (
sheet: Extract<ReviewSheet, { kind: 'range' }>,
terminals?: readonly MobileReviewTerminalTab[]
) => {
const saved = await saveRangeNote(sheet)
if (!('comment' in saved)) {
return saved.error
}
sheets.closeSheet('range', sheet)
await sendNoteNow(saved.comment, terminals)
return null
},
// Staging drops the file from the uncommitted queue; land on the file after it, wrapping from the last.
stageAndOpenNext: async () => {
if (!currentItem?.canStage) {
@@ -0,0 +1,56 @@
import { useCallback } from 'react'
import type { DiffComment, MobileDiffReviewState } from '../../../src/shared/diff-comment-types'
import { addMobileDiffComment } from './mobile-diff-comments'
import type { MobileDiffReviewQueueItem } from './mobile-diff-review-queue'
import type { ReviewScreenState } from './mobile-diff-review-screen-model'
type Input = {
worktreeId: string
screenState: ReviewScreenState
currentItem: MobileDiffReviewQueueItem | null
composerBody: string
saveCommentsAndReviewState: (
comments: DiffComment[],
reviewState: MobileDiffReviewState
) => Promise<void>
}
/** Saves the range sheet's text as a note on those lines, for the agent; null when there is nothing to save. */
export function useMobileDiffReviewRangeNote(input: Input) {
const { worktreeId, screenState, currentItem, composerBody, saveCommentsAndReviewState } = input
return useCallback(
async (range: {
startLine: number
line: number
}): Promise<{ comment: DiffComment } | { error: string | null }> => {
const body = composerBody.trim()
if (!body || !currentItem || screenState.kind !== 'ready' || currentItem.scope === 'commit') {
return { error: null }
}
const now = Date.now()
const result = addMobileDiffComment(screenState.comments, {
id: `mobile-${now}-${Math.random().toString(36).slice(2)}`,
worktreeId,
filePath: currentItem.filePath,
oldPath: currentItem.oldPath,
...(range.startLine < range.line ? { startLine: range.startLine } : {}),
lineNumber: range.line,
body,
createdAt: now,
scope: currentItem.scope,
diffIdentity: currentItem.diffIdentity
})
if (!result.comment) {
return { error: null }
}
try {
await saveCommentsAndReviewState(result.comments, screenState.reviewState)
} catch (err) {
// The screen's banner sits under the open sheet, so the sheet shows why nothing was sent.
return { error: err instanceof Error ? err.message : null }
}
return { comment: result.comment }
},
[composerBody, currentItem, saveCommentsAndReviewState, screenState, worktreeId]
)
}
@@ -117,6 +117,7 @@ describe('useMobileDiffReviewSendActions', () => {
let setActionError: Mock<SendActionArgs['setActionError']>
let sheets: {
openSheet: Mock<ReviewSheetIntents['openSheet']>
openSheetWhenIdle: Mock<ReviewSheetIntents['openSheetWhenIdle']>
closeSheet: Mock<ReviewSheetIntents['closeSheet']>
updateSendSheet: Mock<ReviewSheetIntents['updateSendSheet']>
}
@@ -128,7 +129,12 @@ describe('useMobileDiffReviewSendActions', () => {
clipboardMock.setStringAsync.mockReset().mockResolvedValue(true)
resetMobileNativeChatStaleInputForTests()
setActionError = vi.fn()
sheets = { openSheet: vi.fn(), closeSheet: vi.fn(), updateSendSheet: vi.fn() }
sheets = {
openSheet: vi.fn(),
openSheetWhenIdle: vi.fn(),
closeSheet: vi.fn(),
updateSendSheet: vi.fn()
}
saveCommentsAndReviewState = vi.fn().mockResolvedValue(undefined)
})
@@ -8,12 +8,15 @@ import { triggerSuccess } from '../platform/haptics'
import { formatDiffComments } from '../../../src/shared/diff-comments-format'
import { clearSentMobileDiffComments, markMobileDiffCommentsSent } from './mobile-diff-comment-edit'
import { reviewTerminalListRead, reviewTerminalSendRun } from './mobile-review-terminal-operations'
import type { MobileReviewTerminalTab } from './review-terminal-reply-schema'
import { launchAgentWithPrompt } from './pr-ai-triage-launch'
import { interpretOrThrowRefusalMessage } from '../transport/rpc-refusal-message'
import { healMobileNativeChatStaleInput } from './native-chat/mobile-native-chat-stale-input'
import type { ReviewScreenState } from './mobile-diff-review-screen-model'
import type { ReviewSheetIntents } from './mobile-diff-review-sheets'
const NO_NOTES: readonly DiffComment[] = []
type SendActionsInput = {
client: RpcClient | null
connState: ConnectionState
@@ -21,7 +24,10 @@ type SendActionsInput = {
worktreeId: string
screenState: ReviewScreenState
setActionError: Dispatch<SetStateAction<string | null>>
sheets: Pick<ReviewSheetIntents, 'openSheet' | 'closeSheet' | 'updateSendSheet'>
sheets: Pick<
ReviewSheetIntents,
'openSheet' | 'openSheetWhenIdle' | 'closeSheet' | 'updateSendSheet'
>
saveCommentsAndReviewState: (
comments: DiffComment[],
reviewState: MobileDiffReviewState
@@ -41,29 +47,36 @@ export function useMobileDiffReviewSendActions(input: SendActionsInput) {
sheets,
saveCommentsAndReviewState
} = input
const { openSheet, closeSheet, updateSendSheet } = sheets
const { openSheet, openSheetWhenIdle, closeSheet, updateSendSheet } = sheets
const copyNotes = useCallback(async () => {
if (screenState.kind !== 'ready' || screenState.comments.length === 0) {
return
}
// Contain clipboard refusal because copyNotes is fire-and-forget; never leave copied as the last feedback.
try {
await clipboard.writeText(formatDiffComments(screenState.comments))
} catch (err) {
// Copies these notes, or every note when none are named.
const copyNotes = useCallback(
async (only?: readonly DiffComment[]) => {
const comments = only ?? (screenState.kind === 'ready' ? screenState.comments : NO_NOTES)
if (comments.length === 0) {
return
}
// Contain clipboard refusal because copyNotes is fire-and-forget; never leave copied as the last feedback.
try {
await clipboard.writeText(formatDiffComments(comments))
} catch (err) {
setActionError(
err instanceof Error
? err.message
: translate(
'session.useDiffReviewSendActions.copyFailed',
'Unable to copy the review notes'
)
)
return
}
triggerSuccess()
setActionError(
err instanceof Error
? err.message
: translate(
'session.useDiffReviewSendActions.copyFailed',
'Unable to copy the review notes'
)
translate('session.useDiffReviewSendActions.notesCopied', 'Review notes copied')
)
return
}
triggerSuccess()
setActionError(translate('session.useDiffReviewSendActions.notesCopied', 'Review notes copied'))
}, [clipboard, screenState, setActionError])
},
[clipboard, screenState, setActionError]
)
const clearSentNotes = useCallback(async () => {
if (screenState.kind !== 'ready') {
@@ -94,8 +107,9 @@ export function useMobileDiffReviewSendActions(input: SendActionsInput) {
await save(next, current.reviewState)
}, [])
// From the picker, a send closes it; a direct send has no picker and must not close one opened meanwhile.
const sendPromptToTerminal = useCallback(
async (terminal: string, comments: readonly DiffComment[]) => {
async (terminal: string, comments: readonly DiffComment[], fromPicker = true) => {
if (!client || connState !== 'connected') {
throw new Error(
translate('session.useDiffReviewSendActions.waitingForDesktop', 'Waiting for desktop...')
@@ -125,7 +139,9 @@ export function useMobileDiffReviewSendActions(input: SendActionsInput) {
await markNotesSent(comments)
triggerSuccess()
setActionError(translate('session.useDiffReviewSendActions.notesSent', 'Review notes sent'))
closeSheet('send')
if (fromPicker) {
closeSheet('send')
}
},
[client, connState, closeSheet, markNotesSent, setActionError]
)
@@ -231,11 +247,72 @@ export function useMobileDiffReviewSendActions(input: SendActionsInput) {
}
}, [client, connState, openSheet, setActionError, updateSendSheet, worktreeId])
const loadTerminals = useCallback(async () => {
if (!client || connState !== 'connected') {
throw new Error(
translate('session.useDiffReviewSendActions.waitingForDesktop', 'Waiting for desktop...')
)
}
const response = await reviewTerminalListRead.request(client, {
worktree: `id:${worktreeId}`
})
return interpretOrThrowRefusalMessage(
() => reviewTerminalListRead.interpret(response),
translate(
'session.useDiffReviewSendActions.loadSessionsFailed',
'Unable to load agent sessions'
)
)
}, [client, connState, worktreeId])
// A note written in the diff goes straight to the workspace's one agent; with none or several, the picker asks.
const sendNoteNow = useCallback(
async (comment: DiffComment, listed?: readonly MobileReviewTerminalTab[]) => {
let terminals
try {
terminals = listed ?? (await loadTerminals())
} catch (err) {
setActionError(
err instanceof Error
? err.message
: translate(
'session.useDiffReviewSendActions.loadSessionsFailed',
'Unable to load agent sessions'
)
)
return
}
const agents = terminals.filter((terminal) => terminal.agent !== undefined)
const [onlyAgent] = agents
if (agents.length === 1 && onlyAgent) {
try {
await sendPromptToTerminal(onlyAgent.terminal, [comment], false)
} catch (err) {
setActionError(
err instanceof Error
? err.message
: translate('session.useDiffReviewSendActions.sendFailed', 'Failed to send notes')
)
}
return
}
// The terminal list can outlast a sheet the user opened meanwhile; wait for it rather than replace it.
openSheetWhenIdle({
kind: 'send',
load: { kind: 'ready', terminals: [...terminals] },
only: [comment.id]
})
},
[loadTerminals, openSheetWhenIdle, sendPromptToTerminal, setActionError]
)
return {
clearSentNotes,
copyNotes,
createTerminalAndSend,
loadTerminals,
openSendSheet,
sendNoteNow,
sendPromptToTerminal
}
}
@@ -1,5 +1,6 @@
import { useCallback, useEffect, useRef, useState } from 'react'
import type { ConnectionState } from '../transport/types'
import type { GitHubPRFileViewedState } from '../../../src/shared/github/pull-request-types'
import type { RpcClient } from '../transport/rpc-client'
import {
fetchGithubRepoSlug,
@@ -279,6 +280,37 @@ export function useMobilePrSidebarController(input: PrSidebarControllerInput) {
}
}, [identity, state, load, ensurePrSidebarDetails])
// Desktop's patchCachedPRFileViewedState: a viewed tick shows at once, and a failed write puts the old state back.
const patchPrFileViewed = useCallback(
(pullRequestId: string, path: string, viewed: GitHubPRFileViewedState) => {
setState((prev) => {
const files = prev.kind === 'ready' ? prev.data.details?.files : undefined
// A write for one PR must not land on another the workspace was relinked to meanwhile.
if (
prev.kind !== 'ready' ||
!prev.data.details ||
!files ||
prev.data.details.pullRequestId !== pullRequestId
) {
return prev
}
return {
kind: 'ready',
data: {
...prev.data,
details: {
...prev.data.details,
files: files.map((file) =>
file.path === path ? { ...file, viewerViewedState: viewed } : file
)
}
}
}
})
},
[]
)
const retry = useCallback(() => {
void load({ includeDetails: true })
}, [load])
@@ -292,7 +324,8 @@ export function useMobilePrSidebarController(input: PrSidebarControllerInput) {
openPRSidebar,
retryPRSidebar: retry,
refetchPRSidebar: load,
ensurePrSidebarDetails
ensurePrSidebarDetails,
patchPrFileViewed
}
}
@@ -7,6 +7,8 @@ export type MobileReviewRouteTarget = {
worktreeId: string
worktreeName: string
filePath: string
/** Scrolls to this new-side line, e.g. a review thread's "Open in diff". */
line?: number
} & ({ area: MobileReviewRouteArea } | { area: 'commit'; commitOid: string })
export function buildMobileReviewFileRoute(target: MobileReviewRouteTarget): string {
@@ -16,6 +18,9 @@ export function buildMobileReviewFileRoute(target: MobileReviewRouteTarget): str
if (target.area === 'commit') {
params.set('commit', target.commitOid)
}
if (target.line !== undefined) {
params.set('line', String(target.line))
}
if (target.worktreeName) {
params.set('name', target.worktreeName)
}
@@ -4,7 +4,7 @@ import { buildMobileReviewFileRoute, type MobileReviewRouteArea } from './mobile
// A changed file in the worktree or on the branch, or a file in one commit (diffed against its parent).
export type MobileDiffFileTarget =
| { kind: 'changes'; filePath: string; area: MobileReviewRouteArea }
| { kind: 'changes'; filePath: string; area: MobileReviewRouteArea; line?: number }
| { kind: 'commit'; filePath: string; commitOid: string }
type Params = {
@@ -26,6 +26,7 @@ export function useMobileSourceControlOpeners({ hostId, worktreeId, name }: Para
worktreeId,
worktreeName: name,
filePath: target.filePath,
...(target.kind === 'changes' && target.line !== undefined ? { line: target.line } : {}),
...(target.kind === 'commit'
? { area: 'commit', commitOid: target.commitOid }
: { area: target.area })
@@ -116,7 +116,7 @@ export const BRIDGED_PARITY_EXCLUSIONS: Readonly<Partial<Record<BridgedParityCla
// Pin strict History verdicts and unchanged legacy counts in both directions.
export const BRIDGED_PARITY_BASELINE: Readonly<Record<BridgedParityClass | 'identical', number>> = {
identical: 386,
identical: 387,
'reply-meta-required': 0,
'result-absent-settlement': 335,
'result-absent-observation': 3,
@@ -102,6 +102,8 @@ export function diffReviewActionMountAdapters(
selectIndex: () => {},
selectKey: () => {},
moveSelection: () => {},
// Viewed state stays on the phone here; GitHub-viewed files have their own recordings.
remoteViewed: () => null,
setComposerBody: () => {},
setActionError: (update) => {
actionError = typeof update === 'function' ? update(actionError) : update
@@ -1,5 +1,6 @@
import type { MountAdapter } from '../recording-scenario'
import type { operationModuleLoader } from '../operation-module-loader'
import type * as LineMutations from '../../../session/github-pr-review-line-mutations'
const WORKTREE = 'repo-9::/w'
const PR_NUMBER = 12
@@ -65,6 +66,8 @@ export function githubPrMountAdapters(
const mutations = modules.load<typeof import('../../../session/github-pr-mutations')>(
'mobile/src/session/github-pr-mutations.ts'
)
const lineMutations = () =>
modules.load<typeof LineMutations>('mobile/src/session/github-pr-review-line-mutations.ts')
const results: Record<string, unknown> = {}
function send(name: string, args: Record<string, unknown>): Promise<unknown> {
const prRepo = args.fork === true ? FORK_REPO : null
@@ -130,6 +133,25 @@ export function githubPrMountAdapters(
prRepo
})
}
if (name === 'line-comment') {
return lineMutations().fetchAddPRReviewComment(client, WORKTREE, {
prNumber: PR_NUMBER,
commitId: 'head-sha-1',
path: 'src/app.ts',
startLine: 3,
line: 5,
body: 'recorded line comment',
prRepo
})
}
if (name === 'file-viewed') {
return lineMutations().fetchSetPRFileViewed(client, WORKTREE, {
pullRequestId: 'PR_kwDO',
path: 'src/app.ts',
viewed: true,
prRepo
})
}
if (name === 'resolve-thread') {
return mutations.fetchResolveReviewThread(client, WORKTREE, {
threadId: 'thread-1',