Merge pull request #303 from Ark0N/feat/overview-activity-order

Sort the home-screen session lists by activity, not tab order

test/home-sessions.test.ts

@@ -2,11 +2,11 @@
 //
 // The desktop home screen's tab column (src/web/public/home-sessions.js) fills
 // the welcome overlay's left gutter. Two things about it can silently go wrong
-// and are pinned here: the row ORDER (it mirrors the tab strip, unlike the phone
-// overview which sorts by urgency, and the number badges are only correct if it
-// does), and the WIDTH GATE, which lives in two places at once — the JS constant
-// and a CSS media query — because the column is absolutely positioned and would
-// overlap the search panel in a narrow window.
+// and are pinned here: the row ORDER (shared with the phone overview via
+// CodemanSessionOrder, with the number badge still carrying the TAB index so
+// Alt+N keeps working), and the WIDTH GATE, which lives in two places at once —
+// the JS constant and a CSS media query — because the column is absolutely
+// positioned and would overlap the search panel in a narrow window.
 import { readFileSync } from 'node:fs';
 import { resolve } from 'node:path';
 import vm from 'node:vm';
@@ -35,9 +35,10 @@ function fakeElement(): any {
 
 /**
  * home-sessions.js reuses `_mobileOverviewState` / `_mobileOverviewCaseFor` /
- * `shouldUseMobileOverview` from mobile-overview.js, so both files run in the
- * same context — which is also the point: if that reuse ever breaks, these
- * tests stop loading rather than quietly testing a divergent copy.
+ * `shouldUseMobileOverview` from mobile-overview.js and the row comparator from
+ * constants.js, so all three files run in the same context, which is also the
+ * point: if that reuse ever breaks, these tests stop loading rather than
+ * quietly testing a divergent copy.
  */
 function loadHomeSessionsApp(overrides: Record<string, any> = {}, innerWidth = 1512) {
   const CodemanApp = function CodemanApp(this: any) {};
@@ -52,7 +53,7 @@ function loadHomeSessionsApp(overrides: Record<string, any> = {}, innerWidth = 1
     },
     MobileDetection: { getDeviceType: () => (innerWidth < 430 ? 'mobile' : 'desktop') },
   });
