Files
orca/mobile/src/session/mobile-diff-review-queue.ts
T
Neil 77f23b013f refactor(shared): drop the shared/types barrel and import from the real modules (#14447)
#14397 split `shared/types.ts` into 46 per-domain modules but kept the path as
a re-export barrel so the import sites did not have to change. This removes
the barrel: every consumer now imports from the module that actually declares
the type, and `src/shared/types.ts` is deleted.

Barrels hide where a type lives, make every consumer look like it depends on
the whole domain, and let an unrelated edit invalidate a module that ~2,000
files transitively import.

2,323 import declarations across 2,321 files. Rewritten mechanically: each
specifier was resolved to an absolute path via the TypeScript AST and
recomputed, rather than string-substituted, so alias forms (`@/../../shared/
types`) and per-specifier `type` modifiers survive.

Four cases the mechanical pass had to handle, each found by a gate rather than
by reading the diff:

- Modules inside `src/shared` import the barrel as `./types`, not
  `shared/types`. A pre-filter on the latter string skipped 176 of them and
  left imports dangling at a deleted file, which surfaced as confusing
  `Property 'x' is optional in type 'Repo' but required in Pick<Repo, ...>`
  errors rather than "module not found".
- The barrel RENAMED one type on the way through
  (`WorkspaceSource as WorkspaceCreateTelemetrySource`), so the original name
  in the owning module has to be re-aliased at each consumer.
- Three test files put `;(globalThis as ...)` on the line after the import.
  TypeScript parses that `;` as the import statement's terminator, so
  replacing through `statement.getEnd()` deletes it and breaks ASI. The
  rewrite now stops at the module specifier.
- A file that already imported directly from a module got a SECOND import
  from it, because the barrel re-exported those same names — which trips
  `import/no-duplicates` under `--deny-warnings`. A post-pass merges
  declarations sharing a specifier and type-only-ness; the `import type` plus
  `import` pair from one module is left alone, since that form is allowed.

Splitting one barrel import into several genuinely adds lines, which pushed
`terminal-layout-pty-ownership.ts` to 301 counted lines: its 107-character
import must wrap, and neither local type collapses onto one line (101 and 116
characters). Rather than contort a type declaration to fit a line budget,
`collectLeafIds` and `pruneLeaves` move to `terminal-pane-layout-tree.ts` —
they are pure structural operations on the layout tree and independent of PTY
ownership. `visible-worktrees.ts` similarly loses its own mini-barrel
re-export of `isDefaultBranchWorkspace`, with the four real consumers
repointed at the declaring module. No `max-lines` bypass added.

Verified: cold `tsc --noEmit` green on node, cli, and web (buildinfo deleted
first — these projects are `composite: true` and reuse stale caches); the full
`pnpm lint` green, not just bare oxlint — the narrower local check is what let
the duplicate imports reach CI; max-lines ratchet OK at 344.
2026-08-13 22:48:24 -07:00

275 lines
7.9 KiB
TypeScript

import type {
DiffComment,
DiffReviewScope,
MobileDiffReviewState
} from '../../../src/shared/diff-comment-types'
import type { MobileGitBranchChangeEntry } from '../source-control/mobile-branch-compare'
import {
isMobileGitDiscardableEntry,
isMobileGitStageableEntry,
type MobileGitFileStatus,
type MobileGitStagingArea,
type MobileGitStatusEntry
} from '../source-control/mobile-git-status'
import {
buildMobileDiffIdentity,
didMobileDiffReviewFileChangeSinceReview,
isMobileDiffReviewFileReviewed
} from './mobile-diff-review-state'
export type MobileDiffReviewQueueFilter =
| 'all'
| 'unreviewed'
| 'notes'
| 'unstaged'
| 'staged'
| 'branch'
export type MobileDiffReviewQueueItem = {
key: string
scope: DiffReviewScope
area: MobileGitStagingArea | 'branch'
filePath: string
oldPath?: string
status: MobileGitFileStatus
title: string
subtitle: string
added?: number
removed?: number
canStage: boolean
canUnstage: boolean
canDiscard: boolean
isGeneratedOrLockFile: boolean
diffIdentity: string
noteCount: number
unsentNoteCount: number
staleNoteCount: number
reviewedAt?: number
isReviewed: boolean
changedSinceReview: boolean
}
export type BuildMobileDiffReviewQueueInput = {
worktreeId: string
statusEntries: readonly MobileGitStatusEntry[]
branchEntries: readonly MobileGitBranchChangeEntry[]
branchHeadOid?: string | null
branchMergeBase?: string | null
comments: readonly DiffComment[]
reviewState: MobileDiffReviewState
}
const SCOPE_SORT_ORDER: Record<DiffReviewScope, number> = {
unstaged: 0,
staged: 1,
branch: 2
}
function scopeForStatusArea(area: MobileGitStagingArea): DiffReviewScope {
return area === 'staged' ? 'staged' : 'unstaged'
}
export function createMobileDiffReviewFileKey(
scope: DiffReviewScope,
area: MobileGitStagingArea | 'branch',
filePath: string,
oldPath?: string
): string {
return [scope, area, oldPath ?? '', filePath].join('\0')
}
function statusEntryIdentity(entry: MobileGitStatusEntry, scope: DiffReviewScope): string {
return buildMobileDiffIdentity([
scope,
entry.area,
entry.status,
entry.oldPath ?? '',
entry.path,
String(entry.added ?? ''),
String(entry.removed ?? ''),
entry.conflictStatus ?? ''
])
}
function branchEntryIdentity(
entry: MobileGitBranchChangeEntry,
branchHeadOid: string | null | undefined,
branchMergeBase: string | null | undefined
): string {
return buildMobileDiffIdentity([
'branch',
branchMergeBase ?? '',
branchHeadOid ?? '',
entry.status,
entry.oldPath ?? '',
entry.path,
String(entry.added ?? ''),
String(entry.removed ?? '')
])
}
function isGeneratedOrLockFile(filePath: string): boolean {
const normalized = filePath.toLowerCase()
return (
normalized.endsWith('package-lock.json') ||
normalized.endsWith('pnpm-lock.yaml') ||
normalized.endsWith('yarn.lock') ||
normalized.endsWith('bun.lockb') ||
normalized.endsWith('.lock') ||
normalized.includes('/dist/') ||
normalized.includes('/build/') ||
normalized.includes('/coverage/') ||
normalized.endsWith('.generated.ts') ||
normalized.endsWith('.generated.tsx')
)
}
export function mobileDiffReviewCommentMatchesItem(
comment: DiffComment,
item: Pick<MobileDiffReviewQueueItem, 'filePath' | 'oldPath' | 'scope' | 'diffIdentity'>
): boolean {
if (comment.source === 'markdown' || comment.filePath !== item.filePath) {
return false
}
if (comment.scope !== undefined && comment.scope !== item.scope) {
return false
}
if (comment.oldPath !== undefined && comment.oldPath !== item.oldPath) {
return false
}
return true
}
function queueNoteCounts(
item: Pick<MobileDiffReviewQueueItem, 'filePath' | 'oldPath' | 'scope' | 'diffIdentity'>,
comments: readonly DiffComment[]
): { noteCount: number; unsentNoteCount: number; staleNoteCount: number } {
let noteCount = 0
let unsentNoteCount = 0
let staleNoteCount = 0
for (const comment of comments) {
if (!mobileDiffReviewCommentMatchesItem(comment, item)) {
continue
}
noteCount += 1
if (comment.sentAt === undefined) {
unsentNoteCount += 1
}
if (comment.diffIdentity !== undefined && comment.diffIdentity !== item.diffIdentity) {
staleNoteCount += 1
}
}
return { noteCount, unsentNoteCount, staleNoteCount }
}
function statusEntryToQueueItem(
entry: MobileGitStatusEntry,
comments: readonly DiffComment[],
reviewState: MobileDiffReviewState
): MobileDiffReviewQueueItem {
const scope = scopeForStatusArea(entry.area)
const key = createMobileDiffReviewFileKey(scope, entry.area, entry.path, entry.oldPath)
const diffIdentity = statusEntryIdentity(entry, scope)
const reviewFileState = reviewState.files[key]
const counts = queueNoteCounts(
{ filePath: entry.path, oldPath: entry.oldPath, scope, diffIdentity },
comments
)
return {
key,
scope,
area: entry.area,
filePath: entry.path,
oldPath: entry.oldPath,
status: entry.status,
title: entry.path,
subtitle: scope === 'staged' ? 'Staged' : 'Unstaged',
added: entry.added,
removed: entry.removed,
canStage: isMobileGitStageableEntry(entry),
canUnstage: entry.area === 'staged',
canDiscard: isMobileGitDiscardableEntry(entry) && entry.area !== 'staged',
isGeneratedOrLockFile: isGeneratedOrLockFile(entry.path),
diffIdentity,
...counts,
reviewedAt: reviewFileState?.reviewedAt,
isReviewed: isMobileDiffReviewFileReviewed(reviewFileState, diffIdentity),
changedSinceReview: didMobileDiffReviewFileChangeSinceReview(reviewFileState, diffIdentity)
}
}
function branchEntryToQueueItem(
entry: MobileGitBranchChangeEntry,
input: BuildMobileDiffReviewQueueInput
): MobileDiffReviewQueueItem {
const scope: DiffReviewScope = 'branch'
const key = createMobileDiffReviewFileKey(scope, 'branch', entry.path, entry.oldPath)
const diffIdentity = branchEntryIdentity(entry, input.branchHeadOid, input.branchMergeBase)
const reviewFileState = input.reviewState.files[key]
const counts = queueNoteCounts(
{ filePath: entry.path, oldPath: entry.oldPath, scope, diffIdentity },
input.comments
)
return {
key,
scope,
area: 'branch',
filePath: entry.path,
oldPath: entry.oldPath,
status: entry.status,
title: entry.path,
subtitle: 'Committed on branch',
added: entry.added,
removed: entry.removed,
canStage: false,
canUnstage: false,
canDiscard: false,
isGeneratedOrLockFile: isGeneratedOrLockFile(entry.path),
diffIdentity,
...counts,
reviewedAt: reviewFileState?.reviewedAt,
isReviewed: isMobileDiffReviewFileReviewed(reviewFileState, diffIdentity),
changedSinceReview: didMobileDiffReviewFileChangeSinceReview(reviewFileState, diffIdentity)
}
}
function compareQueueItems(
first: MobileDiffReviewQueueItem,
second: MobileDiffReviewQueueItem
): number {
return (
SCOPE_SORT_ORDER[first.scope] - SCOPE_SORT_ORDER[second.scope] ||
Number(first.isGeneratedOrLockFile) - Number(second.isGeneratedOrLockFile) ||
first.filePath.localeCompare(second.filePath, undefined, { numeric: true })
)
}
export function buildMobileDiffReviewQueue(
input: BuildMobileDiffReviewQueueInput
): MobileDiffReviewQueueItem[] {
return [
...input.statusEntries.map((entry) =>
statusEntryToQueueItem(entry, input.comments, input.reviewState)
),
...input.branchEntries.map((entry) => branchEntryToQueueItem(entry, input))
].sort(compareQueueItems)
}
export function filterMobileDiffReviewQueue(
queue: readonly MobileDiffReviewQueueItem[],
filter: MobileDiffReviewQueueFilter
): MobileDiffReviewQueueItem[] {
switch (filter) {
case 'unreviewed':
return queue.filter((item) => !item.isReviewed)
case 'notes':
return queue.filter((item) => item.noteCount > 0)
case 'unstaged':
case 'staged':
case 'branch':
return queue.filter((item) => item.scope === filter)
case 'all':
return [...queue]
}
}