Skip to content

Commit 5d98966

Browse files
vincerevuquanru
andauthored
perf(report): memoize derived task and timeline data arrays (#2320)
* perf(report): memoize derived task and timeline data arrays Memoizes the `useAllCurrentTasks` global selector output to prevent unnecessary array allocations and referential instability on every render. Also memoizes the heavy reduction for `allScreenshots` and `idTaskMap` inside the `Timeline` component so it is not re-computed unless the task array changes. * refactor(report): extract pure helpers for memoized selectors and add tests Builds on the memoization in #2320 by lifting the derivation logic out of useAllCurrentTasks and Timeline into pure helpers, then covering them with unit tests so the stable-reference promise is verifiable. - flattenGroupedDumpTasks() and buildTimelineScreenshots() are now pure modules; the React hooks/components only call them inside useMemo. - buildTimelineScreenshots is rewritten as compute-startingTime then collect-then-sort, removing the closure-mutated startingTime/idCount that the original reduce relied on. Legacy starting-time semantics (recorders with no screenshot still contribute their ts) are preserved and pinned by a regression test. - idTaskMap no longer keeps dead keys for filtered-out recorder items. - 14 new vitest cases under apps/report cover null input, empty/no-screenshot tasks, ordering across tasks, id uniqueness, repeated-call equality, and the legacy starting-time edge case. - Drop .jules/bolt.md: agent scratchpad that should not live in the repo. Validation: pnpm run lint, npx nx build report, vitest run (19 passed). * test(report): add fixture-driven regression test for timeline builder Runs flattenGroupedDumpTasks + buildTimelineScreenshots against every real dump JSON under apps/report/test-data/ so future schema or refactor regressions surface immediately instead of waiting for a browser run. Each fixture is asserted to: produce an array, never emit an id that cannot be reverse-mapped, and never emit a non-finite timeOffset. --------- Co-authored-by: quanruzhuoxiu <quanruzhuoxiu@gmail.com>
1 parent 481c905 commit 5d98966

7 files changed

Lines changed: 446 additions & 57 deletions

File tree

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
import type {
2+
ExecutionDump,
3+
ExecutionTask,
4+
GroupedActionDump,
5+
} from '@midscene/core';
6+
import { describe, expect, it } from 'vitest';
7+
import { flattenGroupedDumpTasks } from './flatten-tasks';
8+
9+
const makeTask = (id: string): ExecutionTask =>
10+
({ taskId: id }) as unknown as ExecutionTask;
11+
12+
const makeExecution = (taskIds: string[]): ExecutionDump =>
13+
({
14+
name: `exec-${taskIds.join('-')}`,
15+
tasks: taskIds.map(makeTask),
16+
}) as unknown as ExecutionDump;
17+
18+
const makeDump = (executionTaskIds: string[][]): GroupedActionDump =>
19+
({
20+
groupName: 'group',
21+
groupDescription: 'desc',
22+
executions: executionTaskIds.map(makeExecution),
23+
}) as unknown as GroupedActionDump;
24+
25+
describe('flattenGroupedDumpTasks', () => {
26+
it('returns an empty array when groupedDump is null', () => {
27+
expect(flattenGroupedDumpTasks(null)).toEqual([]);
28+
});
29+
30+
it('returns an empty array when no executions are present', () => {
31+
expect(flattenGroupedDumpTasks(makeDump([]))).toEqual([]);
32+
});
33+
34+
it('flattens tasks across executions while preserving order', () => {
35+
const dump = makeDump([['a1', 'a2'], ['b1'], ['c1', 'c2', 'c3']]);
36+
37+
const result = flattenGroupedDumpTasks(dump);
38+
39+
expect(result).toHaveLength(6);
40+
expect(result.map((t) => (t as any).taskId)).toEqual([
41+
'a1',
42+
'a2',
43+
'b1',
44+
'c1',
45+
'c2',
46+
'c3',
47+
]);
48+
});
49+
50+
it('returns referentially identical task instances (no cloning)', () => {
51+
const sharedTask = makeTask('shared');
52+
const dump = {
53+
executions: [{ tasks: [sharedTask] }],
54+
} as unknown as GroupedActionDump;
55+
56+
const result = flattenGroupedDumpTasks(dump);
57+
58+
expect(result[0]).toBe(sharedTask);
59+
});
60+
61+
it('produces equal output for repeated calls so memoization is meaningful', () => {
62+
const dump = makeDump([['a', 'b'], ['c']]);
63+
64+
const first = flattenGroupedDumpTasks(dump);
65+
const second = flattenGroupedDumpTasks(dump);
66+
67+
expect(second).toEqual(first);
68+
expect(second).toHaveLength(3);
69+
});
70+
});
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
import type { ExecutionTask, GroupedActionDump } from '@midscene/core';
2+
3+
export const flattenGroupedDumpTasks = (
4+
groupedDump: GroupedActionDump | null,
5+
): ExecutionTask[] => {
6+
if (!groupedDump) return [];
7+
return groupedDump.executions.reduce<ExecutionTask[]>(
8+
(acc, execution) => acc.concat(execution.tasks),
9+
[],
10+
);
11+
};

apps/report/src/components/store/index.tsx

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,9 @@ import {
1515
extractDumpMetaInfo,
1616
generateAnimationScripts,
1717
} from '@midscene/visualizer';
18+
import { useMemo } from 'react';
1819
import * as Z from 'zustand';
20+
import { flattenGroupedDumpTasks } from './flatten-tasks';
1921

2022
const { create } = Z;
2123

@@ -247,12 +249,5 @@ export const useExecutionDump = create<DumpStoreType>((set, get) => {
247249

248250
export const useAllCurrentTasks = (): ExecutionTask[] => {
249251
const groupedDump = useExecutionDump((store) => store.dump);
250-
if (!groupedDump) return [];
251-
252-
const tasksInside = groupedDump.executions.reduce<ExecutionTask[]>(
253-
(acc2, execution) => acc2.concat(execution.tasks),
254-
[],
255-
);
256-
257-
return tasksInside;
252+
return useMemo(() => flattenGroupedDumpTasks(groupedDump), [groupedDump]);
258253
};
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
import fs from 'node:fs';
2+
import path from 'node:path';
3+
import { describe, expect, it } from 'vitest';
4+
import { flattenGroupedDumpTasks } from '../store/flatten-tasks';
5+
import { buildTimelineScreenshots } from './build-timeline-screenshots';
6+
7+
const testDataDir = path.resolve(__dirname, '../../../test-data');
8+
const fixtureFiles = fs
9+
.readdirSync(testDataDir)
10+
.filter((file) => file.endsWith('.json'));
11+
12+
describe('helpers against real test-data fixtures', () => {
13+
for (const file of fixtureFiles) {
14+
it(`processes ${file} without errors`, () => {
15+
const raw = fs.readFileSync(path.join(testDataDir, file), 'utf-8');
16+
const dump = JSON.parse(raw);
17+
18+
const allTasks = flattenGroupedDumpTasks(dump);
19+
expect(Array.isArray(allTasks)).toBe(true);
20+
21+
const result = buildTimelineScreenshots(allTasks);
22+
expect(Array.isArray(result.allScreenshots)).toBe(true);
23+
expect(result.idTaskMap).toBeTypeOf('object');
24+
expect(typeof result.startingTime).toBe('number');
25+
26+
// Every emitted id must reverse-map to a real task
27+
for (const shot of result.allScreenshots) {
28+
expect(result.idTaskMap[shot.id]).toBeDefined();
29+
}
30+
31+
// No NaN offsets — would imply startingTime mismatch
32+
for (const shot of result.allScreenshots) {
33+
expect(Number.isFinite(shot.timeOffset)).toBe(true);
34+
}
35+
});
36+
}
37+
});
Lines changed: 227 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,227 @@
1+
import type { ExecutionTask } from '@midscene/core';
2+
import { describe, expect, it } from 'vitest';
3+
import { buildTimelineScreenshots } from './build-timeline-screenshots';
4+
5+
interface TaskFixtureOptions {
6+
id: string;
7+
startTs?: number;
8+
uiContextScreenshot?: { base64: string };
9+
recorder?: { ts: number; base64?: string }[];
10+
}
11+
12+
const makeTask = (opts: TaskFixtureOptions): ExecutionTask =>
13+
({
14+
taskId: opts.id,
15+
timing: opts.startTs !== undefined ? { start: opts.startTs } : undefined,
16+
uiContext: opts.uiContextScreenshot
17+
? { screenshot: opts.uiContextScreenshot }
18+
: undefined,
19+
recorder: opts.recorder?.map((r) => ({
20+
type: 'screenshot',
21+
ts: r.ts,
22+
screenshot: r.base64 !== undefined ? { base64: r.base64 } : undefined,
23+
})),
24+
}) as unknown as ExecutionTask;
25+
26+
describe('buildTimelineScreenshots', () => {
27+
it('returns empty result for empty input', () => {
28+
const { allScreenshots, idTaskMap, startingTime } =
29+
buildTimelineScreenshots([]);
30+
expect(allScreenshots).toEqual([]);
31+
expect(idTaskMap).toEqual({});
32+
expect(startingTime).toBe(-1);
33+
});
34+
35+
it('drops recorder items that have no screenshot', () => {
36+
const task = makeTask({
37+
id: 'no-shot',
38+
startTs: 1000,
39+
recorder: [{ ts: 1100 }],
40+
});
41+
42+
const { allScreenshots } = buildTimelineScreenshots([task]);
43+
44+
expect(allScreenshots).toEqual([]);
45+
});
46+
47+
it('emits a single uiContext screenshot for a task without recorders', () => {
48+
const task = makeTask({
49+
id: 'ctx-only',
50+
startTs: 5000,
51+
uiContextScreenshot: { base64: 'CTX' },
52+
});
53+
54+
const { allScreenshots, idTaskMap, startingTime } =
55+
buildTimelineScreenshots([task]);
56+
57+
expect(startingTime).toBe(5000);
58+
expect(allScreenshots).toHaveLength(1);
59+
expect(allScreenshots[0]).toMatchObject({
60+
img: 'CTX',
61+
timeOffset: 0,
62+
});
63+
expect(idTaskMap[allScreenshots[0].id]).toBe(task);
64+
});
65+
66+
it('uses the earliest recorder ts as the starting time and computes offsets', () => {
67+
const task = makeTask({
68+
id: 'with-recorder',
69+
startTs: 2000,
70+
recorder: [
71+
{ ts: 2500, base64: 'B' },
72+
{ ts: 1800, base64: 'A' },
73+
{ ts: 3000, base64: 'C' },
74+
],
75+
});
76+
77+
const { allScreenshots, startingTime } = buildTimelineScreenshots([task]);
78+
79+
expect(startingTime).toBe(1800);
80+
expect(allScreenshots.map((s) => s.timeOffset)).toEqual([0, 700, 1200]);
81+
expect(allScreenshots.map((s) => s.img)).toEqual(['A', 'B', 'C']);
82+
});
83+
84+
it('orders screenshots from multiple tasks by absolute time', () => {
85+
const taskA = makeTask({
86+
id: 'A',
87+
startTs: 1000,
88+
recorder: [{ ts: 1500, base64: 'A1' }],
89+
});
90+
const taskB = makeTask({
91+
id: 'B',
92+
startTs: 500,
93+
recorder: [
94+
{ ts: 800, base64: 'B1' },
95+
{ ts: 2000, base64: 'B2' },
96+
],
97+
});
98+
99+
const { allScreenshots, startingTime } = buildTimelineScreenshots([
100+
taskA,
101+
taskB,
102+
]);
103+
104+
expect(startingTime).toBe(500);
105+
expect(allScreenshots.map((s) => s.img)).toEqual(['B1', 'A1', 'B2']);
106+
expect(allScreenshots.map((s) => s.timeOffset)).toEqual([300, 1000, 1500]);
107+
});
108+
109+
it('maps every emitted id back to the originating task', () => {
110+
const taskA = makeTask({
111+
id: 'A',
112+
startTs: 1000,
113+
uiContextScreenshot: { base64: 'CTX-A' },
114+
recorder: [{ ts: 1100, base64: 'A1' }],
115+
});
116+
const taskB = makeTask({
117+
id: 'B',
118+
startTs: 2000,
119+
recorder: [{ ts: 2100, base64: 'B1' }],
120+
});
121+
122+
const { allScreenshots, idTaskMap } = buildTimelineScreenshots([
123+
taskA,
124+
taskB,
125+
]);
126+
127+
const taskByImg = (img: string) =>
128+
idTaskMap[allScreenshots.find((s) => s.img === img)!.id];
129+
130+
expect(taskByImg('CTX-A')).toBe(taskA);
131+
expect(taskByImg('A1')).toBe(taskA);
132+
expect(taskByImg('B1')).toBe(taskB);
133+
});
134+
135+
it('assigns unique ids across all emitted screenshots', () => {
136+
const tasks = [
137+
makeTask({
138+
id: 'A',
139+
startTs: 100,
140+
uiContextScreenshot: { base64: 'CTX' },
141+
recorder: [
142+
{ ts: 110, base64: 'A1' },
143+
{ ts: 120, base64: 'A2' },
144+
],
145+
}),
146+
makeTask({
147+
id: 'B',
148+
startTs: 200,
149+
recorder: [{ ts: 210, base64: 'B1' }],
150+
}),
151+
];
152+
153+
const { allScreenshots } = buildTimelineScreenshots(tasks);
154+
const ids = allScreenshots.map((s) => s.id);
155+
156+
expect(new Set(ids).size).toBe(ids.length);
157+
});
158+
159+
it('produces equal output for repeated calls (allowing useMemo to short-circuit)', () => {
160+
const tasks = [
161+
makeTask({
162+
id: 'A',
163+
startTs: 1000,
164+
recorder: [{ ts: 1100, base64: 'A1' }],
165+
}),
166+
];
167+
168+
const first = buildTimelineScreenshots(tasks);
169+
const second = buildTimelineScreenshots(tasks);
170+
171+
expect(second.allScreenshots).toEqual(first.allScreenshots);
172+
expect(second.startingTime).toBe(first.startingTime);
173+
});
174+
175+
it('does not put dropped recorder items into idTaskMap', () => {
176+
const task = makeTask({
177+
id: 'mixed',
178+
startTs: 1000,
179+
recorder: [
180+
{ ts: 1100, base64: 'KEEP' },
181+
{ ts: 1200 }, // no screenshot — should be dropped from idTaskMap too
182+
],
183+
});
184+
185+
const { allScreenshots, idTaskMap } = buildTimelineScreenshots([task]);
186+
187+
expect(allScreenshots).toHaveLength(1);
188+
expect(Object.keys(idTaskMap)).toHaveLength(1);
189+
expect(idTaskMap[allScreenshots[0].id]).toBe(task);
190+
});
191+
192+
it('still uses dropped recorder ts to compute starting time', () => {
193+
// legacy behaviour: a recorder with no screenshot still contributes its ts
194+
// to startingTime so that surviving entries render at the right offset.
195+
const taskA = makeTask({
196+
id: 'A',
197+
startTs: 5000,
198+
recorder: [{ ts: 800 }], // no screenshot but earliest ts
199+
});
200+
const taskB = makeTask({
201+
id: 'B',
202+
startTs: 6000,
203+
recorder: [{ ts: 6100, base64: 'B1' }],
204+
});
205+
206+
const { allScreenshots, startingTime } = buildTimelineScreenshots([
207+
taskA,
208+
taskB,
209+
]);
210+
211+
expect(startingTime).toBe(800);
212+
expect(allScreenshots).toHaveLength(1);
213+
expect(allScreenshots[0]).toMatchObject({
214+
img: 'B1',
215+
timeOffset: 5300, // 6100 - 800
216+
});
217+
});
218+
219+
it('does not crash when a task has no timing or uiContext at all', () => {
220+
const task = makeTask({ id: 'bare' });
221+
222+
const { allScreenshots, startingTime } = buildTimelineScreenshots([task]);
223+
224+
expect(allScreenshots).toEqual([]);
225+
expect(startingTime).toBe(-1);
226+
});
227+
});

0 commit comments

Comments
 (0)