diff --git a/src/workspaces/officerdev/src/apps/Chat/order-tasks.test.ts b/src/workspaces/officerdev/src/apps/Chat/order-tasks.test.ts new file mode 100644 index 00000000..867993b0 --- /dev/null +++ b/src/workspaces/officerdev/src/apps/Chat/order-tasks.test.ts @@ -0,0 +1,49 @@ +import { describe, expect, test } from 'bun:test'; +import { orderTasks, type BackgroundTask } from './useBackgroundTasks'; + +// The scenario this exists for: a long-lived job is asked for, then short-lived ones run while you talk. +// Newest-first alone pushed the running job off the right-hand end of a strip that scrolls. +const running = (taskId: string): BackgroundTask => ({ role: 'task', taskId, description: taskId }); +const done = (taskId: string): BackgroundTask => ({ + role: 'task', + taskId, + description: taskId, + status: 'completed', + summary: 'done', +}); + +const ids = (tasks: BackgroundTask[]) => tasks.map((t) => t.taskId); + +describe('orderTasks', () => { + test('a running job stays ahead of finished ones that started later', () => { + // Newest-first input: two quick jobs finished after the dev server was started. + expect(ids(orderTasks([done('quick-2'), done('quick-1'), running('dev-server')]))).toEqual([ + 'dev-server', + 'quick-2', + 'quick-1', + ]); + }); + + test('newest-first survives inside each group', () => { + const input = [done('d3'), running('r3'), done('d2'), running('r2'), done('d1'), running('r1')]; + expect(ids(orderTasks(input))).toEqual(['r3', 'r2', 'r1', 'd3', 'd2', 'd1']); + }); + + test('a stopped or failed job counts as finished, not running', () => { + const stopped: BackgroundTask = { role: 'task', taskId: 'stopped', description: 'x', status: 'stopped' }; + const failed: BackgroundTask = { role: 'task', taskId: 'failed', description: 'x', status: 'failed' }; + expect(ids(orderTasks([stopped, failed, running('live')]))).toEqual(['live', 'stopped', 'failed']); + }); + + test('all-running and all-finished are left as they came', () => { + expect(ids(orderTasks([running('a'), running('b')]))).toEqual(['a', 'b']); + expect(ids(orderTasks([done('a'), done('b')]))).toEqual(['a', 'b']); + expect(orderTasks([])).toEqual([]); + }); + + test('does not mutate its input', () => { + const input = [done('d'), running('r')]; + orderTasks(input); + expect(ids(input)).toEqual(['d', 'r']); + }); +}); diff --git a/src/workspaces/officerdev/src/apps/Chat/useBackgroundTasks.ts b/src/workspaces/officerdev/src/apps/Chat/useBackgroundTasks.ts index 19374f25..046ef5d1 100644 --- a/src/workspaces/officerdev/src/apps/Chat/useBackgroundTasks.ts +++ b/src/workspaces/officerdev/src/apps/Chat/useBackgroundTasks.ts @@ -16,9 +16,10 @@ export type BackgroundTask = Extract; * lands while you are reading something else, and a chip that removes itself the instant it succeeds is a * chip you never saw. They do not stay forever, though — see FINISHED_KEPT. * - * NEWEST FIRST. The tray is a horizontal strip, so the far end of it is off-screen, and the task you want - * is almost always the one that just started. Chronological order put it exactly where you could not see - * it without scrolling. + * RUNNING FIRST, then newest first. The tray is a horizontal strip, so its far end is off-screen — and + * what falls off the end has to be the stuff you are not watching. Newest-first alone was not that: ask + * for a long-lived job, talk for a while, and the short-lived jobs an agent starts along the way each push + * it further right until the one thing still running is the one thing you cannot see. See `orderTasks`. */ /** @@ -30,6 +31,21 @@ export type BackgroundTask = Extract; */ const FINISHED_KEPT = 5; +/** + * Running tasks ahead of finished ones, newest first within each. + * + * A finished chip is a receipt — it is there so a notification you missed is still readable when you look + * up. A running chip is a live thing you may need to open or stop. Sorting them together by age treats + * those as the same kind of object, and the strip scrolls, so the difference shows up as the receipts + * slowly evicting the job from view. + * + * Two passes rather than a comparator: it does not depend on the sort being stable, and it reads as the + * partition it is. + */ +export function orderTasks(tasks: BackgroundTask[]): BackgroundTask[] { + return [...tasks.filter((t) => !t.status), ...tasks.filter((t) => t.status)]; +} + export function useBackgroundTasks(messages: ChatMessage[]) { const [dismissed, setDismissed] = useState([]); const [pinnedIds, setPinnedIds] = useState([]); @@ -44,7 +60,7 @@ export function useBackgroundTasks(messages: ChatMessage[]) { let finished = 0; // `tasks` is already newest-first, so counting forwards drops the oldest finished chips and keeps the // ones whose notifications are still recent enough to be worth reading. - return tasks.filter((t) => { + const kept = tasks.filter((t) => { if (dismissed.includes(t.taskId)) return false; // A pin is an explicit "keep this", so it outranks the cap — otherwise pinning the one task you care // about would not stop five newer ones from pushing it out. @@ -52,6 +68,9 @@ export function useBackgroundTasks(messages: ChatMessage[]) { finished += 1; return finished <= FINISHED_KEPT; }); + // Ordered after the cap, not before: the cap is about which finished chips are worth keeping, and that + // question is answered by age. Where they then sit is a different one. + return orderTasks(kept); }, [tasks, dismissed, pinnedIds]); const running = visible.filter((t) => !t.status);