mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-12 22:56:58 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The board UI keeps its caches fresh from live events in `LiveUpdatesProvider` > - Each task activity event invalidates the whole `["issues", companyId]` query prefix > - That prefix also holds the `@task` mention pool: the 100 most recently updated tasks, about 315 KB per response > - A chat turn emits many comment, status and run events, so an open task page downloads that list again for almost every event > - This pull request keeps the pool fresh from push events without a refetch per event: a closed server flag marks real membership or title changes, progress on pooled tasks reorders the cache in place, and other progress refetches at most once per 30 seconds > - The benefit is less bandwidth and CPU for users with an open task or chat, and smaller eval traces ## Linked Issues or Issue Description **What happened** While a task chat page is open, the browser repeats `GET /api/companies/:id/issues?limit=100&sortField=updated&sortDir=desc` (about 315 KB each). One 10-minute chat-eval attempt recorded about 194 of these requests. The trigger is `invalidateActivityQueries` in `ui/src/context/LiveUpdatesProvider.tsx`. For every `entityType: "issue"` activity event, it invalidates `queryKeys.issues.list(companyId)`. That prefix matches `queryKeys.issues.mentionPool(companyId)`, which `IssueDetail` (and `NewIssueDialog`) keep active for the `@task` picker. Invalidation refetches active queries and ignores `staleTime`. **Expected behavior** Comments, run progress and status changes on any task do not download the full mention pool again. The pool still updates when a task that the picker can show is created, deleted or renamed. **Steps to reproduce** 1. Run `pnpm dev` and create a company with about 140 tasks. 2. Open a task page, for example `/REF/issues/REF-1`. 3. Every 4 seconds for 15 rounds, post comments on the open task, on a task in the top 100, and on a task outside the top 100, and change the open task's priority (60 events). 4. Count requests to `issues?limit=100&sortField=updated`. Before this change there are 15 (about 3.3 MB). After this change there are 2 (about 480 KB). ## What Changed - New shared helper `issueActivityChangesTaskIdentity` (`packages/shared/src/issue-activity-identity.ts`). It is true for a created, deleted, hidden, unhidden or renamed task. - `server/src/realtime/live-events-ws.ts`: task `activity.logged` events now carry the closed boolean `taskIdentityChanged`. Authenticated sockets still get no `details`; the flag carries no task content. - `ui/src/context/LiveUpdatesProvider.tsx` refreshes the mention pool this way: - The task identity flag is true (or full details show a create, delete, hide, unhide or rename): the pool refetches at once. - Other events on a task already in the pool: the task moves to the front of the cached list. There is no request. The freshness clock does not change, and a pending invalidation is never overwritten. - Other events on a task outside the pool: one trailing refetch at most every 30 seconds, because the task can now be in the top 100. - The picker is closed: the pool is only marked stale. - `ui/src/lib/query-invalidation-batcher.ts`: new `excludingQueryKeys()` keyed exclusion predicate. The batcher coalesces it and uses it for prefix coverage. The broad `["issues", companyId]` invalidation uses it to skip the pool, so a burst is again one broad refresh per batch. - Tests: `ui/src/context/LiveUpdatesProvider.mention-pool.test.ts` (authenticated event shape, hide and unhide, the 30-second window, reorder safety, a burst through the batcher), new batcher tests, `packages/shared/src/issue-activity-identity.test.ts`, and socket assertions in `server/src/__tests__/live-events-toast.test.ts`. ## Verification - `cd ui && npx vitest run src/context/LiveUpdatesProvider src/lib/query-invalidation-batcher`: 231 tests pass. - `cd server && npx vitest run src/__tests__/live-events-toast.test.ts`: 11 tests pass. In authenticated mode the socket keeps `details` off and sends `taskIdentityChanged` (false for routine updates, true for a create, rename or hide). - `cd packages/shared && npx vitest run src/issue-activity-identity.test.ts`: 14 tests pass. - The new UI tests fail against the first commit of this PR: the authenticated burst, hide, unhide, the 30-second window and the batcher burst. - `tsc --noEmit` for ui, server and shared: no new errors. - Browser measurement: a local `pnpm dev` instance (local trusted), headless Chromium on a task page, 141 tasks, 60 events over about 63 seconds (the reproduction steps above): - Before (master UI): **15** mention-pool requests, about 3.3 MB. - After: **2** requests, about 480 KB (one per 30-second window). - Freshness after the fix: one rename, hide, unhide or create each caused **1** refetch. A priority change caused **0**. - I did not run a browser session in authenticated mode. The authenticated event shape is covered by the socket test and the UI unit tests. ## Risks - A task that moves into the top 100 can be missing from the `@task` picker for up to 30 seconds. - The cached order is approximate between refetches: a moved task goes to the front, but its `updatedAt` value is not changed. - Other queries under `["issues", companyId]` (descendants, created-from, summaries, onboarding first task) still refresh for each event batch. They are small, but they can get the same treatment later. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), with tool use and code execution in Claude Code. ## 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 - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
322 lines
13 KiB
TypeScript
322 lines
13 KiB
TypeScript
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
|
import { QueryClient, QueryObserver } from "@tanstack/react-query";
|
|
import {
|
|
createCoalescingQueryClient,
|
|
createInvalidationBatcher,
|
|
excludingQueryKeys,
|
|
} from "./query-invalidation-batcher";
|
|
|
|
function fakeClient() {
|
|
const calls: unknown[] = [];
|
|
const client = {
|
|
invalidateQueries: vi.fn((filters?: unknown) => {
|
|
calls.push(filters);
|
|
return Promise.resolve();
|
|
}),
|
|
};
|
|
return { client, calls };
|
|
}
|
|
|
|
describe("createInvalidationBatcher", () => {
|
|
beforeEach(() => vi.useFakeTimers());
|
|
afterEach(() => { vi.useRealTimers(); vi.unstubAllGlobals(); });
|
|
|
|
it("marks queries stale without refetching when the tab hides before flush", async () => {
|
|
const { client } = fakeClient();
|
|
const batcher = createInvalidationBatcher(client);
|
|
const pending = batcher.schedule({ queryKey: ["dashboard", "c1"] });
|
|
vi.stubGlobal("document", { visibilityState: "hidden" });
|
|
await batcher.flush();
|
|
await pending;
|
|
expect(client.invalidateQueries).toHaveBeenCalledExactlyOnceWith({ queryKey: ["dashboard", "c1"], refetchType: "none" });
|
|
});
|
|
|
|
it("coalesces repeated invalidations of the same key into one call per window", () => {
|
|
const { client } = fakeClient();
|
|
const batcher = createInvalidationBatcher(client, 300);
|
|
|
|
for (let i = 0; i < 20; i++) batcher.schedule({ queryKey: ["dashboard", "c1"] });
|
|
expect(client.invalidateQueries).not.toHaveBeenCalled(); // nothing until flush
|
|
|
|
vi.advanceTimersByTime(300);
|
|
expect(client.invalidateQueries).toHaveBeenCalledTimes(1);
|
|
expect(client.invalidateQueries).toHaveBeenCalledWith({ queryKey: ["dashboard", "c1"] });
|
|
});
|
|
|
|
it("keeps distinct keys and distinct refetchType variants separate", () => {
|
|
const { client } = fakeClient();
|
|
const batcher = createInvalidationBatcher(client, 300);
|
|
|
|
batcher.schedule({ queryKey: ["a"] });
|
|
batcher.schedule({ queryKey: ["b"] });
|
|
batcher.schedule({ queryKey: ["a"] }); // dup of first
|
|
batcher.schedule({ queryKey: ["a"], refetchType: "inactive" }); // distinct variant
|
|
|
|
vi.advanceTimersByTime(300);
|
|
expect(client.invalidateQueries).toHaveBeenCalledTimes(3);
|
|
});
|
|
|
|
it("never coalesces predicate-based invalidations", () => {
|
|
const { client } = fakeClient();
|
|
const batcher = createInvalidationBatcher(client, 300);
|
|
|
|
// Two distinct predicate filters must both run — they can't be proven equal.
|
|
batcher.schedule({ predicate: () => true });
|
|
batcher.schedule({ predicate: () => false });
|
|
|
|
vi.advanceTimersByTime(300);
|
|
expect(client.invalidateQueries).toHaveBeenCalledTimes(2);
|
|
});
|
|
|
|
it("schedule() resolves only after the flush has invalidated", async () => {
|
|
const { client } = fakeClient();
|
|
const batcher = createInvalidationBatcher(client, 300);
|
|
|
|
let resolved = false;
|
|
const p = batcher.schedule({ queryKey: ["a"] }).then(() => {
|
|
resolved = true;
|
|
});
|
|
|
|
await Promise.resolve();
|
|
expect(resolved).toBe(false); // not yet — window still open
|
|
expect(client.invalidateQueries).not.toHaveBeenCalled();
|
|
|
|
vi.advanceTimersByTime(300);
|
|
await p;
|
|
expect(resolved).toBe(true);
|
|
expect(client.invalidateQueries).toHaveBeenCalledTimes(1);
|
|
});
|
|
|
|
it("starts a fresh window after flushing", () => {
|
|
const { client } = fakeClient();
|
|
const batcher = createInvalidationBatcher(client, 300);
|
|
|
|
batcher.schedule({ queryKey: ["a"] });
|
|
vi.advanceTimersByTime(300);
|
|
expect(client.invalidateQueries).toHaveBeenCalledTimes(1);
|
|
|
|
batcher.schedule({ queryKey: ["a"] });
|
|
vi.advanceTimersByTime(300);
|
|
expect(client.invalidateQueries).toHaveBeenCalledTimes(2);
|
|
});
|
|
|
|
it("dispose cancels a pending flush", () => {
|
|
const { client } = fakeClient();
|
|
const batcher = createInvalidationBatcher(client, 300);
|
|
|
|
batcher.schedule({ queryKey: ["a"] });
|
|
batcher.dispose();
|
|
vi.advanceTimersByTime(1000);
|
|
expect(client.invalidateQueries).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it("flush() invalidates immediately", () => {
|
|
const { client } = fakeClient();
|
|
const batcher = createInvalidationBatcher(client, 300);
|
|
|
|
batcher.schedule({ queryKey: ["a"] });
|
|
batcher.flush();
|
|
expect(client.invalidateQueries).toHaveBeenCalledTimes(1);
|
|
});
|
|
});
|
|
|
|
describe("createCoalescingQueryClient", () => {
|
|
beforeEach(() => vi.useFakeTimers());
|
|
afterEach(() => { vi.useRealTimers(); vi.unstubAllGlobals(); });
|
|
|
|
it("batches invalidateQueries but passes other methods straight through", () => {
|
|
const setQueryData = vi.fn();
|
|
const getQueryData = vi.fn(() => ({ some: "data" }));
|
|
const realInvalidate = vi.fn(() => Promise.resolve());
|
|
const real = { invalidateQueries: realInvalidate, setQueryData, getQueryData } as unknown as QueryClient;
|
|
|
|
const batcher = createInvalidationBatcher(
|
|
{ invalidateQueries: realInvalidate } as unknown as QueryClient,
|
|
300,
|
|
);
|
|
const proxied = createCoalescingQueryClient(real, batcher);
|
|
|
|
// setQueryData / getQueryData pass through immediately.
|
|
proxied.setQueryData(["k"], 1);
|
|
expect(setQueryData).toHaveBeenCalledWith(["k"], 1);
|
|
expect(proxied.getQueryData(["k"])).toEqual({ some: "data" });
|
|
|
|
// invalidateQueries is deferred through the batcher.
|
|
void proxied.invalidateQueries({ queryKey: ["k"] });
|
|
void proxied.invalidateQueries({ queryKey: ["k"] });
|
|
expect(realInvalidate).not.toHaveBeenCalled();
|
|
|
|
vi.advanceTimersByTime(300);
|
|
expect(realInvalidate).toHaveBeenCalledTimes(1);
|
|
});
|
|
});
|
|
|
|
describe("overlapping live invalidations", () => {
|
|
it.each([false, true])("does not restart a request already covered by its collection invalidation (child first=%s)", async childFirst => {
|
|
const client = new QueryClient({ defaultOptions: { queries: { retry: false, staleTime: Infinity, gcTime: Infinity } } });
|
|
const key = ["issues", "company-1", "mine-by-me", "index"];
|
|
client.setQueryData(key, "cached");
|
|
const completions: Array<() => void> = [];
|
|
const aborted = vi.fn();
|
|
const read = vi.fn(({ signal }: { signal: AbortSignal }) => new Promise<string>((resolve, reject) => {
|
|
completions.push(() => resolve("fresh"));
|
|
signal.addEventListener("abort", () => { aborted(); reject(signal.reason); }, { once: true });
|
|
}));
|
|
const observer = new QueryObserver(client, { queryKey: key, queryFn: read });
|
|
const unsubscribe = observer.subscribe(() => undefined);
|
|
const batcher = createInvalidationBatcher(client);
|
|
try {
|
|
const filters = [{ queryKey: ["issues", "company-1"] }, { queryKey: ["issues", "company-1", "mine-by-me"] }];
|
|
for (const filter of childFirst ? filters.reverse() : filters) void batcher.schedule(filter);
|
|
const flushed = batcher.flush();
|
|
completions.forEach(complete => complete());
|
|
await flushed;
|
|
expect(client.getQueryData(key)).toBe("fresh");
|
|
expect(read).toHaveBeenCalledOnce();
|
|
expect(aborted).not.toHaveBeenCalled();
|
|
} finally {
|
|
batcher.dispose();
|
|
unsubscribe();
|
|
client.clear();
|
|
}
|
|
});
|
|
|
|
it.each(["exact", "type", "stale", "refetch", "predicate"])("preserves a child refresh when the parent has a different %s constraint", async constraint => {
|
|
const client = new QueryClient({ defaultOptions: { queries: { retry: false, staleTime: Infinity, gcTime: Infinity } } });
|
|
const key = ["issues", "company-1", "mine-by-me"];
|
|
client.setQueryData(key, "cached");
|
|
const read = vi.fn(async () => "fresh");
|
|
const observer = new QueryObserver(client, { queryKey: key, queryFn: read });
|
|
const unsubscribe = observer.subscribe(() => undefined);
|
|
const batcher = createInvalidationBatcher(client);
|
|
try {
|
|
const parent = {
|
|
queryKey: ["issues", "company-1"],
|
|
...(constraint === "exact" ? { exact: true } : {}),
|
|
...(constraint === "type" ? { type: "inactive" as const } : {}),
|
|
...(constraint === "stale" ? { stale: true } : {}),
|
|
...(constraint === "refetch" ? { refetchType: "none" as const } : {}),
|
|
...(constraint === "predicate" ? { predicate: () => false } : {}),
|
|
};
|
|
void batcher.schedule(parent);
|
|
void batcher.schedule({ queryKey: key });
|
|
await batcher.flush();
|
|
expect(client.getQueryData(key)).toBe("fresh");
|
|
expect(read).toHaveBeenCalledOnce();
|
|
} finally {
|
|
batcher.dispose();
|
|
unsubscribe();
|
|
client.clear();
|
|
}
|
|
});
|
|
|
|
it.each(["stale", "fetchStatus"])("retains distinct %s filters on the same key", async constraint => {
|
|
const client = new QueryClient({ defaultOptions: { queries: { retry: false, staleTime: Infinity, gcTime: Infinity } } });
|
|
const key = ["issues", "company-1"];
|
|
client.setQueryData(key, "cached");
|
|
const observer = new QueryObserver(client, { queryKey: key, queryFn: async () => "fresh" });
|
|
const unsubscribe = observer.subscribe(() => undefined);
|
|
const batcher = createInvalidationBatcher(client);
|
|
try {
|
|
if (constraint === "stale") {
|
|
void batcher.schedule({ queryKey: key, stale: false });
|
|
void batcher.schedule({ queryKey: key, stale: true });
|
|
} else {
|
|
void batcher.schedule({ queryKey: key, fetchStatus: "idle" });
|
|
void batcher.schedule({ queryKey: key, fetchStatus: "paused" });
|
|
}
|
|
await batcher.flush();
|
|
expect(client.getQueryData(key)).toBe("fresh");
|
|
} finally {
|
|
batcher.dispose();
|
|
unsubscribe();
|
|
client.clear();
|
|
}
|
|
});
|
|
|
|
it("starts a newer read when another event arrives in a later batch", async () => {
|
|
const client = new QueryClient({ defaultOptions: { queries: { retry: false, staleTime: Infinity, gcTime: Infinity } } });
|
|
const key = ["issues", "company-1", "mine-by-me"];
|
|
client.setQueryData(key, 0);
|
|
let serverValue = 1;
|
|
const completions: Array<() => void> = [];
|
|
const read = vi.fn(({ signal }: { signal: AbortSignal }) => new Promise<number>((resolve, reject) => {
|
|
const snapshot = serverValue;
|
|
completions.push(() => resolve(snapshot));
|
|
signal.addEventListener("abort", () => reject(signal.reason), { once: true });
|
|
}));
|
|
const observer = new QueryObserver(client, { queryKey: key, queryFn: read });
|
|
const unsubscribe = observer.subscribe(() => undefined);
|
|
const batcher = createInvalidationBatcher(client);
|
|
try {
|
|
void batcher.schedule({ queryKey: ["issues", "company-1"] });
|
|
const first = batcher.flush();
|
|
serverValue = 2;
|
|
void batcher.schedule({ queryKey: key });
|
|
const second = batcher.flush();
|
|
completions.forEach(complete => complete());
|
|
await Promise.all([first, second]);
|
|
expect(read).toHaveBeenCalledTimes(2);
|
|
expect(client.getQueryData(key)).toBe(2);
|
|
} finally {
|
|
batcher.dispose();
|
|
unsubscribe();
|
|
client.clear();
|
|
}
|
|
});
|
|
});
|
|
|
|
describe("excludingQueryKeys", () => {
|
|
beforeEach(() => vi.useFakeTimers());
|
|
afterEach(() => vi.useRealTimers());
|
|
|
|
it("coalesces a burst of keyed exclusions into one invalidation per window", () => {
|
|
const { client } = fakeClient();
|
|
const batcher = createInvalidationBatcher(client, 300);
|
|
for (let i = 0; i < 25; i += 1) {
|
|
batcher.schedule({ queryKey: ["issues", "c1"], predicate: excludingQueryKeys(["issues", "c1", "mention-pool"]) });
|
|
}
|
|
vi.advanceTimersByTime(300);
|
|
expect(client.invalidateQueries).toHaveBeenCalledTimes(1);
|
|
});
|
|
|
|
it("keeps different exclusions separate", () => {
|
|
const { client } = fakeClient();
|
|
const batcher = createInvalidationBatcher(client, 300);
|
|
batcher.schedule({ queryKey: ["issues", "c1"], predicate: excludingQueryKeys(["issues", "c1", "a"]) });
|
|
batcher.schedule({ queryKey: ["issues", "c1"], predicate: excludingQueryKeys(["issues", "c1", "b"]) });
|
|
vi.advanceTimersByTime(300);
|
|
expect(client.invalidateQueries).toHaveBeenCalledTimes(2);
|
|
});
|
|
|
|
it("covers non-excluded children but never the excluded subtree", () => {
|
|
const { client, calls } = fakeClient();
|
|
const batcher = createInvalidationBatcher(client, 300);
|
|
const broad = { queryKey: ["issues", "c1"], predicate: excludingQueryKeys(["issues", "c1", "mention-pool"]) };
|
|
batcher.schedule(broad);
|
|
batcher.schedule({ queryKey: ["issues", "c1", "search", "q"] });
|
|
batcher.schedule({ queryKey: ["issues", "c1", "mention-pool"] });
|
|
batcher.schedule({ queryKey: ["issues", "c1"] });
|
|
vi.advanceTimersByTime(300);
|
|
// The plain prefix is not covered by the exclusion filter: it also reaches the pool.
|
|
expect(calls).toEqual([{ queryKey: ["issues", "c1"] }]);
|
|
});
|
|
|
|
it("is covered by a plain shorter prefix and covers a sibling child", () => {
|
|
const { client, calls } = fakeClient();
|
|
const batcher = createInvalidationBatcher(client, 300);
|
|
const broad = { queryKey: ["issues", "c1"], predicate: excludingQueryKeys(["issues", "c1", "mention-pool"]) };
|
|
batcher.schedule(broad);
|
|
batcher.schedule({ queryKey: ["issues", "c1", "search", "q"] });
|
|
batcher.schedule({ queryKey: ["issues", "c1", "mention-pool"] });
|
|
vi.advanceTimersByTime(300);
|
|
expect(calls).toEqual([broad, { queryKey: ["issues", "c1", "mention-pool"] }]);
|
|
});
|
|
|
|
it("skips excluded queries when matching", () => {
|
|
const predicate = excludingQueryKeys(["issues", "c1", "mention-pool"]);
|
|
expect(predicate({ queryKey: ["issues", "c1", "mention-pool"] })).toBe(false);
|
|
expect(predicate({ queryKey: ["issues", "c1", "search"] })).toBe(true);
|
|
});
|
|
});
|