-  for (const file of ['mobile-overview.js', 'home-sessions.js']) {
+  for (const file of ['constants.js', 'mobile-overview.js', 'home-sessions.js']) {
     vm.runInContext(readFileSync(resolve(PUBLIC, file), 'utf8'), context, { filename: file });
   }
 
@@ -76,9 +77,9 @@ function sessionMap(list: Array<Record<string, any>>) {
 }
 
 describe('home sessions column: model', () => {
-  it('lists rows in TAB order, not by urgency, so the number badges match Alt+1..9', () => {
-    // The phone overview would hoist 'needy' to the top; this surface must not,
-    // because its badges are the Alt+N indices.
+  it('hoists a session blocked on you, and keeps its badge on the TAB index', () => {
+    // The badge names the Alt+N shortcut, so a sorted rail shows 2,1,3 rather
+    // than renumbering itself 1,2,3 and lying about which key selects what.
     const app = loadHomeSessionsApp({
       sessions: sessionMap([{ id: 'first' }, { id: 'needy' }, { id: 'third' }]),
       sessionOrder: ['first', 'needy', 'third'],
@@ -87,10 +88,34 @@ describe('home sessions column: model', () => {
     });
 
     const rows = app.buildHomeSessionRows();
-    expect(rows.map((r: any) => r.id)).toEqual(['first', 'needy', 'third']);
-    expect(rows.map((r: any) => r.index)).toEqual([0, 1, 2]);
-    expect(rows[1].state).toBe('needs');
-    expect(rows[1].pill).toBe('needs you');
+    expect(rows.map((r: any) => r.id)).toEqual(['needy', 'first', 'third']);
+    expect(rows.map((r: any) => r.orderIndex)).toEqual([1, 0, 2]);
+    expect(rows[0].state).toBe('needs');
+    expect(rows[0].pill).toBe('needs you');
+  });
+
+  it('orders running sessions longest-turn-first and quiet ones most-recent-first', () => {
+    // The same rule the phone overview follows, and the reason the rail exists:
+    // what is running longest is what is most likely to be done or stuck, and
+    // once nothing is running the session that just stopped is the one you came
+    // back for.
+    const app = loadHomeSessionsApp({
+      sessions: sessionMap([
+        { id: 'young-turn', status: 'busy', lastSubmitAt: 9_000, lastActivityAt: 10_000 },
+        { id: 'old-turn', status: 'busy', lastSubmitAt: 1_000, lastActivityAt: 10_000 },
+        { id: 'stale-idle', status: 'idle', lastActivityAt: 2_000 },
+        { id: 'fresh-idle', status: 'idle', lastActivityAt: 8_000 },
+      ]),
+      sessionOrder: ['young-turn', 'old-turn', 'stale-idle', 'fresh-idle'],
+      cases: CASES,
+    });
+
+    expect(app.buildHomeSessionRows().map((r: any) => r.id)).toEqual([
+      'old-turn',
+      'young-turn',
+      'fresh-idle',
+      'stale-idle',
+    ]);
   });
 
   it('shows a session that is not in the order list yet', () => {
@@ -117,11 +142,13 @@ describe('home sessions column: model', () => {
       cases: CASES,
     });
 
+    // Unstamped rows fall back to the tab order inside a state, so this reads
+    // as the state ranking alone: an errored session is blocked on you.
     expect(app.buildHomeSessionRows().map((r: any) => [r.state, r.pill])).toEqual([
+      ['error', 'error'],
       ['working', 'working'],
       ['idle', 'idle'],
       ['done', 'done'],
-      ['error', 'error'],
     ]);
   });
 
@@ -236,8 +263,10 @@ describe('home sessions column: wiring', () => {
     expect(aside).toBeGreaterThan(overlayStart);
     expect(aside).toBeLessThan(content);
     // Load order: the module reuses prototype methods installed by
-    // mobile-overview.js. Compare the <script> tags, not any mention: both
-    // files are named in explanatory comments earlier in the document.
+    // mobile-overview.js and the comparator installed by constants.js. Compare
+    // the <script> tags, not any mention: both files are named in explanatory
+    // comments earlier in the document.
     expect(html.indexOf('src="home-sessions.js"')).toBeGreaterThan(html.indexOf('src="mobile-overview.js"'));
+    expect(html.indexOf('src="mobile-overview.js"')).toBeGreaterThan(html.indexOf('src="constants.js"'));
   });
 });

test/mobile-overview.test.ts

@@ -42,9 +42,12 @@ function loadOverviewApp(overrides: Record<string, any> = {}) {
     },
     MobileDetection: { getDeviceType: () => 'mobile' },
   });
-  vm.runInContext(readFileSync(resolve(PUBLIC, 'mobile-overview.js'), 'utf8'), context, {
-    filename: 'mobile-overview.js',
-  });
+  // constants.js first: it installs the row comparator (window.CodemanSessionOrder)
+  // that buildMobileOverviewModel() sorts every section with, shared with the
+  // desktop rail so the two home screens cannot order the same list differently.
+  for (const file of ['constants.js', 'mobile-overview.js']) {
+    vm.runInContext(readFileSync(resolve(PUBLIC, file), 'utf8'), context, { filename: file });
+  }
 
   const app = new (CodemanApp as any)();
   app.getSessionName = (session: any) => session.name || session.workingDir?.split('/').pop() || session.id.slice(0, 8);
@@ -123,7 +126,7 @@ describe('mobile overview model', () => {
     expect(model.sessionCount).toBe(4);
   });
 
-  it('keeps the user tab order as the tiebreak inside a section', () => {
+  it('keeps the user tab order as the tiebreak when nothing is stamped', () => {
     const app = loadOverviewApp();
     const model = app.buildMobileOverviewModel({
       sessions: [session({ id: 'first' }), session({ id: 'second' }), session({ id: 'third' })],
@@ -134,6 +137,42 @@ describe('mobile overview model', () => {
     expect(model.current.map((r: any) => r.id)).toEqual(['third', 'first', 'second']);
   });
 
+  it('sorts running sessions longest-turn-first and quiet ones most-recent-first', () => {
+    // A working pane repaints about once a second, so its last-activity stamp
+    // is always "now": the running group has to key off the pane's last Enter
+    // instead, or every turn ranks as freshly started.
+    const app = loadOverviewApp();
+    const model = app.buildMobileOverviewModel({
+      sessions: [
+        session({ id: 'quiet-old', status: 'idle', lastActivityAt: 2_000 }),
+        session({ id: 'turn-young', status: 'busy', lastSubmitAt: 9_000, lastActivityAt: 10_000 }),
+        session({ id: 'quiet-new', status: 'idle', lastActivityAt: 8_000 }),
+        session({ id: 'turn-old', status: 'busy', lastSubmitAt: 1_000, lastActivityAt: 10_000 }),
+      ],
+      cases: CASES,
+      sessionOrder: ['quiet-old', 'turn-young', 'quiet-new', 'turn-old'],
+    });
+
+    expect(model.current.map((r: any) => r.id)).toEqual(['turn-old', 'turn-young', 'quiet-new', 'quiet-old']);
+  });
+
+  it('puts the longest-blocked session at the top of NEEDS YOU', () => {
+    const app = loadOverviewApp();
+    const model = app.buildMobileOverviewModel({
+      sessions: [
+        session({ id: 'just-asked', lastActivityAt: 9_000 }),
+        session({ id: 'starving', lastActivityAt: 1_000 }),
+      ],
+      cases: CASES,
+      pendingHooks: new Map([
+        ['just-asked', new Set(['permission_prompt'])],
+        ['starving', new Set(['permission_prompt'])],
+      ]),
+    });
+
+    expect(model.needsYou.map((r: any) => r.id)).toEqual(['starving', 'just-asked']);
+  });
+
   it('matches a session started in a subdirectory to its case (longest prefix)', () => {
     const app = loadOverviewApp();
     const model = app.buildMobileOverviewModel({

test/session-overview-order.test.ts

@@ -0,0 +1,175 @@
+// Port: none (pure comparator — no browser, no server).
+//
+// `CodemanSessionOrder` (src/web/public/constants.js) is the single row order
+// behind both home screens: the phone overview and the desktop tab rail. It is
+// the one place the two surfaces can disagree about which session you should
+// look at next, which is why it is pure and pinned here rather than living
+// inside either renderer.
+//
+// The rule it encodes, and the thing worth protecting: the tiebreak FLIPS
+// direction halfway down the list. For a state a session is still in, older is
+// more urgent (blocked longest, running longest). For a state it has stopped
+// in, newer is more relevant (just finished beats abandoned yesterday).
+import { readFileSync } from 'node:fs';
+import { resolve } from 'node:path';
+import vm from 'node:vm';
+import { describe, expect, it } from 'vitest';
+
+type Row = {
+  id: string;
+  state: string;
+  lastActivityAt?: number;
+  lastSubmitAt?: number;
+  orderIndex?: number;
+};
+
+function loadOrderHelper() {
+  const context = vm.createContext({ window: {}, globalThis: {} });
+  const source = readFileSync(resolve(import.meta.dirname, '../src/web/public/constants.js'), 'utf8');
+  vm.runInContext(source, context, { filename: 'constants.js' });
+  return (
+    context.window as {
+      CodemanSessionOrder: {
+        RANK: Record<string, number>;
+        anchor: (row: Row) => number;
+        compare: (a: Row, b: Row) => number;
+        sort: (rows: Row[]) => Row[];
+      };
+    }
+  ).CodemanSessionOrder;
+}
+
+const order = loadOrderHelper();
+const ids = (rows: Row[]) => order.sort(rows).map((r) => r.id);
+
+describe('session overview order: state ranking', () => {
+  it('puts everything blocked on a human above everything else', () => {
+    // Red question, then a hard error, then the yellow "waiting for input"
+    // prompt, then work, then whatever has stopped.
+    const rows: Row[] = [
+      { id: 'done', state: 'done' },
+      { id: 'idle', state: 'idle' },
+      { id: 'working', state: 'working' },
+      { id: 'waiting', state: 'waiting' },
+      { id: 'error', state: 'error' },
+      { id: 'needs', state: 'needs' },
+    ];
+    expect(ids(rows)).toEqual(['needs', 'error', 'waiting', 'working', 'idle', 'done']);
+  });
+
+  it('sorts an unknown state last instead of dropping it or crashing', () => {
+    // A state added to one renderer and not to the rank map must still render,
+    // just at the bottom — a missing row is a worse failure than a misplaced one.
+    const rows: Row[] = [
+      { id: 'mystery', state: 'quantum' },
+      { id: 'done', state: 'done' },
+    ];
+    expect(ids(rows)).toEqual(['done', 'mystery']);
+  });
+});
+
+describe('session overview order: in-progress states sort oldest first', () => {
+  it('ranks the longest-running turn above a turn that just started', () => {
+    const rows: Row[] = [
+      { id: 'young', state: 'working', lastSubmitAt: 9_000, lastActivityAt: 10_000 },
+      { id: 'old', state: 'working', lastSubmitAt: 1_000, lastActivityAt: 10_000 },
+    ];
+    expect(ids(rows)).toEqual(['old', 'young']);
+  });
+
+  it('measures a running turn from the last Enter, not the last repaint', () => {
+    // A working pane repaints about once a second, so last-activity is always
+    // "now" and would rank every running turn identically.
+    expect(order.anchor({ id: 'w', state: 'working', lastSubmitAt: 1_000, lastActivityAt: 999_000 })).toBe(1_000);
+    expect(order.anchor({ id: 'i', state: 'idle', lastSubmitAt: 1_000, lastActivityAt: 999_000 })).toBe(999_000);
+  });
+
+  it('falls back to last activity for a working pane that never submitted', () => {
+    // Spawned with its prompt on the command line, or an external CLI whose
+    // Enter never went through Codeman. Its fallback stamp is ~now, so it sits
+    // at the SHORT end of the running group rather than falsely leading it.
+    const rows: Row[] = [
+      { id: 'no-submit', state: 'working', lastActivityAt: 10_000 },
+      { id: 'submitted', state: 'working', lastSubmitAt: 1_000, lastActivityAt: 10_000 },
+    ];
+    expect(ids(rows)).toEqual(['submitted', 'no-submit']);
+  });
+
+  it('ranks the longest-blocked session above one that just asked', () => {
+    const rows: Row[] = [
+      { id: 'just-asked', state: 'needs', lastActivityAt: 9_000 },
+      { id: 'starving', state: 'needs', lastActivityAt: 1_000 },
+    ];
+    expect(ids(rows)).toEqual(['starving', 'just-asked']);
+  });
+});
+
+describe('session overview order: stopped states sort newest first', () => {
+  it('puts the session that just went quiet above one idle since yesterday', () => {
+    const rows: Row[] = [
+      { id: 'yesterday', state: 'idle', lastActivityAt: 1_000 },
+      { id: 'just-now', state: 'idle', lastActivityAt: 9_000 },
+      { id: 'this-morning', state: 'idle', lastActivityAt: 5_000 },
+    ];
+    expect(ids(rows)).toEqual(['just-now', 'this-morning', 'yesterday']);
+  });
+
+  it('applies the same recency rule to finished sessions', () => {
+    const rows: Row[] = [
+      { id: 'old-exit', state: 'done', lastActivityAt: 1_000 },
+      { id: 'fresh-exit', state: 'done', lastActivityAt: 9_000 },
+    ];
+    expect(ids(rows)).toEqual(['fresh-exit', 'old-exit']);
+  });
+});
+
+describe('session overview order: tiebreaks', () => {
+  it('falls back to the tab order when two rows share a stamp', () => {
+    const rows: Row[] = [
+      { id: 'third', state: 'idle', lastActivityAt: 5_000, orderIndex: 2 },
+      { id: 'first', state: 'idle', lastActivityAt: 5_000, orderIndex: 0 },
+    ];
+    expect(ids(rows)).toEqual(['first', 'third']);
+  });
+
+  it('sorts an unstamped row last within its state, never first', () => {
+    // 0 is "we have no stamp", not "the epoch": treating it as a timestamp
+    // would park a brand-new session at the head of the oldest-first groups.
+    expect(
+      ids([
+        { id: 'none', state: 'idle', orderIndex: 0 },
+        { id: 'stamped', state: 'idle', lastActivityAt: 1_000, orderIndex: 1 },
+      ])
+    ).toEqual(['stamped', 'none']);
+    expect(
+      ids([
+        { id: 'none', state: 'working', orderIndex: 0 },
+        { id: 'stamped', state: 'working', lastSubmitAt: 1_000, orderIndex: 1 },
+      ])
+    ).toEqual(['stamped', 'none']);
+  });
+
+  it('is deterministic: two unstamped rows keep tab order in both directions', () => {
+    const a: Row = { id: 'a', state: 'idle', orderIndex: 0 };
+    const b: Row = { id: 'b', state: 'idle', orderIndex: 1 };
+    expect(order.compare(a, b)).toBeLessThan(0);
+    expect(order.compare(b, a)).toBeGreaterThan(0);
+    expect(order.compare(a, a)).toBe(0);
+  });
+
+  it('copies rather than sorting the caller array in place', () => {
+    // Both renderers hand it a filtered slice of a shared row array; mutating
+    // that would reorder the other surface's list as a side effect.
+    const rows: Row[] = [
+      { id: 'b', state: 'idle', lastActivityAt: 1_000 },
+      { id: 'a', state: 'idle', lastActivityAt: 9_000 },
+    ];
+    order.sort(rows);
+    expect(rows.map((r) => r.id)).toEqual(['b', 'a']);
+  });
+
+  it('survives junk input rather than throwing inside a render', () => {
+    expect(order.sort(undefined as unknown as Row[])).toEqual([]);
+    expect(order.anchor({} as Row)).toBe(0);
+  });
+});