diff --git a/config/scripts/mobile-task-sort-benchmark.mjs b/config/scripts/mobile-task-sort-benchmark.mjs new file mode 100644 index 00000000000..b7d6cb469a3 --- /dev/null +++ b/config/scripts/mobile-task-sort-benchmark.mjs @@ -0,0 +1,113 @@ +import assert from 'node:assert/strict' +import { execFileSync } from 'node:child_process' +import { readFileSync } from 'node:fs' +import { dirname, resolve } from 'node:path' +import { performance } from 'node:perf_hooks' +import { build } from 'esbuild' +import { buildCounterbalancedSchedule } from './counterbalanced-benchmark-schedule.mjs' + +const baseline = process.argv[2] +if (!baseline) { + throw new Error('Usage: node config/scripts/mobile-task-sort-benchmark.mjs ') +} +async function load(file, contents) { + const result = await build({ + stdin: { contents, loader: 'ts', resolveDir: dirname(resolve(file)) }, + bundle: true, + platform: 'node', + format: 'esm', + write: false, + logLevel: 'silent', + tsconfigRaw: {}, + plugins: [ + { + name: 'theme-only', + setup(bundler) { + bundler.onResolve({ filter: /mobile-tasks-dependencies$/ }, () => ({ + path: resolve('mobile/src/theme/mobile-theme.ts') + })) + } + } + ] + }) + return await import( + `data:text/javascript;base64,${Buffer.from(result.outputFiles[0].text).toString('base64')}` + ) +} +const file = 'mobile/src/tasks/mobile-tasks-repository-presentation.ts' +const before = await load( + file, + execFileSync('git', ['show', `${baseline}:${file}`], { encoding: 'utf8' }) +) +const after = await load(file, readFileSync(file, 'utf8')) +const repos = new Map() +const results = [] +for (const count of [0, 1, 25, 1000]) { + const items = Array.from({ length: count }, (_, index) => ({ + key: `item-${index}`, + provider: 'github', + title: 'task', + subtitle: '', + status: 'open', + updatedAt: new Date(1700000000000 - index * 100000).toISOString(), + source: { repoId: `repo-${index % 25}`, repoName: `Repository ${index % 25}` } + })) + for (const sort of ['updated', 'repository']) { + const arms = { + before: () => + [...items].sort( + sort === 'repository' + ? (a, b) => before.compareTasksByRepository(a, b, repos) + : before.compareTasksByUpdated + ), + after: () => after.sortMobileTaskItems(items, sort, repos) + } + assert.deepEqual(arms.after(), arms.before()) + const iterations = count < 100 ? 100 : 10 + function run(arm) { + const start = performance.now() + for (let i = 0; i < iterations; i++) { + arms[arm]() + } + return (performance.now() - start) / iterations + } + const samples = { before: [], after: [] } + run('before') + run('after') + for (const pair of buildCounterbalancedSchedule(10, 'before', 'after')) { + for (const arm of pair) { + samples[arm].push(run(arm)) + } + } + function median(values) { + const sorted = [...values].sort((a, b) => a - b) + return (sorted[4] + sorted[5]) / 2 + } + const dateParses = {} + const nativeParse = Date.parse + for (const arm of ['before', 'after']) { + let calls = 0 + Date.parse = (value) => { + calls++ + return nativeParse(value) + } + try { + arms[arm]() + } finally { + Date.parse = nativeParse + } + dateParses[arm] = calls + } + results.push({ + count, + sort, + dateParses, + beforeMs: median(samples.before), + afterMs: median(samples.after), + samples + }) + } +} +console.log( + JSON.stringify({ baseline, node: process.version, platform: process.platform, results }, null, 2) +) diff --git a/mobile/src/tasks/mobile-task-sort.test.ts b/mobile/src/tasks/mobile-task-sort.test.ts new file mode 100644 index 00000000000..dbe39a1abb0 --- /dev/null +++ b/mobile/src/tasks/mobile-task-sort.test.ts @@ -0,0 +1,96 @@ +import { describe, expect, it, vi } from 'vitest' +import type { TaskItem } from './mobile-tasks-project-workspace-types' +import type { RepoSummary } from './mobile-tasks-provider-detail-types' +import { sortMobileTaskItems, taskRepositoryMeta } from './mobile-tasks-repository-presentation' +import { taskTime } from './mobile-tasks-item-mapping' + +vi.mock('./mobile-tasks-dependencies', () => import('../theme/mobile-theme')) + +function task( + provider: TaskItem['provider'], + label: string, + updatedAt: string, + key: string +): TaskItem { + const source = + provider === 'linear' + ? { team: { id: label, name: label }, state: { color: '' } } + : provider === 'gitlabTodo' + ? { projectPath: label } + : { repoId: label, repoName: label } + return { provider, source, updatedAt, key, title: key, subtitle: '', status: '' } as TaskItem +} +const repos = new Map([ + ['alias', { id: 'alias', displayName: 'Álpha', path: '/repo' }] +]) +const items = [ + task('github', 'alias', '2026-01-01', 'a'), + task('gitlab', 'alpha', '2026-02-01', 'b'), + task('gitlabTodo', 'Zeta', 'invalid', 'c'), + task('linear', 'Älpha', '2026-02-01', 'd'), + task('github', 'alpha', '2026-02-01', 'e'), + task('linear', 'Zeta', '1970-01-01', 'f') +] + +describe('mobile task sorting', () => { + it.each(['repository', 'updated'] as const)( + 'preserves %s order, ties, fallbacks, and input identity', + (sort) => { + const compareBefore = (a: TaskItem, b: TaskItem) => { + const labelOrder = + sort === 'repository' + ? taskRepositoryMeta(a, repos).label.localeCompare( + taskRepositoryMeta(b, repos).label, + undefined, + { sensitivity: 'base' } + ) + : 0 + return labelOrder || taskTime(b.updatedAt) - taskTime(a.updatedAt) + } + const expected = [...items].sort(compareBefore) + const input = Object.freeze([...items]) + const actual = sortMobileTaskItems(input, sort, repos) + expect(actual).toEqual(expected) + actual.forEach((item, index) => expect(item).toBe(expected[index])) + expect(input).toEqual(items) + repos.set('alias', { id: 'alias', displayName: 'zzzz', path: '/repo' }) + try { + expect(sortMobileTaskItems(input, sort, repos)).toEqual([...items].sort(compareBefore)) + } finally { + repos.set('alias', { id: 'alias', displayName: 'Álpha', path: '/repo' }) + } + } + ) + + it.each(['repository', 'updated'] as const)('computes %s keys only once per item', (sort) => { + const parse = vi.spyOn(Date, 'parse') + const getRepo = vi.spyOn(repos, 'get') + const localeCompare = vi.spyOn(String.prototype, 'localeCompare') + try { + sortMobileTaskItems(items, sort, repos) + expect(parse).toHaveBeenCalledTimes(items.length) + expect(getRepo).toHaveBeenCalledTimes(sort === 'repository' ? 3 : 0) + expect(localeCompare).not.toHaveBeenCalled() + } finally { + parse.mockRestore() + getRepo.mockRestore() + localeCompare.mockRestore() + } + }) + + it('skips setup for empty/singleton arrays while returning a fresh array', () => { + const parse = vi.spyOn(Date, 'parse') + const collator = vi.spyOn(Intl, 'Collator') + try { + expect(sortMobileTaskItems([], 'repository', repos)).toEqual([]) + const one = [items[0]] + expect(sortMobileTaskItems(one, 'repository', repos)).toEqual(one) + expect(sortMobileTaskItems(one, 'updated', repos)).not.toBe(one) + expect(parse).not.toHaveBeenCalled() + expect(collator).not.toHaveBeenCalled() + } finally { + parse.mockRestore() + collator.mockRestore() + } + }) +}) diff --git a/mobile/src/tasks/mobile-tasks-refactor-parity.test.ts b/mobile/src/tasks/mobile-tasks-refactor-parity.test.ts index 8d35398c3df..ce4e20995c4 100644 --- a/mobile/src/tasks/mobile-tasks-refactor-parity.test.ts +++ b/mobile/src/tasks/mobile-tasks-refactor-parity.test.ts @@ -16,50 +16,51 @@ const hash = (parts: string[] | string): string => .update(Array.isArray(parts) ? parts.join('\n') : parts) .digest('hex') -const PRE_REFACTOR_SCREEN_HOOKS = '42174315a76c475d09dcb7209af4481f01258c4c9dc012127ff07a893d8cd291' -const PRE_REFACTOR_DIFF_HOOKS = '93c7189b32bed8456cc51814fffa8ce80cf62011ef968a9d53ddec2b9686f58f' -const PRE_REFACTOR_STATEMENTS = '9323fbee7c3806f37de42578ba73ce659c786c0ed5f8b6bcbc321b201ca50a73' -const PRE_REFACTOR_DECLARATIONS = 'cff54172af17a877789be1479c2eb6ca97d83c3e31dd831cd59395962f2b4c4a' -const PRE_REFACTOR_SEMANTICS = '5219d210d6f274e9ce2716a37c4c6fc4860a736a80f059ab6e89da6123043263' -const PRE_REFACTOR_STYLES = '1db6af69c791d9963928541ad5310942fcbda6d984b422c90b6eb92b6816579a' -const PRE_REFACTOR_RENDER_TREE = '2111145136b1e4fbca150d4792d735a90e992488e9934cfc1a8b8f3be981f39f' +// Task-sort key precomputation is covered by mobile-task-sort.test.ts; other source guards remain. +const EXPECTED_SCREEN_HOOKS = '6bbbbbcea5faec9cef22c9e7b4ada24b69517bd7d6bd03259e5f239eca036a9d' +const EXPECTED_DIFF_HOOKS = '93c7189b32bed8456cc51814fffa8ce80cf62011ef968a9d53ddec2b9686f58f' +const EXPECTED_STATEMENTS = '71b6d3e02022568b0300cba3fa33e19b39509b9bfd987af6bb6f7d378a75939c' +const EXPECTED_DECLARATIONS = 'e79d877078362cef301ac8bdc46fef07eb0ac9414e5b8f752a879559ec6dbde1' +const EXPECTED_SEMANTICS = '4758ba019e4ff7cadd7ee02338719fa4fc4e1443e34cc290842819cfa1a70181' +const EXPECTED_STYLES = '1db6af69c791d9963928541ad5310942fcbda6d984b422c90b6eb92b6816579a' +const EXPECTED_RENDER_TREE = '2111145136b1e4fbca150d4792d735a90e992488e9934cfc1a8b8f3be981f39f' describe('Mobile Tasks refactor parity', () => { it('preserves recursively flattened hook and dependency order', () => { const screenHooks = readFlattenedMobileTasksHookSignatures('MobileTasksScreen') expect(screenHooks).toHaveLength(350) - expect(hash(screenHooks)).toBe(PRE_REFACTOR_SCREEN_HOOKS) + expect(hash(screenHooks)).toBe(EXPECTED_SCREEN_HOOKS) const diffHooks = readFlattenedMobileTasksHookSignatures('GitHubPrFileDiff') expect(diffHooks).toHaveLength(3) - expect(hash(diffHooks)).toBe(PRE_REFACTOR_DIFF_HOOKS) + expect(hash(diffHooks)).toBe(EXPECTED_DIFF_HOOKS) }) it('preserves every screen statement in execution order', () => { const statements = readFlattenedMobileTasksCoreStatements() expect(statements).toHaveLength(417) - expect(hash(statements)).toBe(PRE_REFACTOR_STATEMENTS) + expect(hash(statements)).toBe(EXPECTED_STATEMENTS) }) it('preserves every moved top-level declaration', () => { const declarations = readMobileTasksDeclarationSignatures() - expect(declarations).toHaveLength(193) - expect(hash(declarations)).toBe(PRE_REFACTOR_DECLARATIONS) + expect(declarations).toHaveLength(192) + expect(hash(declarations)).toBe(EXPECTED_DECLARATIONS) }) it('preserves RPC calls, runtime strings, and JSX host signatures', () => { const semantics = readMobileTasksSemanticSource() - expect(semantics.split('\n')).toHaveLength(3_499) - expect(hash(semantics)).toBe(PRE_REFACTOR_SEMANTICS) + expect(semantics.split('\n')).toHaveLength(3_500) + expect(hash(semantics)).toBe(EXPECTED_SEMANTICS) }) it('preserves render expressions and event handlers in tree order', () => { const tokens = readFlattenedMobileTasksRenderTokens() expect(tokens).toHaveLength(35_195) - expect(hash(tokens)).toBe(PRE_REFACTOR_RENDER_TREE) + expect(hash(tokens)).toBe(EXPECTED_RENDER_TREE) }) it('preserves every StyleSheet property and value', () => { - expect(hash(readMobileTasksStyleSource())).toBe(PRE_REFACTOR_STYLES) + expect(hash(readMobileTasksStyleSource())).toBe(EXPECTED_STYLES) }) }) diff --git a/mobile/src/tasks/mobile-tasks-repository-presentation.ts b/mobile/src/tasks/mobile-tasks-repository-presentation.ts index b3ef72c0a4f..ae263793304 100644 --- a/mobile/src/tasks/mobile-tasks-repository-presentation.ts +++ b/mobile/src/tasks/mobile-tasks-repository-presentation.ts @@ -2,6 +2,7 @@ import { colors } from './mobile-tasks-dependencies' import { taskTime } from './mobile-tasks-item-mapping' import type { TaskItem } from './mobile-tasks-project-workspace-types' import type { RepoSummary } from './mobile-tasks-provider-detail-types' +import type { TaskSort } from './mobile-tasks-view-state-types' export function isFailedGitHubCheck(check: { conclusion?: string | null }): boolean { return ['failure', 'cancelled', 'timed_out'].includes(check.conclusion ?? '') @@ -64,17 +65,25 @@ export function taskRepositoryMeta( } } -export function compareTasksByUpdated(a: TaskItem, b: TaskItem): number { - return taskTime(b.updatedAt) - taskTime(a.updatedAt) -} - -export function compareTasksByRepository( - a: TaskItem, - b: TaskItem, +export function sortMobileTaskItems( + items: readonly TaskItem[], + sort: TaskSort, reposById: Map -): number { - const aRepo = taskRepositoryMeta(a, reposById) - const bRepo = taskRepositoryMeta(b, reposById) - const repoComparison = aRepo.label.localeCompare(bRepo.label, undefined, { sensitivity: 'base' }) - return repoComparison || compareTasksByUpdated(a, b) +): TaskItem[] { + if (items.length < 2) { + return [...items] + } + const byRepository = sort === 'repository' + const collator = byRepository ? new Intl.Collator(undefined, { sensitivity: 'base' }) : null + return items + .map((item) => ({ + item, + updatedAt: taskTime(item.updatedAt), + repositoryLabel: byRepository ? taskRepositoryMeta(item, reposById).label : '' + })) + .sort( + (a, b) => + (collator?.compare(a.repositoryLabel, b.repositoryLabel) ?? 0) || b.updatedAt - a.updatedAt + ) + .map(({ item }) => item) } diff --git a/mobile/src/tasks/use-mobile-tasks-picker-projection.tsx b/mobile/src/tasks/use-mobile-tasks-picker-projection.tsx index 476e29155fd..90ab79d5d68 100644 --- a/mobile/src/tasks/use-mobile-tasks-picker-projection.tsx +++ b/mobile/src/tasks/use-mobile-tasks-picker-projection.tsx @@ -8,8 +8,7 @@ import { type RepoSummary, SORT_OPTIONS, type TaskListEntry, - compareTasksByRepository, - compareTasksByUpdated, + sortMobileTaskItems, getRepoBadgeColor, hasGitHubIssueSourceChoice, issueSourceSlug, @@ -133,15 +132,10 @@ export function useMobileTasksPickerProjection(model: DetailCommentRenderersMode })), [workspaceRepos] ) - const sortedItems = useMemo(() => { - const next = [...items] - if (taskSort === 'repository') { - next.sort((a, b) => compareTasksByRepository(a, b, reposById)) - } else { - next.sort(compareTasksByUpdated) - } - return next - }, [items, reposById, taskSort]) + const sortedItems = useMemo( + () => sortMobileTaskItems(items, taskSort, reposById), + [items, reposById, taskSort] + ) const displayedEntries = useMemo(() => { if (taskSort !== 'repository') { return sortedItems.map((item) => ({ type: 'item', key: item.key, item }))