Skip to content

Commit 7cfb759

Browse files
committed
Render without attribution until build resolves
Yield before the first chunk so cold start doesn't block the initial paint, and clear stale source on file switch so prior attribution doesn't flash against the new file's AST on re-navigation.
1 parent fe35743 commit 7cfb759

5 files changed

Lines changed: 83 additions & 13 deletions

File tree

claude-notes/plans/2026-04-15-node-attribution.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ Minor tweaks to fix build issues under hub-client's stricter TypeScript config:
118118

119119
8. **Run-length encoding as the default producer** (`3265de11`) — Automerge history is replayed into a sorted `AttributionRun[]` rather than a per-char `CharAttribution[]`. Realistic batched workloads see 4× faster updates, 5× faster queries, 20× smaller storage, and arbitrary-size bulk inserts don't need the splice-chunking workaround. Numbers below.
120120

121-
9. **Cold-start latency tuning** (`d880d643`) — Two small, unambiguous changes to `buildRunListAttribution` / `waitForIdle`: (a) skip the `await waitForIdle()` before the *first* chunk so attribution work begins immediately, and (b) pass `{ timeout: 100 }` to `requestIdleCallback` so the build isn't starved when React is still mounting. A progressive-rendering variant was tried and reverted — partial runs describe an intermediate text state whose byte positions don't align with the current AST, so intermediate paints could briefly show attribution against the wrong bytes.
121+
9. **Cold-start latency tuning** — The hook returns `null` until the build resolves, so the document renders with no attribution visible and re-renders once the map is ready. Given that, `buildRunListAttribution` now yields via `waitForIdle()` before *every* chunk including the first, letting the initial paint settle before attribution CPU work begins. `waitForIdle` still passes `{ timeout: 100 }` so subsequent chunks aren't starved during sustained activity. An earlier iteration (`d880d643`) skipped the first yield to reduce time-to-attribution, but that made the first chunk block the post-mount paint — reverted in favour of prioritizing initial load over fast attribution. A progressive-rendering variant was tried and reverted — partial runs describe an intermediate text state whose byte positions don't align with the current AST, so intermediate paints could briefly show attribution against the wrong bytes.
122122

123123
## Performance results
124124

hub-client/src/hooks/useAttribution.test.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,42 @@ describe('useAttribution', () => {
225225
expect(result.current!.source).toBeDefined();
226226
});
227227

228+
it('clears stale source when filePath changes, before new build resolves', async () => {
229+
const firstMap = createMockMap();
230+
const handle = { __mock: true };
231+
mockGetFileHandle.mockReturnValue(handle as any);
232+
233+
let resolveSecond: (map: RunListAttribution | null) => void;
234+
mockBuildRunListAttribution
235+
.mockResolvedValueOnce(firstMap)
236+
.mockReturnValueOnce(
237+
new Promise(resolve => { resolveSecond = resolve as any; })
238+
);
239+
240+
const { result, rerender } = renderHook(
241+
({ path }) => useAttribution(path, 'hello'),
242+
{ initialProps: { path: 'file1.qmd' } },
243+
);
244+
245+
// Let the first build complete
246+
await act(async () => {});
247+
expect(result.current).not.toBeNull();
248+
249+
// Switch to a different file — the second build is still pending
250+
rerender({ path: 'file2.qmd' });
251+
252+
// The stale source from file1 must NOT be visible while file2 is building.
253+
// Otherwise the AST of file2 would render briefly attributed against
254+
// file1's runs/byteToChar map — visibly jarring on re-navigation.
255+
expect(result.current).toBeNull();
256+
257+
// Complete the second build — attribution reappears.
258+
await act(async () => {
259+
resolveSecond!(createMockMap({ processedHistoryIndex: 2 }));
260+
});
261+
expect(result.current).not.toBeNull();
262+
});
263+
228264
it('aborts in-flight build on unmount', async () => {
229265
const handle = { __mock: true };
230266
mockGetFileHandle.mockReturnValue(handle as any);

hub-client/src/hooks/useAttribution.ts

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -74,12 +74,15 @@ export function useAttribution(
7474
const controller = new AbortController();
7575
abortRef.current = controller;
7676

77+
// Clear any stale attribution up-front. Without this, a previous file's
78+
// `source` (with its own byteToChar map baked in) briefly renders
79+
// against the new file's AST on re-navigation, which flashes colors on
80+
// the first block before the new build lands.
81+
setResult(null);
82+
mapRef.current = null;
83+
7784
const handle = getFileHandle(path);
78-
if (!handle) {
79-
setResult(null);
80-
mapRef.current = null;
81-
return;
82-
}
85+
if (!handle) return;
8386

8487
buildRunListAttribution(handle, 'text', controller.signal).then(map => {
8588
if (controller.signal.aborted) return;

hub-client/src/services/attribution-runs.test.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,38 @@ describe('buildRunListAttribution', () => {
242242
expect(result!.runs).toHaveLength(1);
243243
expect(result!.runs[0]).toEqual({ start: 0, end: N, actor: 'a1', time: 1000 });
244244
});
245+
246+
it('yields to the event loop before touching history', async () => {
247+
// The hook renders the document without attribution during the build;
248+
// the build must yield first so the initial paint isn't blocked by
249+
// the first chunk's CPU work.
250+
const entries: MockHistoryEntry[] = [{
251+
heads: ['h1'], actor: 'a1', time: 1000,
252+
patches: [{ action: 'splice', path: ['text', 0], value: 'x' }],
253+
}];
254+
const handle = createMockHandle(entries);
255+
mockDiff.mockImplementation(createMockDiff(entries));
256+
257+
let idleCalls = 0;
258+
let metadataCalledBeforeIdle = false;
259+
const originalMetadata = handle.metadata;
260+
handle.metadata = vi.fn((hash: string) => {
261+
if (idleCalls === 0) metadataCalledBeforeIdle = true;
262+
return originalMetadata(hash);
263+
});
264+
265+
// @ts-expect-error -- mock global
266+
globalThis.requestIdleCallback = (cb: IdleRequestCallback) => {
267+
idleCalls++;
268+
cb({ didTimeout: false, timeRemaining: () => 50 } as IdleDeadline);
269+
return idleCalls;
270+
};
271+
272+
await buildRunListAttribution(handle as never, 'text');
273+
274+
expect(idleCalls).toBeGreaterThan(0);
275+
expect(metadataCalledBeforeIdle).toBe(false);
276+
});
245277
});
246278

247279
describe('updateRunListAttribution', () => {

hub-client/src/services/attribution-runs.ts

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -201,13 +201,12 @@ export async function buildRunListAttribution(
201201
let lastHeads: unknown[] = [];
202202

203203
for (let chunkStart = 0; chunkStart < history.length; chunkStart += CHUNK_SIZE) {
204-
// Skip the yield before the first chunk — the main thread is usually
205-
// already busy at cold-start and yielding immediately only adds latency
206-
// to time-to-first-paint.
207-
if (chunkStart > 0) {
208-
await waitForIdle();
209-
if (signal?.aborted) return null;
210-
}
204+
// Yield before every chunk, including the first. The hook returns null
205+
// until the build resolves, so nothing about attribution is visible to
206+
// the user yet — we can afford to let the document's initial paint
207+
// complete on an idle tick before starting CPU work.
208+
await waitForIdle();
209+
if (signal?.aborted) return null;
211210

212211
const chunkEnd = Math.min(chunkStart + CHUNK_SIZE, history.length);
213212
for (let i = chunkStart; i < chunkEnd; i++) {

0 commit comments

Comments
 (0)