From ec84eb7342ed613537bbc8fe047f108295a90ce6 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Sat, 1 Aug 2026 20:46:55 -0500 Subject: [PATCH] fix(ui): defer Inbox re-sort to attention boundaries (#10623) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The Inbox lists work items by recent activity. > - An archive changes activity data and can change the computed order. > - The list can then move rows under the pointer during a fast archive sequence. > - The Inbox must keep its shown order while the operator is engaged. > - This pull request adopts fresh order only at an idle, visibility, or view-change boundary. > - The benefit is a stable Inbox that still receives fresh data and new items. ## Linked Issues or Issue Description **What happened** The Inbox re-sorts while an operator archives items quickly. An archive can lower a parent group's activity time. Unrelated rows then move, and the row under the pointer can change. **Expected behavior** Keep the shown order stable while the operator works in the Inbox. Insert new items at their computed positions. Adopt a fresh order after an idle period, after a long hidden-tab interval, or when the view changes. **Steps to reproduce** 1. Open the Inbox on the Mine tab with several items, including nested items. 2. Archive several items quickly with the pointer or keyboard. 3. Observe rows move before the archive sequence is complete. **Deployment mode** Any deployment. This change affects only the web UI. **Additional context** This PR supersedes the closed subset PR #10621. It keeps the full change in one review. ## What Changed - Added an order-pin utility for sections, root rows, non-issue items, and nested children. - Kept archived rows in place for the existing five-second undo interval. - Added an attention-boundary hook for idle, visibility, and view-change commits. - Connected pointer, wheel, hover, keyboard, and archive interactions to the idle boundary. - Made idle commits repeat while the Inbox stays idle. - Made view changes adopt the fresh order in the same render. - Added unit and integration coverage for order pins and attention boundaries. ## Verification - `pnpm exec vitest run ui/src/hooks/useInboxSortAttention.test.tsx ui/src/lib/inboxOrderPin.test.ts ui/src/pages/Inbox.test.tsx` — 33 tests passed. - `pnpm check:token-gates` — passed. - `pnpm -r typecheck` — passed. - `pnpm build` — passed. - `pnpm test:run` — 3,429 tests passed, 2 skipped, and 1 unrelated server test failed. The isolated failure expects `issue_commented` but current master records `heartbeat.scheduling_suppressed` in `plugin-orchestration-apis.test.ts`. ## Risks - Low risk. The change is client-only and does not change the sort algorithm. - A bad pin can show an old order until the next boundary. The tests cover repeated idle commits and immediate view changes. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Anthropic Claude Opus 4.8 (`claude-opus-4-8`), extended thinking, tool use, and code execution for the implementation. - OpenAI Codex (`gpt-5`), reasoning, tool use, and code execution for PR preparation and verification. The serving snapshot, context-window size, and hidden reasoning configuration were not exposed. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip --- ui/src/hooks/useInboxSortAttention.test.tsx | 175 ++++++++++++++ ui/src/hooks/useInboxSortAttention.ts | 127 ++++++++++ ui/src/lib/inboxArchiveCache.ts | 2 +- ui/src/lib/inboxOrderPin.test.ts | 230 +++++++++++++++++++ ui/src/lib/inboxOrderPin.ts | 242 ++++++++++++++++++++ ui/src/pages/Inbox.test.tsx | 101 ++++++++ ui/src/pages/Inbox.tsx | 101 +++++++- 7 files changed, 972 insertions(+), 6 deletions(-) create mode 100644 ui/src/hooks/useInboxSortAttention.test.tsx create mode 100644 ui/src/hooks/useInboxSortAttention.ts create mode 100644 ui/src/lib/inboxOrderPin.test.ts create mode 100644 ui/src/lib/inboxOrderPin.ts diff --git a/ui/src/hooks/useInboxSortAttention.test.tsx b/ui/src/hooks/useInboxSortAttention.test.tsx new file mode 100644 index 0000000000..b4016e0dd2 --- /dev/null +++ b/ui/src/hooks/useInboxSortAttention.test.tsx @@ -0,0 +1,175 @@ +// @vitest-environment jsdom + +import { useEffect } from "react"; +import { flushSync } from "react-dom"; +import { createRoot } from "react-dom/client"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { + INBOX_SORT_HIDDEN_COMMIT_MS, + INBOX_SORT_IDLE_COMMIT_MS, + type InboxSortAttention, + type InboxSortCommitReason, + useInboxSortAttention, +} from "./useInboxSortAttention"; + +type HarnessProps = { + viewIdentity: string; + onCommit: (reason: InboxSortCommitReason) => void; + onReady: (attention: InboxSortAttention) => void; +}; + +function Harness({ viewIdentity, onCommit, onReady }: HarnessProps) { + const attention = useInboxSortAttention({ viewIdentity, onCommit }); + useEffect(() => onReady(attention), [attention, onReady]); + return null; +} + +function setVisibility(state: "visible" | "hidden") { + Object.defineProperty(document, "visibilityState", { + configurable: true, + get: () => state, + }); +} + +function mountHook(viewIdentity = "mine:all:none") { + const container = document.createElement("div"); + const root = createRoot(container); + const onCommit = vi.fn<(reason: InboxSortCommitReason) => void>(); + let attention: InboxSortAttention | null = null; + const onReady = (value: InboxSortAttention) => { + attention = value; + }; + + const render = (nextViewIdentity = viewIdentity) => { + flushSync(() => { + root.render( + , + ); + }); + }; + + render(); + return { + onCommit, + render, + attention: () => { + if (!attention) throw new Error("Hook did not mount"); + return attention; + }, + unmount: () => flushSync(() => root.unmount()), + }; +} + +describe("useInboxSortAttention", () => { + beforeEach(() => { + vi.useFakeTimers(); + setVisibility("visible"); + }); + + afterEach(() => { + vi.runOnlyPendingTimers(); + vi.useRealTimers(); + setVisibility("visible"); + }); + + it("commits after the visible inbox is idle for the threshold", () => { + const hook = mountHook(); + hook.onCommit.mockClear(); + + vi.advanceTimersByTime(INBOX_SORT_IDLE_COMMIT_MS - 1); + expect(hook.onCommit).not.toHaveBeenCalled(); + + vi.advanceTimersByTime(1); + expect(hook.onCommit).toHaveBeenCalledWith("idle"); + hook.unmount(); + }); + + it("keeps committing every interval while the inbox stays idle", () => { + const hook = mountHook(); + hook.onCommit.mockClear(); + + vi.advanceTimersByTime(INBOX_SORT_IDLE_COMMIT_MS); + vi.advanceTimersByTime(INBOX_SORT_IDLE_COMMIT_MS); + vi.advanceTimersByTime(INBOX_SORT_IDLE_COMMIT_MS); + // The idle timer re-arms after each fire, so a long-idle inbox never freezes + // at the first snapshot. + const idleCalls = hook.onCommit.mock.calls.filter(([reason]) => reason === "idle"); + expect(idleCalls.length).toBe(3); + hook.unmount(); + }); + + it("resets the idle threshold on inbox interaction", () => { + const hook = mountHook(); + hook.onCommit.mockClear(); + + vi.advanceTimersByTime(INBOX_SORT_IDLE_COMMIT_MS - 1_000); + hook.attention().noteInteraction(); + vi.advanceTimersByTime(1_000); + expect(hook.onCommit).not.toHaveBeenCalled(); + + vi.advanceTimersByTime(INBOX_SORT_IDLE_COMMIT_MS - 1_000); + expect(hook.onCommit).toHaveBeenCalledWith("idle"); + hook.unmount(); + }); + + it("commits after a long hide but not after a brief hide", () => { + const hook = mountHook(); + hook.onCommit.mockClear(); + + setVisibility("hidden"); + document.dispatchEvent(new Event("visibilitychange")); + vi.advanceTimersByTime(INBOX_SORT_HIDDEN_COMMIT_MS - 1); + setVisibility("visible"); + document.dispatchEvent(new Event("visibilitychange")); + expect(hook.onCommit).not.toHaveBeenCalled(); + + setVisibility("hidden"); + document.dispatchEvent(new Event("visibilitychange")); + vi.advanceTimersByTime(INBOX_SORT_HIDDEN_COMMIT_MS); + setVisibility("visible"); + document.dispatchEvent(new Event("visibilitychange")); + expect(hook.onCommit).toHaveBeenCalledTimes(1); + expect(hook.onCommit).toHaveBeenCalledWith("visibility"); + hook.unmount(); + }); + + it("always commits when remounted", () => { + const first = mountHook(); + expect(first.onCommit).toHaveBeenCalledWith("mount"); + first.unmount(); + + const second = mountHook(); + expect(second.onCommit).toHaveBeenCalledWith("mount"); + second.unmount(); + }); + + it("commits for tab, filter, and group-by identity changes", () => { + const hook = mountHook(); + hook.onCommit.mockClear(); + + hook.render("assigned:open:project"); + hook.render("assigned:closed:project"); + hook.render("assigned:closed:assignee"); + expect(hook.onCommit).toHaveBeenCalledTimes(3); + expect(hook.onCommit).toHaveBeenNthCalledWith(1, "view-identity-change"); + expect(hook.onCommit).toHaveBeenNthCalledWith(2, "view-identity-change"); + expect(hook.onCommit).toHaveBeenNthCalledWith(3, "view-identity-change"); + hook.unmount(); + }); + + it("always commits on manual refresh", () => { + const hook = mountHook(); + hook.onCommit.mockClear(); + + hook.attention().commitManualRefresh(); + hook.attention().commitManualRefresh(); + expect(hook.onCommit).toHaveBeenCalledTimes(2); + expect(hook.onCommit).toHaveBeenNthCalledWith(1, "manual-refresh"); + expect(hook.onCommit).toHaveBeenNthCalledWith(2, "manual-refresh"); + hook.unmount(); + }); +}); diff --git a/ui/src/hooks/useInboxSortAttention.ts b/ui/src/hooks/useInboxSortAttention.ts new file mode 100644 index 0000000000..b6de99f0c1 --- /dev/null +++ b/ui/src/hooks/useInboxSortAttention.ts @@ -0,0 +1,127 @@ +import { useCallback, useEffect, useRef } from "react"; + +export const INBOX_SORT_IDLE_COMMIT_MS = 150_000; +export const INBOX_SORT_HIDDEN_COMMIT_MS = 30_000; + +export type InboxSortCommitReason = + | "mount" + | "idle" + | "visibility" + | "view-identity-change" + | "manual-refresh"; + +type UseInboxSortAttentionOptions = { + viewIdentity: string; + onCommit: (reason: InboxSortCommitReason) => void; + idleCommitMs?: number; + hiddenCommitMs?: number; +}; + +export type InboxSortAttention = { + /** Call for pointer, keyboard, hover, archive, expand/collapse, or scroll activity. */ + noteInteraction: () => void; + /** Commit immediately at an explicit refresh boundary. */ + commitManualRefresh: () => void; +}; + +function isDocumentVisible(): boolean { + return typeof document === "undefined" || document.visibilityState === "visible"; +} + +/** + * Owns the attention boundaries at which the inbox may safely adopt a newly + * computed sort order. The caller owns order reconciliation and calls + * `noteInteraction` for activity scoped to the inbox list. + */ +export function useInboxSortAttention({ + viewIdentity, + onCommit, + idleCommitMs = INBOX_SORT_IDLE_COMMIT_MS, + hiddenCommitMs = INBOX_SORT_HIDDEN_COMMIT_MS, +}: UseInboxSortAttentionOptions): InboxSortAttention { + const onCommitRef = useRef(onCommit); + const idleTimerRef = useRef | null>(null); + const hiddenAtRef = useRef(null); + const committedInitialViewRef = useRef(false); + const previousViewIdentityRef = useRef(viewIdentity); + + useEffect(() => { + onCommitRef.current = onCommit; + }, [onCommit]); + + const clearIdleTimer = useCallback(() => { + if (idleTimerRef.current !== null) { + clearTimeout(idleTimerRef.current); + idleTimerRef.current = null; + } + }, []); + + const restartIdleTimer = useCallback(() => { + clearIdleTimer(); + if (!isDocumentVisible()) return; + + // Re-arm after each idle commit so an inbox left untouched keeps adopting the + // freshly computed order every interval, rather than freezing at the first + // idle snapshot until the next interaction or attention boundary. + const armIdleTimer = () => { + idleTimerRef.current = setTimeout(() => { + idleTimerRef.current = null; + if (!isDocumentVisible()) return; + onCommitRef.current("idle"); + armIdleTimer(); + }, idleCommitMs); + }; + armIdleTimer(); + }, [clearIdleTimer, idleCommitMs]); + + const noteInteraction = useCallback(() => { + restartIdleTimer(); + }, [restartIdleTimer]); + + const commitManualRefresh = useCallback(() => { + onCommitRef.current("manual-refresh"); + restartIdleTimer(); + }, [restartIdleTimer]); + + useEffect(() => { + if (!committedInitialViewRef.current) { + committedInitialViewRef.current = true; + onCommitRef.current("mount"); + } else if (previousViewIdentityRef.current !== viewIdentity) { + onCommitRef.current("view-identity-change"); + } + + previousViewIdentityRef.current = viewIdentity; + restartIdleTimer(); + return clearIdleTimer; + }, [clearIdleTimer, restartIdleTimer, viewIdentity]); + + useEffect(() => { + if (typeof document === "undefined") return; + + if (document.visibilityState !== "visible") { + hiddenAtRef.current = Date.now(); + clearIdleTimer(); + } + + const handleVisibilityChange = () => { + if (document.visibilityState !== "visible") { + hiddenAtRef.current = Date.now(); + clearIdleTimer(); + return; + } + + const hiddenAt = hiddenAtRef.current; + hiddenAtRef.current = null; + if (hiddenAt !== null && Date.now() - hiddenAt >= hiddenCommitMs) { + onCommitRef.current("visibility"); + } + restartIdleTimer(); + }; + + document.addEventListener("visibilitychange", handleVisibilityChange); + return () => document.removeEventListener("visibilitychange", handleVisibilityChange); + }, [clearIdleTimer, hiddenCommitMs, restartIdleTimer]); + + return { noteInteraction, commitManualRefresh }; +} diff --git a/ui/src/lib/inboxArchiveCache.ts b/ui/src/lib/inboxArchiveCache.ts index 92b185f936..1f8dd12ffa 100644 --- a/ui/src/lib/inboxArchiveCache.ts +++ b/ui/src/lib/inboxArchiveCache.ts @@ -5,7 +5,7 @@ import { queryKeys } from "./queryKeys"; export type InboxIssueCacheSnapshot = Array; -const INBOX_ARCHIVE_CONFIRMATION_GRACE_MS = 5_000; +export const INBOX_ARCHIVE_CONFIRMATION_GRACE_MS = 5_000; const INBOX_ARCHIVE_MAX_GUARD_MS = 30_000; const EMPTY_ARCHIVED_ISSUE_IDS: ReadonlySet = new Set(); diff --git a/ui/src/lib/inboxOrderPin.test.ts b/ui/src/lib/inboxOrderPin.test.ts new file mode 100644 index 0000000000..30d3269d26 --- /dev/null +++ b/ui/src/lib/inboxOrderPin.test.ts @@ -0,0 +1,230 @@ +import type { Approval, HeartbeatRun, Issue, JoinRequest } from "@paperclipai/shared"; +import { describe, expect, it } from "vitest"; +import { + getInboxWorkItemKey, + type InboxGroupedSection, + type InboxWorkItem, +} from "./inbox"; +import { INBOX_ARCHIVE_CONFIRMATION_GRACE_MS } from "./inboxArchiveCache"; +import { + captureInboxOrderPin, + reconcileInboxOrderPin, +} from "./inboxOrderPin"; + +function issue(id: string, title = id): Issue { + return { + id, + title, + status: "todo", + } as Issue; +} + +function issueItem(value: Issue, timestamp = 0): InboxWorkItem { + return { kind: "issue", issue: value, timestamp }; +} + +function section( + key: string, + items: Issue[], + childrenByIssueId: ReadonlyArray = [], +): InboxGroupedSection { + return { + key, + label: key, + displayItems: items.map((item) => issueItem(item)), + childrenByIssueId: new Map(childrenByIssueId), + searchSection: "none", + }; +} + +function sectionKeys(sections: readonly InboxGroupedSection[]): string[] { + return sections.map((value) => value.key); +} + +function workItemSection(key: string, displayItems: InboxWorkItem[]): InboxGroupedSection { + return { + key, + label: key, + displayItems, + childrenByIssueId: new Map(), + searchSection: "none", + }; +} + +function rowIds(value: InboxGroupedSection): string[] { + return value.displayItems.map((item) => item.kind === "issue" ? item.issue.id : "non-issue"); +} + +describe("inboxOrderPin", () => { + it("keeps a parent, its group, and neighboring groups in place when a child is archived", () => { + const parent = issue("parent"); + const child = issue("child"); + const neighbor = issue("neighbor"); + const committed = [ + section("parent-group", [parent], [[parent.id, [child]]]), + section("neighbor-group", [neighbor]), + ]; + const pin = captureInboxOrderPin(committed); + + const fresh = [ + section("neighbor-group", [neighbor]), + section("parent-group", [parent], [[parent.id, []]]), + ]; + const result = reconcileInboxOrderPin(pin, fresh, 100); + + expect(sectionKeys(result.sections)).toEqual(["parent-group", "neighbor-group"]); + expect(rowIds(result.sections[0]!)).toEqual([parent.id]); + expect(result.sections[0]!.childrenByIssueId.get(parent.id)).toEqual([]); + }); + + it("removes a group in place when its last row is archived", () => { + const archived = issue("archived"); + const neighbor = issue("neighbor"); + const pin = captureInboxOrderPin([ + section("archived-group", [archived]), + section("neighbor-group", [neighbor]), + ]); + + const result = reconcileInboxOrderPin( + pin, + [section("neighbor-group", [neighbor])], + 200, + ); + + expect(sectionKeys(result.sections)).toEqual(["neighbor-group"]); + expect(rowIds(result.sections[0]!)).toEqual([neighbor.id]); + expect(result.pin.sections.map((value) => [value.key, value.missingSince])).toEqual([ + ["archived-group", 200], + ["neighbor-group", null], + ]); + }); + + it("restores an archived row to its original position within the tombstone grace window", () => { + const first = issue("first"); + const restored = issue("restored"); + const last = issue("last"); + const pin = captureInboxOrderPin([section("group", [first, restored, last])]); + const archived = reconcileInboxOrderPin(pin, [section("group", [last, first])], 1_000); + + expect(rowIds(archived.sections[0]!)).toEqual([first.id, last.id]); + + const undone = reconcileInboxOrderPin( + archived.pin, + [section("group", [last, restored, first])], + 1_000 + INBOX_ARCHIVE_CONFIRMATION_GRACE_MS - 1, + ); + + expect(rowIds(undone.sections[0]!)).toEqual([first.id, restored.id, last.id]); + expect(undone.pin.sections[0]!.rows.every((entry) => entry.missingSince === null)).toBe(true); + }); + + it("treats a row restored after the grace window as a new algorithmic insertion", () => { + const first = issue("first"); + const expired = issue("expired"); + const last = issue("last"); + const pin = captureInboxOrderPin([section("group", [first, expired, last])]); + const archived = reconcileInboxOrderPin(pin, [section("group", [last, first])], 1_000); + + const restored = reconcileInboxOrderPin( + archived.pin, + [section("group", [last, first, expired])], + 1_000 + INBOX_ARCHIVE_CONFIRMATION_GRACE_MS, + ); + + expect(rowIds(restored.sections[0]!)).toEqual([first.id, last.id, expired.id]); + }); + + it("inserts new rows and groups algorithmically without reordering existing entries", () => { + const first = issue("first"); + const last = issue("last"); + const inserted = issue("inserted"); + const neighbor = issue("neighbor"); + const newGroupItem = issue("new-group-item"); + const pin = captureInboxOrderPin([ + section("existing", [first, last]), + section("neighbor", [neighbor]), + ]); + + const result = reconcileInboxOrderPin( + pin, + [ + section("neighbor", [neighbor]), + section("new-group", [newGroupItem]), + section("existing", [last, inserted, first]), + ], + 300, + ); + + expect(sectionKeys(result.sections)).toEqual(["existing", "new-group", "neighbor"]); + expect(rowIds(result.sections[0]!)).toEqual([first.id, inserted.id, last.id]); + }); + + it("uses fresh row content while retaining the pinned order", () => { + const first = issue("first", "Old first"); + const last = issue("last", "Old last"); + const pin = captureInboxOrderPin([section("group", [first, last])]); + const freshLast = { ...last, title: "Updated last", status: "in_progress" } as Issue; + const freshFirst = { ...first, title: "Updated first", status: "blocked" } as Issue; + + const result = reconcileInboxOrderPin( + pin, + [section("group", [freshLast, freshFirst])], + 400, + ); + + expect(rowIds(result.sections[0]!)).toEqual([first.id, last.id]); + expect(result.sections[0]!.displayItems[0]).toBeDefined(); + expect(result.sections[0]!.displayItems[0]!.kind).toBe("issue"); + expect(result.sections[0]!.displayItems[0]).toMatchObject({ + issue: { title: "Updated first", status: "blocked" }, + }); + expect(result.sections[0]!.displayItems[1]).toMatchObject({ + issue: { title: "Updated last", status: "in_progress" }, + }); + }); + + it("pins approval, failed-run, and join-request rows by their stable keys", () => { + const approval = { + kind: "approval", + timestamp: 3, + approval: { id: "approval", status: "pending" } as Approval, + } satisfies InboxWorkItem; + const failedRun = { + kind: "failed_run", + timestamp: 2, + run: { id: "run", status: "failed" } as HeartbeatRun, + } satisfies InboxWorkItem; + const joinRequest = { + kind: "join_request", + timestamp: 1, + joinRequest: { id: "join", status: "pending_approval" } as JoinRequest, + } satisfies InboxWorkItem; + const pin = captureInboxOrderPin([ + workItemSection("mixed", [approval, failedRun, joinRequest]), + ]); + + const result = reconcileInboxOrderPin( + pin, + [workItemSection("mixed", [ + { ...joinRequest, timestamp: 4 }, + { ...failedRun, timestamp: 5 }, + { + ...approval, + timestamp: 6, + approval: { ...approval.approval, status: "approved" } as Approval, + }, + ])], + 500, + ); + + expect(result.sections[0]!.displayItems.map(getInboxWorkItemKey)).toEqual([ + "approval:approval", + "run:run", + "join:join", + ]); + expect(result.sections[0]!.displayItems[0]).toMatchObject({ + timestamp: 6, + approval: { status: "approved" }, + }); + }); +}); diff --git a/ui/src/lib/inboxOrderPin.ts b/ui/src/lib/inboxOrderPin.ts new file mode 100644 index 0000000000..db62c80515 --- /dev/null +++ b/ui/src/lib/inboxOrderPin.ts @@ -0,0 +1,242 @@ +import type { Issue } from "@paperclipai/shared"; +import { + getInboxWorkItemKey, + type InboxGroupedSection, + type InboxWorkItem, +} from "./inbox"; +import { INBOX_ARCHIVE_CONFIRMATION_GRACE_MS } from "./inboxArchiveCache"; + +export interface InboxPinnedKey { + key: string; + missingSince: number | null; +} + +export interface InboxPinnedSectionOrder extends InboxPinnedKey { + rows: readonly InboxPinnedKey[]; + childrenByIssueId: ReadonlyMap; +} + +export interface InboxOrderPin { + sections: readonly InboxPinnedSectionOrder[]; +} + +export interface InboxOrderPinReconcileResult { + sections: InboxGroupedSection[]; + pin: InboxOrderPin; +} + +interface ReconciledKeys { + entries: InboxPinnedKey[]; + retainedPinnedKeys: ReadonlySet; + visibleKeys: string[]; +} + +function inboxOrderRowKey(item: InboxWorkItem): string { + return item.kind === "issue" ? item.issue.id : getInboxWorkItemKey(item); +} + +function captureKeys(keys: Iterable): InboxPinnedKey[] { + return Array.from(keys, (key) => ({ key, missingSince: null })); +} + +function captureSection(section: InboxGroupedSection): InboxPinnedSectionOrder { + return { + key: section.key, + missingSince: null, + rows: captureKeys(section.displayItems.map(inboxOrderRowKey)), + childrenByIssueId: new Map( + Array.from(section.childrenByIssueId, ([issueId, children]) => [ + issueId, + captureKeys(children.map((child) => child.id)), + ]), + ), + }; +} + +export function captureInboxOrderPin( + sections: readonly InboxGroupedSection[], +): InboxOrderPin { + return { sections: sections.map(captureSection) }; +} + +function insertionIndexForVisiblePosition( + entries: readonly InboxPinnedKey[], + visibleKeys: ReadonlySet, + visibleIndex: number, +): number { + let seenVisible = 0; + for (let index = 0; index < entries.length; index += 1) { + if (seenVisible === visibleIndex) return index; + if (visibleKeys.has(entries[index]!.key)) seenVisible += 1; + } + return entries.length; +} + +function reconcileKeys( + pinned: readonly InboxPinnedKey[], + freshKeys: readonly string[], + nowMs: number, +): ReconciledKeys { + const freshKeySet = new Set(freshKeys); + const entries = pinned.flatMap((entry): InboxPinnedKey[] => { + if (freshKeySet.has(entry.key)) { + return entry.missingSince === null + || nowMs - entry.missingSince < INBOX_ARCHIVE_CONFIRMATION_GRACE_MS + ? [{ key: entry.key, missingSince: null }] + : []; + } + + const missingSince = entry.missingSince ?? nowMs; + return nowMs - missingSince < INBOX_ARCHIVE_CONFIRMATION_GRACE_MS + ? [{ key: entry.key, missingSince }] + : []; + }); + const retainedPinnedKeys = new Set(entries.map((entry) => entry.key)); + + const visibleKeys = new Set( + entries + .filter((entry) => entry.missingSince === null) + .map((entry) => entry.key), + ); + + freshKeys.forEach((key, freshIndex) => { + if (retainedPinnedKeys.has(key)) return; + const insertionIndex = insertionIndexForVisiblePosition(entries, visibleKeys, freshIndex); + entries.splice(insertionIndex, 0, { key, missingSince: null }); + visibleKeys.add(key); + }); + + return { + entries, + retainedPinnedKeys, + visibleKeys: entries + .filter((entry) => entry.missingSince === null) + .map((entry) => entry.key), + }; +} + +function reconcileChildren( + pinned: ReadonlyMap, + fresh: ReadonlyMap, + nowMs: number, +): { + pin: ReadonlyMap; + display: Map; +} { + const parentIssueIds = new Set([...pinned.keys(), ...fresh.keys()]); + const nextPin = new Map(); + const display = new Map(); + + for (const parentIssueId of parentIssueIds) { + const freshChildren = fresh.get(parentIssueId) ?? []; + const freshChildById = new Map(freshChildren.map((child) => [child.id, child])); + const reconciled = reconcileKeys( + pinned.get(parentIssueId) ?? [], + freshChildren.map((child) => child.id), + nowMs, + ); + + if (reconciled.entries.length > 0) { + nextPin.set(parentIssueId, reconciled.entries); + } + if (fresh.has(parentIssueId)) { + display.set( + parentIssueId, + reconciled.visibleKeys.flatMap((key) => { + const child = freshChildById.get(key); + return child ? [child] : []; + }), + ); + } + } + + return { pin: nextPin, display }; +} + +function reconcileSection( + pinned: InboxPinnedSectionOrder | undefined, + fresh: InboxGroupedSection, + nowMs: number, +): { pin: InboxPinnedSectionOrder; display: InboxGroupedSection } { + if (!pinned) { + return { pin: captureSection(fresh), display: fresh }; + } + + const freshItemByKey = new Map( + fresh.displayItems.map((item) => [inboxOrderRowKey(item), item]), + ); + const rows = reconcileKeys( + pinned.rows, + fresh.displayItems.map(inboxOrderRowKey), + nowMs, + ); + const children = reconcileChildren(pinned.childrenByIssueId, fresh.childrenByIssueId, nowMs); + + return { + pin: { + key: fresh.key, + missingSince: null, + rows: rows.entries, + childrenByIssueId: children.pin, + }, + display: { + ...fresh, + displayItems: rows.visibleKeys.flatMap((key) => { + const item = freshItemByKey.get(key); + return item ? [item] : []; + }), + childrenByIssueId: children.display, + }, + }; +} + +/** + * Applies a previously committed Inbox order to freshly computed sections. + * + * `nowMs` is explicit so reconciliation remains pure. Callers should retain the + * returned pin between reconciliations and replace it with `captureInboxOrderPin` + * at an attention/refresh commit point. + */ +export function reconcileInboxOrderPin( + pinned: InboxOrderPin, + freshSections: readonly InboxGroupedSection[], + nowMs: number, +): InboxOrderPinReconcileResult { + const freshSectionByKey = new Map(freshSections.map((section) => [section.key, section])); + const sections = reconcileKeys( + pinned.sections, + freshSections.map((section) => section.key), + nowMs, + ); + const pinnedSectionByKey = new Map(pinned.sections.map((section) => [section.key, section])); + const reconciledByKey = new Map>(); + + for (const freshSection of freshSections) { + reconciledByKey.set( + freshSection.key, + reconcileSection( + sections.retainedPinnedKeys.has(freshSection.key) + ? pinnedSectionByKey.get(freshSection.key) + : undefined, + freshSection, + nowMs, + ), + ); + } + + const nextPinnedSections = sections.entries.flatMap((entry): InboxPinnedSectionOrder[] => { + const reconciled = reconciledByKey.get(entry.key); + if (reconciled) return [{ ...reconciled.pin, missingSince: entry.missingSince }]; + const previous = pinnedSectionByKey.get(entry.key); + return previous ? [{ ...previous, missingSince: entry.missingSince }] : []; + }); + + return { + sections: sections.visibleKeys.flatMap((key) => { + const freshSection = freshSectionByKey.get(key); + if (!freshSection) return []; + return [reconciledByKey.get(key)?.display ?? freshSection]; + }), + pin: { sections: nextPinnedSections }, + }; +} diff --git a/ui/src/pages/Inbox.test.tsx b/ui/src/pages/Inbox.test.tsx index 48a3deccea..69518d147f 100644 --- a/ui/src/pages/Inbox.test.tsx +++ b/ui/src/pages/Inbox.test.tsx @@ -631,6 +631,107 @@ describe("Inbox toolbar", () => { } }); + it("holds the inbox order across a reordering poll, then re-sorts at an attention boundary (PAP-16015)", async () => { + routerMock.location.pathname = "/inbox/mine"; + const base = new Date("2026-03-11T00:00:00.000Z").getTime(); + const issueA = createIssue({ + id: "issue-a", + identifier: "PAP-3001", + title: "Pin row A", + lastActivityAt: new Date(base + 3000), + }); + const issueB = createIssue({ + id: "issue-b", + identifier: "PAP-3002", + title: "Pin row B", + lastActivityAt: new Date(base + 2000), + }); + const issueC = createIssue({ + id: "issue-c", + identifier: "PAP-3003", + title: "Pin row C", + lastActivityAt: new Date(base + 1000), + }); + apiMocks.issuesList.mockResolvedValue([issueA, issueB, issueC]); + + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false, staleTime: 0, gcTime: 0 } }, + }); + const root = createRoot(container); + + // Collapse each displayed row to its A/B/C identity so we can assert order. + const orderOf = () => + [...container.querySelectorAll("[data-inbox-item]")].flatMap((row) => { + const text = row.textContent ?? ""; + if (text.includes("Pin row A")) return ["A"]; + if (text.includes("Pin row B")) return ["B"]; + if (text.includes("Pin row C")) return ["C"]; + return []; + }); + + const visibilityDescriptor = Object.getOwnPropertyDescriptor(document, "visibilityState"); + const setVisibility = (state: DocumentVisibilityState) => { + Object.defineProperty(document, "visibilityState", { configurable: true, get: () => state }); + }; + let nowValue = base + 1_000_000; + const nowSpy = vi.spyOn(Date, "now").mockImplementation(() => nowValue); + + try { + await act(async () => { + root.render( + + + , + ); + }); + await vi.waitFor(() => { + expect(container.querySelectorAll("[data-inbox-item]").length).toBeGreaterThanOrEqual(3); + }); + expect(orderOf()).toEqual(["A", "B", "C"]); + + // A poll makes row C the most-recently-active: the fresh sort is now [C, A, B]. + apiMocks.issuesList.mockResolvedValue([ + { ...issueA }, + { ...issueB }, + { ...issueC, lastActivityAt: new Date(base + 9000) }, + ]); + await act(async () => { + await queryClient.invalidateQueries(); + }); + await vi.waitFor(() => { + expect(container.textContent).toContain("Pin row C"); + }); + + // No attention boundary has fired, so the displayed order is held, not reshuffled. + expect(orderOf()).toEqual(["A", "B", "C"]); + + // The tab is hidden long enough to lose attention, then regains focus: that + // visibility boundary is a commit point, so the inbox adopts the fresh order. + await act(async () => { + setVisibility("hidden"); + document.dispatchEvent(new Event("visibilitychange")); + }); + nowValue += 31_000; + await act(async () => { + setVisibility("visible"); + document.dispatchEvent(new Event("visibilitychange")); + }); + await vi.waitFor(() => { + expect(orderOf()).toEqual(["C", "A", "B"]); + }); + } finally { + nowSpy.mockRestore(); + if (visibilityDescriptor) { + Object.defineProperty(document, "visibilityState", visibilityDescriptor); + } else { + setVisibility("visible"); + } + act(() => { + root.unmount(); + }); + } + }); + it("keeps other issue archive controls enabled while one archive is pending", async () => { routerMock.location.pathname = "/inbox/mine"; const issueA = createIssue({ id: "issue-a", identifier: "PAP-1001", title: "First inbox row" }); diff --git a/ui/src/pages/Inbox.tsx b/ui/src/pages/Inbox.tsx index b50aace18d..2cfa3569c3 100644 --- a/ui/src/pages/Inbox.tsx +++ b/ui/src/pages/Inbox.tsx @@ -173,6 +173,12 @@ import { type InboxWorkItemGroupBy, } from "../lib/inbox"; import { useDismissedInboxAlerts, useInboxDismissals, useReadInboxItems } from "../hooks/useInboxBadge"; +import { useInboxSortAttention } from "../hooks/useInboxSortAttention"; +import { + captureInboxOrderPin, + reconcileInboxOrderPin, + type InboxOrderPin, +} from "../lib/inboxOrderPin"; const INBOX_HEARTBEAT_RUN_LIMIT = 200; const INBOX_ISSUE_LIST_LIMIT = 500; @@ -1379,7 +1385,7 @@ export function Inbox() { return next; }); }, [selectedCompanyId]); - const groupedSections = useMemo(() => [ + const freshGroupedSections = useMemo(() => [ ...buildGroupedInboxSections(filteredWorkItems, groupBy, inboxWorkspaceGrouping, { nestingEnabled }), ...buildGroupedInboxSections( getInboxWorkItems({ issues: archivedSearchIssues, approvals: [] }), @@ -1402,6 +1408,79 @@ export function Inbox() { nestingEnabled, ]); + // --- Order pinning (PAP-16015) --- + // The freshly computed sort is only *displayed* at attention boundaries. Between + // them we reconcile the fresh sections against the last committed order so that + // archiving mid-engagement (which drops rows from the caches, and can lower a + // parent's subtree-max sort ts) can no longer reshuffle the list under the + // cursor. New items still land at their algorithmic position; archived rows hold + // their slot for the 5s undo grace before collapsing. See `inboxOrderPin.ts`. + const orderPinRef = useRef({ sections: [] }); + // Bumped by an attention commit to force `groupedSections` to adopt the fresh order. + const [orderCommitToken, setOrderCommitToken] = useState(0); + const orderCommitRequestedRef = useRef(false); + const commitInboxOrder = useCallback(() => { + orderCommitRequestedRef.current = true; + setOrderCommitToken((token) => token + 1); + }, []); + + // Distinct view = distinct pin. Switching tab/filter/search/grouping resets the + // held order so we never carry one view's layout into another. + const inboxSortViewIdentity = useMemo( + () => + JSON.stringify([ + selectedCompanyId, + tab, + groupBy, + nestingEnabled, + normalizedSearchQuery, + allCategoryFilter, + allApprovalFilter, + issueFilters, + ]), + [ + allApprovalFilter, + allCategoryFilter, + groupBy, + issueFilters, + nestingEnabled, + normalizedSearchQuery, + selectedCompanyId, + tab, + ], + ); + // Adopt the fresh order in the SAME render the view identity changes, so a + // section key shared between the two views never briefly shows the previous + // view's order before the hook's passive commit effect runs. + const previousInboxSortViewIdentityRef = useRef(inboxSortViewIdentity); + if (previousInboxSortViewIdentityRef.current !== inboxSortViewIdentity) { + previousInboxSortViewIdentityRef.current = inboxSortViewIdentity; + orderCommitRequestedRef.current = true; + } + + const groupedSections = useMemo(() => { + const nowMs = Date.now(); + // An attention boundary (or a view change) fired: adopt the fresh order + // wholesale and re-pin it. + if (orderCommitRequestedRef.current) { + orderCommitRequestedRef.current = false; + orderPinRef.current = captureInboxOrderPin(freshGroupedSections); + return freshGroupedSections; + } + const { sections, pin } = reconcileInboxOrderPin(orderPinRef.current, freshGroupedSections, nowMs); + orderPinRef.current = pin; + return sections; + // orderCommitToken forces re-adoption at a commit boundary; inboxSortViewIdentity + // forces it on a view change even when the fresh sections keep their identity. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [freshGroupedSections, inboxSortViewIdentity, orderCommitToken]); + + const inboxSortAttention = useInboxSortAttention({ + viewIdentity: inboxSortViewIdentity, + onCommit: commitInboxOrder, + }); + const noteInboxSortInteraction = inboxSortAttention.noteInteraction; + const openCreateIssueForGroup = useCallback((group: InboxGroupedSection) => { const defaults = buildInboxIssueGroupCreateDefaults( group.key, @@ -1654,13 +1733,14 @@ export function Inbox() { const hoveredNavKeyRef = useRef(null); const setSelectedIndexFromPointer = useCallback((idx: number) => { if (!pointerMovedSinceKeyNavRef.current) return; + noteInboxSortInteraction(); hoveredIndexRef.current = idx; hoveredNavKeyRef.current = navEntryKey(flatNavItemsRef.current[idx]); // Drop any keyboard selection band the moment the mouse takes over, so we // never show two identical highlights at once. React bails out when the // value is already -1, so continuous hovering triggers no re-render. setSelectedIndex((prev) => (prev < 0 ? prev : -1)); - }, []); + }, [noteInboxSortInteraction]); const invalidateInboxIssueQueryCaches = () => { if (!selectedCompanyId) return; @@ -1670,6 +1750,8 @@ export function Inbox() { const archiveIssueMutation = useMutation({ mutationFn: (id: string) => issuesApi.archiveFromInbox(id), onMutate: async (id) => { + // Keep the sort pinned: archiving is engagement, so defer any idle re-sort. + noteInboxSortInteraction(); setActionError(null); setArchivingIssueIds((prev) => new Set(prev).add(id)); @@ -1810,6 +1892,7 @@ export function Inbox() { }, [markItemRead]); const handleArchiveNonIssue = useCallback((key: string) => { + noteInboxSortInteraction(); setArchivingNonIssueIds((prev) => new Set(prev).add(key)); setTimeout(() => { if (key.startsWith("alert:")) { @@ -1823,7 +1906,7 @@ export function Inbox() { return next; }); }, 200); - }, [dismissAlert, dismissInboxItem]); + }, [dismissAlert, dismissInboxItem, noteInboxSortInteraction]); const nonIssueUnreadState = (key: string): NonIssueUnreadState => { if (!canArchiveFromTab) return null; @@ -1973,6 +2056,9 @@ export function Inbox() { const navCount = navItems.length; if (navCount === 0) return; + // Any inbox keystroke (nav, archive, undo) is engagement: hold the sort. + noteInboxSortInteraction(); + /** Resolve the nav entry at an index to an issue (for child entries) or work item. */ const resolveNavEntry = (idx: number): { issue?: Issue; item?: InboxWorkItem } => { const entry = navItems[idx]; @@ -2115,7 +2201,7 @@ export function Inbox() { }; window.addEventListener("keydown", handleKeyDown); return () => window.removeEventListener("keydown", handleKeyDown); - }, [issueLinkState, keyboardShortcutsEnabled]); + }, [issueLinkState, keyboardShortcutsEnabled, noteInboxSortInteraction]); // Scroll selected item into view useEffect(() => { @@ -2544,7 +2630,12 @@ export function Inbox() { <> {showSeparatorBefore("work_items") && }
-
+
{(() => { const renderInboxIssue = ({ issue,