Skip to content

Commit a5a1b1b

Browse files
SawyerHoodclaude
andcommitted
Make hydrated thread bootstraps usable and harden the persisted query cache
Review fixes for the persistedQueryCache experiment: - A hydrated threadDetailBootstrap did nothing for first paint: the thread route reads the thread from the `thread`/`environment`/`host` caches, which only the live queryFn seeded, so the page still sat on "Loading…" until useThread round-tripped. Re-run ingestThreadDetailBootstrap for every bootstrap the hydrate call landed, stamped with the bootstrap's own fetch time so staleTime and the reconnect invalidation see the derived entries as exactly that old. - The persister re-read and re-parsed the whole IndexedDB blob on every debounce tick (every 1.5 s during a streaming turn). Read the store once and carry the merged snapshot forward in memory; also stop a write whose initial read was overtaken by stop(), so turning the experiment off cannot repopulate the store it just cleared. - The IndexedDB write only watched the transaction, whose abort error can be null while the put request carries the real QuotaExceededError; await the request so the quota path actually clears the store. - A blob from an older app version that crashes the tree would crash every reload for 24 h. AppErrorBoundary gains an onError hook and main.tsx clears the store on a crash that follows a hydration. - Put the experiments literals back on one key per line. Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 9a37434 commit a5a1b1b

11 files changed

Lines changed: 251 additions & 19 deletions

apps/app/src/components/AppErrorBoundary.test.tsx

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,27 @@ describe("AppErrorBoundary", () => {
5353
dispose();
5454
});
5555

