From 570f195142a946efdc305ddf1504e42b8669edb9 Mon Sep 17 00:00:00 2001 From: Yash Datta Date: Sat, 22 Aug 2026 22:18:32 +0800 Subject: [PATCH] feat: order the session list by attention, not by when you created it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sidebar sorted by createdAt — under a comment that read "Sorted list — most recent activity first". The intent was always recency; the code delivered creation order. A session made last week but driven all morning sat at the bottom, which is why the order looked arbitrary. It could not be fixed client-side. The daemon has always tracked `#lastActivityAt` (it orders the resumed session list by it) but never put it on the wire, and `SessionInfo` carried only createdAt. web/src/state/messages.ts has a `lastActivityAt`, but it is derived from messages THIS client received, so it is 0 for every session you have not opened — a trap for the obvious fix. Adds `lastActivityAt` to SessionInfo (optional; clients fall back to createdAt so an older daemon does not sink every session to epoch 0) and bands the list: NEEDS YOU waiting_approval, error WORKING thinking, tool_running IDLE everything else, most recently active first Bands rather than one flat multi-key sort, for three reasons. It is already this project's answer — conductor-frontends-design.md §4 locks a state-grouped list as the fleet view's primary, and a second ordering vocabulary in the same client would be a bug in itself. It absorbs thrash: thinking and tool_running alternate several times a second and both are WORKING, so nothing moves, where a flat sort keyed on status would jitter continuously. And it makes movement legible — a row crossing under a NEEDS YOU header reads as a state change rather than a glitch. Two details that matter more than the sort: - A fleet bands and dates as ONE unit, by the most urgent state and the most recent activity anywhere in it. An orchestrator sits idle while its children work, so banding on the lead alone would file a fleet whose child is blocked on an approval under IDLE — exactly the case NEEDS YOU exists to surface. - Ordering is held while the pointer is over the list and applied on leave. A row that moves between aiming and clicking opens the wrong session. Membership is deliberately NOT held: a destroyed session must disappear, since leaving it clickable trades a misclick for a worse one. Ordering stays client-side. The daemon owns the state and exposes the field; each client decides how to render it, which is what lets the TUI and mobile band differently later. Mirrored in the Rust crate. 13 new tests over the pure ordering functions. Co-Authored-By: Claude Opus 5 (1M context) --- packages/protocol/src/types.ts | 15 ++ src/daemon/session.ts | 1 + web/src/components/SessionListPane.tsx | 44 ++++- web/src/lib/session-order.test.ts | 186 +++++++++++++++++++++ web/src/lib/session-order.ts | 217 +++++++++++++++++++++++++ web/src/state/sessions.ts | 24 ++- 6 files changed, 477 insertions(+), 10 deletions(-) create mode 100644 web/src/lib/session-order.test.ts create mode 100644 web/src/lib/session-order.ts diff --git a/packages/protocol/src/types.ts b/packages/protocol/src/types.ts index 6fbde56..3cbd6c1 100644 --- a/packages/protocol/src/types.ts +++ b/packages/protocol/src/types.ts @@ -202,6 +202,21 @@ export interface SessionInfo { status: SessionStatus; createdBy: string; createdAt: string; + /** + * ISO timestamp of the last time this session changed state — a turn started, + * a tool ran, it went idle. The daemon has always tracked this (it orders the + * resumed session list by it); this puts it on the wire so clients can order + * by RELEVANCE instead of by creation time. + * + * Bumped on state change, so it covers both "I just sent something" and "it + * just did something", and is deliberately NOT bumped by metadata-only writes + * like `rename()` — renaming a session must not reorder the list. + * + * Optional: absent from a daemon that predates it. Clients should fall back to + * `createdAt` rather than treating a missing value as "never active", which + * would sink every session on an older daemon to the bottom. + */ + lastActivityAt?: string; attachedClients: number; /** * Session role. "conductor" marks the per-tenant conductor session (the diff --git a/src/daemon/session.ts b/src/daemon/session.ts index 62cfabf..a86c074 100644 --- a/src/daemon/session.ts +++ b/src/daemon/session.ts @@ -2521,6 +2521,7 @@ export class Session { status: this.#status, createdBy: this.createdBy, createdAt: this.createdAt, + lastActivityAt: this.#lastActivityAt, attachedClients: this.#clients.size, role: this.role, providerId: this.#provider.id, diff --git a/web/src/components/SessionListPane.tsx b/web/src/components/SessionListPane.tsx index ffdbab8..2f28861 100644 --- a/web/src/components/SessionListPane.tsx +++ b/web/src/components/SessionListPane.tsx @@ -15,6 +15,13 @@ import { roleLabel, type FilteredFleetGroup, } from "../lib/fleet"; +import { + bandSections, + holdOrder, + snapshotOrder, + type BandedSection, + type OrderSnapshot, +} from "../lib/session-order"; import { sessionAgentLabel, shortSub } from "../lib/identity"; import { fetchPanels, liveProgress, livePanelMember, resetPanels } from "../state/panels"; import { nowTick } from "../state/clock"; @@ -91,6 +98,20 @@ const SessionListPane: Component = () => { filterFleet(groupFleet(sessionList()), filter()), ); + // Ordering is held while the pointer is over the list: a row that moves + // between aiming and clicking opens the wrong session. The pending reorder + // lands on leave. Membership still updates live — see holdOrder. + const [pointerInside, setPointerInside] = createSignal(false); + let heldOrder: OrderSnapshot | null = null; + const sections = createMemo[]>(() => { + const live = bandSections(groups()); + if (!pointerInside()) { + heldOrder = snapshotOrder(live); + return live; + } + return holdOrder(live, heldOrder); + }); + // The goal whose panels we poll, as a plain STRING. // // A memo over a primitive, deliberately. Reading `focusedSession()?.collaboration` @@ -156,11 +177,26 @@ const SessionListPane: Component = () => { > 0} fallback={}> -
    - - {(g) => } +
    setPointerInside(true)} + onPointerLeave={() => setPointerInside(false)} + > + + {(section) => ( +
      + + + {(g) => } + +
    + )}     -
+ diff --git a/web/src/lib/session-order.test.ts b/web/src/lib/session-order.test.ts new file mode 100644 index 0000000..46d4f2f --- /dev/null +++ b/web/src/lib/session-order.test.ts @@ -0,0 +1,186 @@ +/** + * Session-list ordering. + * + * The behaviour these pin, in order of how badly the failure reads to a user: + * + * 1. A session blocked on YOU is never buried under idle ones — including + * when the blocked party is a fleet's child and the orchestrator is idle. + * 2. The list orders by relevance, not creation time (the original bug). + * 3. It does not reorder under the pointer. + * 4. It degrades sanely against a daemon that never sends lastActivityAt. + */ + +import { describe, it, expect } from "vitest"; + +import { + BAND, + activityKey, + bandOf, + bandOfGroup, + bandSections, + compareByRecency, + holdOrder, + snapshotOrder, +} from "./session-order"; +import type { FleetGroup } from "./fleet"; +import type { SessionInfo, SessionStatus } from "../protocol/types"; + +function mk( + id: string, + over: Partial & { status?: SessionStatus } = {}, +): SessionInfo { + return { + id, + name: id, + workdir: `/repo/${id}`, + status: "idle", + createdBy: "u", + createdAt: "2026-01-01T00:00:00.000Z", + attachedClients: 0, + ...over, + } as SessionInfo; +} + +const group = (lead: SessionInfo, children: SessionInfo[] = []): FleetGroup => ({ + lead, + children, + isFleet: children.length > 0, +}); + +describe("bands", () => { + it("puts anything blocked on a human in NEEDS_YOU", () => { + expect(bandOf("waiting_approval")).toBe(BAND.NEEDS_YOU); + // A failed session is the other thing that wants a human; filing it under + // idle is how a failure goes unnoticed for an hour. + expect(bandOf("error")).toBe(BAND.NEEDS_YOU); + }); + + it("treats thinking and tool_running as one band so they cannot thrash", () => { + // These alternate several times a second. If they banded differently the + // row would jitter continuously. + expect(bandOf("thinking")).toBe(BAND.WORKING); + expect(bandOf("tool_running")).toBe(BAND.WORKING); + }); + + it("idle is idle", () => { + expect(bandOf("idle")).toBe(BAND.IDLE); + }); + + it("a fleet takes the most urgent band of anything in it", () => { + // The orchestrator sits idle while its children work — banding on the lead + // alone would hide a child that is blocked on an approval. + const fleet = group(mk("goal", { status: "idle" }), [ + mk("kid-1", { status: "idle" }), + mk("kid-2", { status: "waiting_approval" }), + ]); + expect(bandOfGroup(fleet)).toBe(BAND.NEEDS_YOU); + }); +}); + +describe("recency", () => { + it("orders by lastActivityAt, not createdAt — the original bug", () => { + // Old session, driven all morning; new session, untouched since creation. + const old = mk("old", { + createdAt: "2026-01-01T00:00:00.000Z", + lastActivityAt: "2026-06-01T12:00:00.000Z", + }); + const fresh = mk("fresh", { + createdAt: "2026-05-01T00:00:00.000Z", + lastActivityAt: "2026-05-01T00:00:00.000Z", + }); + expect([fresh, old].sort(compareByRecency).map((s) => s.id)).toEqual(["old", "fresh"]); + }); + + it("falls back to createdAt when the daemon never sends lastActivityAt", () => { + // An older daemon must not sink every session to epoch 0 — that would be + // worse than the behaviour being replaced. + const s = mk("legacy", { createdAt: "2026-03-03T00:00:00.000Z" }); + expect(activityKey(s)).toBe(Date.parse("2026-03-03T00:00:00.000Z")); + }); + + it("survives an unparseable timestamp instead of producing NaN order", () => { + expect(activityKey(mk("bad", { lastActivityAt: "not-a-date" }))).toBe(0); + }); + + it("a fleet is as recent as its busiest child", () => { + const fleet = group( + mk("goal", { lastActivityAt: "2026-01-01T00:00:00.000Z" }), + [mk("kid", { lastActivityAt: "2026-09-09T00:00:00.000Z" })], + ); + const solo = group(mk("solo", { lastActivityAt: "2026-05-05T00:00:00.000Z" })); + const [section] = bandSections([solo, fleet]); + expect(section!.groups.map((g) => g.lead.id)).toEqual(["goal", "solo"]); + }); +}); + +describe("bandSections", () => { + it("orders bands NEEDS YOU → WORKING → IDLE and drops empty ones", () => { + const sections = bandSections([ + group(mk("i", { status: "idle" })), + group(mk("w", { status: "thinking" })), + group(mk("n", { status: "waiting_approval" })), + ]); + expect(sections.map((s) => s.label)).toEqual(["Needs you", "Working", "Idle"]); + expect(sections.map((s) => s.groups[0]!.lead.id)).toEqual(["n", "w", "i"]); + + // No empty headers when a band has nothing in it. + expect(bandSections([group(mk("only", { status: "idle" }))]).map((s) => s.label)).toEqual([ + "Idle", + ]); + }); + + it("is deterministic for same-millisecond sessions", () => { + // A fan-out creates children within the same millisecond; without a + // tie-break they would swap places on every re-render. + const at = "2026-04-04T00:00:00.000Z"; + const a = group(mk("bbb", { lastActivityAt: at })); + const b = group(mk("aaa", { lastActivityAt: at })); + const once = bandSections([a, b]).flatMap((s) => s.groups.map((g) => g.lead.id)); + const twice = bandSections([b, a]).flatMap((s) => s.groups.map((g) => g.lead.id)); + expect(once).toEqual(twice); + }); +}); + +describe("holdOrder — does not reorder under the pointer", () => { + it("keeps a row in place when its band changes mid-hover", () => { + const idle = group(mk("a", { status: "idle", lastActivityAt: "2026-01-02T00:00:00.000Z" })); + const other = group(mk("b", { status: "idle", lastActivityAt: "2026-01-01T00:00:00.000Z" })); + const before = bandSections([idle, other]); + const snap = snapshotOrder(before); + expect(before.map((s) => s.label)).toEqual(["Idle"]); + + // 'b' starts asking for approval — normally it would jump to a new top band. + const after = bandSections([ + idle, + group(mk("b", { status: "waiting_approval", lastActivityAt: "2026-01-01T00:00:00.000Z" })), + ]); + expect(after.map((s) => s.label)).toEqual(["Needs you", "Idle"]); + + // Held: the layout the user is pointing at is unchanged. + const held = holdOrder(after, snap); + expect(held.map((s) => s.label)).toEqual(["Idle"]); + expect(held[0]!.groups.map((g) => g.lead.id)).toEqual(["a", "b"]); + }); + + it("still shows new sessions and drops destroyed ones while holding", () => { + // Order is frozen; MEMBERSHIP is not. Freezing membership would leave a + // destroyed session clickable — a worse bug than the one being prevented. + const a = group(mk("a", { status: "idle" })); + const b = group(mk("b", { status: "idle" })); + const snap = snapshotOrder(bandSections([a, b])); + + const c = group(mk("c", { status: "idle" })); + const held = holdOrder(bandSections([a, c]), snap); // b destroyed, c appeared + const ids = held.flatMap((s) => s.groups.map((g) => g.lead.id)); + expect(ids).toContain("a"); + expect(ids).toContain("c"); + expect(ids).not.toContain("b"); + // The newcomer appends rather than displacing the row being aimed at. + expect(ids.indexOf("c")).toBeGreaterThan(ids.indexOf("a")); + }); + + it("is a no-op without a snapshot", () => { + const live = bandSections([group(mk("a", { status: "waiting_approval" }))]); + expect(holdOrder(live, null)).toEqual(live); + }); +}); diff --git a/web/src/lib/session-order.ts b/web/src/lib/session-order.ts new file mode 100644 index 0000000..43a7af4 --- /dev/null +++ b/web/src/lib/session-order.ts @@ -0,0 +1,217 @@ +/** + * Session-list ordering — attention bands, then recency. + * + * The list used to sort by `createdAt`, which is why it read as arbitrary: when + * you made a session has nothing to do with whether it wants you now. The + * operator's actual question, in order, is "who is blocked on me", "what is + * running", "what did I just leave". + * + * Bands rather than one flat multi-key sort, for three reasons: + * + * 1. It is already this project's answer. conductor-frontends-design.md §4 + * locks a state-grouped list (Needs you / Working / …) as the fleet view's + * primary, partly because it renders identically in Solid and ratatui. A + * second ordering vocabulary in the same client would be a bug in itself. + * 2. It absorbs thrash. `thinking` and `tool_running` alternate several times + * a second; both are WORKING, so nothing moves. A flat sort keyed on status + * would jitter continuously. + * 3. It makes movement legible. A row crossing under a NEEDS YOU header reads + * as a state change; the same row silently rising in a flat list reads as a + * glitch. + * + * Pure functions, no Solid — the ordering is the part worth testing, and the + * tests shouldn't need a reactive root. Mirrors `lib/fleet.ts`. + */ + +import type { SessionInfo, SessionStatus } from "../protocol/types"; +import type { FleetGroup } from "./fleet"; + +/** Bands, most-urgent first. Numeric so comparisons are a subtraction. */ +export const BAND = { + NEEDS_YOU: 0, + WORKING: 1, + IDLE: 2, +} as const; + +export type Band = (typeof BAND)[keyof typeof BAND]; + +export const BAND_LABEL: Record = { + [BAND.NEEDS_YOU]: "Needs you", + [BAND.WORKING]: "Working", + [BAND.IDLE]: "Idle", +}; + +/** + * Which band one session belongs to. + * + * `error` is deliberately NEEDS_YOU, not IDLE: a failed session is the other + * thing that wants a human, and burying it under idle sessions is how a failure + * goes unnoticed for an hour. + */ +export function bandOf(status: SessionStatus): Band { + switch (status) { + case "waiting_approval": + case "error": + return BAND.NEEDS_YOU; + case "thinking": + case "tool_running": + return BAND.WORKING; + default: + return BAND.IDLE; + } +} + +/** + * A fleet takes the most urgent band of anything in it. + * + * An orchestrator sits `idle` while its role-children work, so banding a fleet + * by its lead alone would file a fleet whose child is blocked on an approval + * under IDLE — the exact case the NEEDS YOU band exists to surface. The group is + * one unit of work (see lib/fleet.ts) and bands as one. + */ +export function bandOfGroup(group: FleetGroup): Band { + let band = bandOf(group.lead.status); + for (const child of group.children) { + const b = bandOf(child.status); + if (b < band) band = b; + } + return band; +} + +/** + * Recency key. Falls back to `createdAt` when `lastActivityAt` is absent — a + * daemon that predates the field would otherwise report every session as + * epoch-0 and sink the entire list into reverse-arbitrary order, which is worse + * than the behaviour being replaced. + */ +export function activityKey(s: SessionInfo): number { + const raw = s.lastActivityAt ?? s.createdAt; + const t = Date.parse(raw); + return Number.isNaN(t) ? 0 : t; +} + +/** Most recently active first. */ +export function compareByRecency(a: SessionInfo, b: SessionInfo): number { + return activityKey(b) - activityKey(a); +} + +/** + * A fleet's recency is the most recent activity anywhere in it — same reasoning + * as the band: the unit of work is the group, and a fleet whose children are + * busy is not stale just because its orchestrator is between turns. + */ +export function groupRecency(group: FleetGroup): number { + let newest = activityKey(group.lead); + for (const child of group.children) { + const k = activityKey(child); + if (k > newest) newest = k; + } + return newest; +} + +export interface BandedSection { + band: Band; + label: string; + groups: G[]; +} + +/** + * Split groups into bands, recency-ordered within each, dropping empty bands. + * + * Stable by construction: `band` and `groupRecency` are both derived purely from + * the input, so the same input always yields the same order — no dependence on + * arrival order or object identity. + */ +export function bandSections(groups: readonly G[]): BandedSection[] { + const byBand = new Map(); + for (const g of groups) { + const band = bandOfGroup(g); + const bucket = byBand.get(band); + if (bucket) bucket.push(g); + else byBand.set(band, [g]); + } + + const sections: BandedSection[] = []; + for (const band of [BAND.NEEDS_YOU, BAND.WORKING, BAND.IDLE] as const) { + const bucket = byBand.get(band); + if (!bucket || bucket.length === 0) continue; + bucket.sort((a, b) => { + const diff = groupRecency(b) - groupRecency(a); + // Tie-break by id so two sessions created in the same millisecond — a + // fleet's children, a scripted burst — have a fixed order instead of + // swapping places on every re-render. + return diff !== 0 ? diff : a.lead.id.localeCompare(b.lead.id); + }); + sections.push({ band, label: BAND_LABEL[band], groups: bucket }); + } + return sections; +} + +/** + * Where a group sat the last time ordering was allowed to change. + * Built by {@link snapshotOrder}, consumed by {@link holdOrder}. + */ +export type OrderSnapshot = ReadonlyMap; + +export function snapshotOrder( + sections: readonly BandedSection[], +): OrderSnapshot { + const snap = new Map(); + for (const s of sections) { + s.groups.forEach((g, i) => snap.set(g.lead.id, { band: s.band, index: i })); + } + return snap; +} + +/** + * Re-project live groups onto a frozen layout — the anti-misclick guarantee. + * + * Rows move when sessions change state, which is the point; but a row moving + * in the instant between aiming and clicking means opening the wrong session. + * The sidebar therefore holds its layout while the pointer is over it and + * applies the pending reorder on leave. + * + * Membership is NOT frozen, only order: a session that appeared is shown (in its + * live band, appended), and one that was destroyed disappears. Freezing + * membership too would leave a dead session clickable, trading a misclick for a + * worse one. + */ +export function holdOrder( + sections: readonly BandedSection[], + snapshot: OrderSnapshot | null, +): BandedSection[] { + if (!snapshot || snapshot.size === 0) return sections as BandedSection[]; + + const held = new Map(); + const fresh: G[] = []; + for (const section of sections) { + for (const g of section.groups) { + const prev = snapshot.get(g.lead.id); + if (!prev) { + fresh.push(g); + continue; + } + const bucket = held.get(prev.band); + if (bucket) bucket.push(g); + else held.set(prev.band, [g]); + } + } + for (const bucket of held.values()) { + bucket.sort((a, b) => snapshot.get(a.lead.id)!.index - snapshot.get(b.lead.id)!.index); + } + // Newcomers go to the END of their LIVE band: they have no frozen position, + // and appending is the one placement that cannot displace a row being aimed at. + for (const g of fresh) { + const band = bandOfGroup(g); + const bucket = held.get(band); + if (bucket) bucket.push(g); + else held.set(band, [g]); + } + + const out: BandedSection[] = []; + for (const band of [BAND.NEEDS_YOU, BAND.WORKING, BAND.IDLE] as const) { + const bucket = held.get(band); + if (bucket && bucket.length > 0) out.push({ band, label: BAND_LABEL[band], groups: bucket }); + } + return out; +} diff --git a/web/src/state/sessions.ts b/web/src/state/sessions.ts index f90982e..20034fe 100644 --- a/web/src/state/sessions.ts +++ b/web/src/state/sessions.ts @@ -13,6 +13,7 @@ import type { SessionInfo, SessionStatus } from "../protocol/types"; import { clearSessionMessages, setFocusedSessionAccessor } from "./messages"; import { clearResumeCursor } from "./resume"; import { clearDraft } from "./prompt-drafts"; +import { compareByRecency } from "../lib/session-order"; interface SessionsState { byId: Record; @@ -26,10 +27,21 @@ const [focusedId, setFocusedId] = createSignal(null); * arriving out of order on reconnect. Non-reactive side map. */ const statusUpdatedAt = new Map(); -/** Sorted list — most recent activity first. */ +/** + * Sorted list — most recent activity first. + * + * Was `createdAt`, under this same comment: the intent was always recency, the + * code delivered creation order, and a session you made last week but have been + * driving all morning sat at the bottom. `compareByRecency` uses the daemon's + * `lastActivityAt` (falling back to `createdAt` on an older daemon). + * + * This is the base order. The sidebar bands it by attention state on top — see + * lib/session-order.ts — but everything else consuming `sessionList()` gets a + * sensible most-recent-first list for free. + */ export const sessionList = createMemo(() => { const items = Object.values(state.byId); - items.sort((a, b) => Date.parse(b.createdAt) - Date.parse(a.createdAt)); + items.sort(compareByRecency); return items; }); @@ -148,12 +160,12 @@ export function ingestSessionList( clearDraft(id); statusUpdatedAt.delete(id); } - // Auto-focus the most recently-created session if nothing is focused. + // Auto-focus the most recently-ACTIVE session if nothing is focused. + // Creation order picked whatever session happened to be newest, which after + // a daemon restart is rarely the one you were working in. const cur = focusedId(); if (!cur || !state.byId[cur]) { - const sorted = [...items].sort( - (a, b) => Date.parse(b.createdAt) - Date.parse(a.createdAt), - ); + const sorted = [...items].sort(compareByRecency); setFocusedId(sorted[0]?.id ?? null); } });