56+
it("reports the caught error to onError so boot can drop a bad persisted cache", () => {
57+
vi.spyOn(console, "error").mockImplementation(() => undefined);
58+
const { container, render, dispose } = mountRoot();
59+
const onError = vi.fn();
60+
61+
function Boom(): never {
62+
throw new Error("hydrated shape mismatch");
63+
}
64+
render(
65+
<AppErrorBoundary onError={onError}>
66+
<Boom />
67+
</AppErrorBoundary>,
68+
);
69+
70+
expect(onError).toHaveBeenCalledTimes(1);
71+
expect(onError.mock.calls[0]?.[0]).toBeInstanceOf(Error);
72+
expect(onError.mock.calls[0]?.[0].message).toBe("hydrated shape mismatch");
73+
expect(container.textContent).toContain("bb hit an error and stopped");
74+
dispose();
75+
});
76+
5677
it("catches the commit-phase removeChild failure instead of blanking the root", () => {
5778
vi.spyOn(console, "error").mockImplementation(() => undefined);
5879
const { container, render, dispose } = mountRoot();

apps/app/src/components/AppErrorBoundary.tsx

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,13 @@ import { Component, type ErrorInfo, type ReactNode } from "react";
1717

1818
interface AppErrorBoundaryProps {
1919
children: ReactNode;
20+
/**
21+
* Runs once per caught error, after the console report. Must not throw and
22+
* must not depend on anything the crashed tree owned; main.tsx uses it to
23+
* drop the persisted query cache so a bad hydration cannot crash every
24+
* reload in a row.
25+
*/
26+
onError?: (error: Error) => void;
2027
}
2128

2229
interface AppErrorBoundaryState {
@@ -37,6 +44,7 @@ export class AppErrorBoundary extends Component<
3744
// The component stack is the part a screenshot of the console never has,
3845
// and the part that names the subtree at fault.
3946
console.error("[bb] the app crashed", error, info.componentStack);
47+
this.props.onError?.(error);
4048
}
4149

4250
override render(): ReactNode {

apps/app/src/components/layout/AppLayout.plugin-panel-header.test.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,8 @@ vi.mock("@/hooks/queries/system-queries", () => ({
3030
claudeCodeMockCliTraffic: false,
3131
editMessages: false,
3232
newOnboarding: false,
33-
persistedQueryCache: false, providerSessionReaping: false,
33+
persistedQueryCache: false,
34+
providerSessionReaping: false,
3435
},
3536
},
3637
}),

apps/app/src/components/layout/AppLayout.root-compose-project.test.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@ vi.mock("@/hooks/queries/system-queries", () => ({
2626
claudeCodeMockCliTraffic: false,
2727
editMessages: false,
2828
newOnboarding: false,
29-
persistedQueryCache: false, providerSessionReaping: false,
29+
persistedQueryCache: false,
30+
providerSessionReaping: false,
3031
},
3132
},
3233
}),

apps/app/src/hooks/cache-owners/thread-detail-cache-owner.ts

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,13 @@ interface UpsertHostListArgs {
2727
export interface ThreadDetailBootstrapIngestionArgs {
2828
queryClient: QueryClient;
2929
thread: ThreadWithIncludesResponse;
30+
/**
31+
* When the bootstrap did not just arrive from the network (a slice hydrated
32+
* from the persisted query cache), stamp the derived entries with the
33+
* bootstrap's own fetch time so their staleTime and the reconnect
34+
* invalidation treat them as exactly that old. Omit for a live response.
35+
*/
36+
updatedAt?: number;
3037
}
3138

3239
function stripThreadIncludes(
@@ -65,24 +72,30 @@ function upsertHostList({ host, hosts }: UpsertHostListArgs): HostList {
6572
export function ingestThreadDetailBootstrap({
6673
queryClient,
6774
thread,
75+
updatedAt,
6876
}: ThreadDetailBootstrapIngestionArgs): void {
77+
const setOptions = updatedAt === undefined ? undefined : { updatedAt };
6978
queryClient.setQueryData(
7079
threadQueryKey(thread.id),
7180
stripThreadIncludes(thread),
81+
setOptions,
7282
);
7383

7484
if (thread.environment) {
7585
queryClient.setQueryData(
7686
environmentQueryKey(thread.environment.id),
7787
thread.environment,
88+
setOptions,
7889
);
7990
}
8091

8192
if (thread.host) {
8293
const host = thread.host;
83-
queryClient.setQueryData(hostQueryKey(host.id), host);
84-
queryClient.setQueryData<HostList>(hostsQueryKey(), (hosts) =>
85-
upsertHostList({ host, hosts }),
94+
queryClient.setQueryData(hostQueryKey(host.id), host, setOptions);
95+
queryClient.setQueryData<HostList>(
96+
hostsQueryKey(),
97+
(hosts) => upsertHostList({ host, hosts }),
98+
setOptions,
8699
);
87100
}
88101

apps/app/src/lib/persisted-query-cache/persisted-query-cache-store.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,11 @@ export function createIndexedDbPersistedQueryCacheStore(): PersistedQueryCacheSt
9494
},
9595
async write(value) {
9696
await withDatabase("readwrite", async (store, transaction) => {
97-
store.put(value, IDB_SNAPSHOT_KEY);
97+
// Await the request itself, not just the transaction: a failed put
98+
// aborts the transaction, but the abort event's `error` can still be
99+
// null while the request's is the real `QuotaExceededError`, which the
100+
// persister needs to see to clear the store.
101+
await requestToPromise(store.put(value, IDB_SNAPSHOT_KEY));
98102
await transactionDone(transaction);
99103
});
100104
},

apps/app/src/lib/persisted-query-cache/persisted-query-cache.test.ts

Lines changed: 107 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,14 @@
11
import { QueryClient } from "@tanstack/react-query";
22
import { afterEach, describe, expect, it, vi } from "vitest";
33
import {
4+
environmentQueryKey,
5+
hostQueryKey,
6+
hostsQueryKey,
47
sidebarNavigationQueryKey,
58
systemConfigQueryKey,
69
threadDetailBootstrapQueryKey,
710
threadHostFilePreviewQueryKey,
11+
threadQueryKey,
812
threadsQueryKey,
913
threadTimelineQueryKey,
1014
} from "@/hooks/queries/query-keys";
@@ -213,6 +217,78 @@ describe("restorePersistedQueryCache", () => {
213217
expect(state?.dataUpdatedAt).toBe(NOW - 60_000);
214218
});
215219

220+
it("seeds the thread, environment and host caches from a hydrated bootstrap with its fetch time", async () => {
221+
const queryClient = new QueryClient();
222+
const fetchedAt = NOW - 90_000;
223+
const bootstrap = entry(threadDetailBootstrapQueryKey("thr_1"), fetchedAt, {
224+
id: "thr_1",
225+
title: "Cached",
226+
environment: { id: "env_1" },
227+
host: { id: "host_1" },
228+
});
229+
const store = createMemoryPersistedQueryCacheStore(
230+
serializePersistedQueryCache([bootstrap], fetchedAt),
231+
);
232+
233+
await restorePersistedQueryCache({ queryClient, store, now: NOW });
234+
235+
// The route reads the thread from `thread`, not from the bootstrap query;
236+
// without this seed a hydrated bootstrap still paints "Loading…".
237+
expect(queryClient.getQueryData(threadQueryKey("thr_1"))).toEqual({
238+
id: "thr_1",
239+
title: "Cached",
240+
});
241+
expect(queryClient.getQueryData(environmentQueryKey("env_1"))).toEqual({
242+
id: "env_1",
243+
});
244+
expect(queryClient.getQueryData(hostQueryKey("host_1"))).toEqual({
245+
id: "host_1",
246+
});
247+
expect(queryClient.getQueryData(hostsQueryKey())).toEqual([
248+
{ id: "host_1" },
249+
]);
250+
// Same age as the bootstrap, so staleTime and reconnect invalidation
251+
// treat the derived entries as stale too.
252+
for (const key of [
253+
threadQueryKey("thr_1"),
254+
environmentQueryKey("env_1"),
255+
hostQueryKey("host_1"),
256+
hostsQueryKey(),
257+
]) {
258+
expect(queryClient.getQueryState(key)?.dataUpdatedAt).toBe(fetchedAt);
259+
}
260+
});
261+
262+
it("leaves the derived caches alone when the live bootstrap is fresher", async () => {
263+
const queryClient = new QueryClient();
264+
seedQuery(
265+
queryClient,
266+
threadDetailBootstrapQueryKey("thr_1"),
267+
{ id: "thr_1", title: "Live", environment: null, host: null },
268+
NOW,
269+
);
270+
const store = createMemoryPersistedQueryCacheStore(
271+
serializePersistedQueryCache(
272+
[
273+
entry(threadDetailBootstrapQueryKey("thr_1"), NOW - 1, {
274+
id: "thr_1",
275+
title: "Stale",
276+
environment: { id: "env_stale" },
277+
host: null,
278+
}),
279+
],
280+
NOW - 1,
281+
),
282+
);
283+
284+
await restorePersistedQueryCache({ queryClient, store, now: NOW });
285+
286+
expect(queryClient.getQueryData(threadQueryKey("thr_1"))).toBeUndefined();
287+
expect(
288+
queryClient.getQueryData(environmentQueryKey("env_stale")),
289+
).toBeUndefined();
290+
});
291+
216292
it("does not overwrite fresher data already in the client", async () => {
217293
const queryClient = new QueryClient();
218294
seedQuery(queryClient, systemConfigQueryKey(), { v: "live" }, NOW);
@@ -345,9 +421,13 @@ describe("startPersistedQueryCachePersister", () => {
345421
collectLivePersistedQueryCacheEntries(queryClient).map((e) => e.queryKey),
346422
).toEqual([sidebarNavigationQueryKey()]);
347423

348-
// A later successful update is picked up on the next flush.
424+
// A later successful update is picked up on the next flush, the previous
425+
// snapshot still survives, and the store is not re-read for it: a
426+
// streaming turn must not parse the whole blob on every debounce tick.
427+
const read = vi.spyOn(store, "read");
349428
seedQuery(queryClient, threadTimelineQueryKey("gcd"), { rows: [] }, NOW);
350429
await persister.flush();
430+
expect(read).not.toHaveBeenCalled();
351431
expect(
352432
parsePersistedQueryCache(store.value, NOW)?.map((e) => e.queryKey),
353433
).toEqual([
@@ -358,6 +438,32 @@ describe("startPersistedQueryCachePersister", () => {
358438
persister.stop();
359439
});
360440

441+
it("does not write after stop() lands during the initial store read", async () => {
442+
const queryClient = new QueryClient();
443+
seedQuery(queryClient, sidebarNavigationQueryKey(), { s: 1 }, NOW);
444+
let resolveRead: (value: string | null) => void = () => {};
445+
const store = createMemoryPersistedQueryCacheStore();
446+
vi.spyOn(store, "read").mockImplementation(
447+
() =>
448+
new Promise((resolve) => {
449+
resolveRead = resolve;
450+
}),
451+
);
452+
const write = vi.spyOn(store, "write");
453+
const persister = startPersistedQueryCachePersister({
454+
queryClient,
455+
store,
456+
now: () => NOW,
457+
});
458+
const flushed = persister.flush();
459+
// The experiment turned off (or the app tore down) mid-read; the store
460+
// is about to be cleared and must not be repopulated by this write.
461+
persister.stop();
462+
resolveRead(null);
463+
await flushed;
464+
expect(write).not.toHaveBeenCalled();
465+
});
466+
361467
it("debounces cache updates into one write and stops after stop()", async () => {
362468
vi.useFakeTimers();
363469
const queryClient = new QueryClient();

apps/app/src/lib/persisted-query-cache/persisted-query-cache.ts

Lines changed: 58 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ import {
2626
type QueryState,
2727
} from "@tanstack/react-query";
2828
import { z } from "zod";
29+
import type { ThreadWithIncludesResponse } from "@bb/server-contract";
30+
import { ingestThreadDetailBootstrap } from "@/hooks/cache-owners/thread-detail-cache-owner";
2931
import {
3032
SIDEBAR_NAVIGATION_QUERY_KEY,
3133
SYSTEM_CONFIG_QUERY_KEY,
@@ -342,9 +344,47 @@ export async function restorePersistedQueryCache({
342344
state: hydratedQueryState(entry),
343345
})),
344346
});
347+
ingestHydratedThreadDetailBootstraps(queryClient, entries);
345348
return { status: "hydrated", entryCount: entries.length };
346349
}
347350

351+
/**
352+
* The live bootstrap query seeds the `thread`, `environment` and `host` caches
353+
* from inside its queryFn, and the thread route reads the thread from those —
354+
* a hydrated bootstrap alone would still leave the page on "Loading…" until
355+
* `useThread` round-trips. Re-run that ingestion for every bootstrap the
356+
* hydrate call actually landed (fresher live data is left alone), stamped with
357+
* the bootstrap's own fetch time so the derived entries stay exactly as stale.
358+
*/
359+
function ingestHydratedThreadDetailBootstraps(
360+
queryClient: QueryClient,
361+
entries: ReadonlyArray<PersistedQueryCacheEntry>,
362+
): void {
363+
for (const entry of entries) {
364+
if (
365+
classifyPersistedQueryKey(entry.queryKey)?.kind !==
366+
"threadDetailBootstrap"
367+
) {
368+
continue;
369+
}
370+
const state = queryClient.getQueryState<ThreadWithIncludesResponse>(
371+
entry.queryKey,
372+
);
373+
if (
374+
state === undefined ||
375+
state.data === undefined ||
376+
state.dataUpdatedAt !== entry.dataUpdatedAt
377+
) {
378+
continue;
379+
}
380+
ingestThreadDetailBootstrap({
381+
queryClient,
382+
thread: state.data,
383+
updatedAt: state.dataUpdatedAt,
384+
});
385+
}
386+
}
387+
348388
export interface RestorePersistedQueryCacheIfEnabledArgs {
349389
queryClient: QueryClient;
350390
/** The local mirror of the experiment; false skips storage entirely. */
@@ -431,6 +471,12 @@ export function startPersistedQueryCachePersister({
431471
let dirty = false;
432472
let timer: ReturnType<typeof setTimeout> | null = null;
433473
let writeChain: Promise<void> = Promise.resolve();
474+
// What the store held the last time we looked. Read from IndexedDB once
475+
// (entries hydrated on a previous launch may since have been garbage-
476+
// collected from memory and must survive), then carried forward from our
477+
// own writes so a streaming turn does not re-read and re-parse the whole
478+
// blob on every debounce tick.
479+
let previousEntries: PersistedQueryCacheEntry[] | null = null;
434480

435481
const cancelTimer = () => {
436482
if (timer !== null) {
@@ -451,14 +497,23 @@ export function startPersistedQueryCachePersister({
451497
dirty = false;
452498
const timestamp = now();
453499
try {
454-
const previous =
455-
parsePersistedQueryCache(await store.read(), timestamp) ?? [];
500+
if (previousEntries === null) {
501+
previousEntries =
502+
parsePersistedQueryCache(await store.read(), timestamp) ?? [];
503+
// stop() may have landed during the read; the caller is gone.
504+
if (disabled || stopped) return;
505+
}
456506
const selected = selectPersistedQueryCacheEntries(
457-
[...collectLivePersistedQueryCacheEntries(queryClient), ...previous],
507+
[
508+
...collectLivePersistedQueryCacheEntries(queryClient),
509+
...previousEntries,
510+
],
458511
{ now: timestamp },
459512
);
460513
await store.write(serializePersistedQueryCache(selected, timestamp));
514+
previousEntries = selected;
461515
} catch (error) {
516+
previousEntries = null;
462517
if (isQuotaExceededError(error)) {
463518
await store.clear();
464519
}

apps/app/src/lib/system-config-atoms.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,8 @@ const unavailableSystemConfig: SystemConfigResponse = {
2424
claudeCodeMockCliTraffic: false,
2525
editMessages: false,
2626
newOnboarding: false,
27-
persistedQueryCache: false, providerSessionReaping: false,
27+
persistedQueryCache: false,
28+
providerSessionReaping: false,
2829
},
2930
appearance: defaultAppTheme,
3031
customThemes: [],

0 commit comments

Comments
 (0)