Skip to content

Commit 98376b9

Browse files
fix: report snapshot timing diagnostics (#798)
* fix: report snapshot timing diagnostics * fix: refine snapshot diagnostics reporting * fix: preserve replay snapshot diagnostics on failure * refactor: simplify snapshot diagnostics plumbing --------- Co-authored-by: Michał Pierzchała <thymikee@gmail.com>
1 parent c4950a9 commit 98376b9

18 files changed

Lines changed: 681 additions & 6 deletions

src/__tests__/client-shared.test.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,3 +151,31 @@ test('serializeSnapshotResult maps capture quality annotation to public snapshot
151151
snapshotQuality,
152152
});
153153
});
154+
155+
test('serializeSnapshotResult includes snapshot diagnostics', () => {
156+
const snapshotDiagnostics = {
157+
stats: {
158+
count: 3,
159+
p50Ms: 450,
160+
p95Ms: 1_800,
161+
maxMs: 1_800,
162+
slowThresholdMs: 1_500,
163+
platform: 'android',
164+
},
165+
warning: 'Warning: android snapshots are slow in this run: p95 1800ms over 3 captures.',
166+
} as const;
167+
const data = serializeSnapshotResult({
168+
nodes: [],
169+
truncated: false,
170+
snapshotDiagnostics,
171+
identifiers: {
172+
session: 'qa',
173+
},
174+
});
175+
176+
assert.deepEqual(data, {
177+
nodes: [],
178+
truncated: false,
179+
snapshotDiagnostics,
180+
});
181+
});
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
import { expect, test } from 'vitest';
2+
import {
3+
mergeSnapshotDiagnostics,
4+
recordSnapshotTiming,
5+
summarizeSnapshotDiagnostics,
6+
} from '../snapshot-diagnostics.ts';
7+
8+
test('records session snapshot timing stats', () => {
9+
const session = {};
10+
11+
recordSnapshotTiming(session, { durationMs: 400, backend: 'android', platform: 'android' });
12+
recordSnapshotTiming(session, { durationMs: 2_100, backend: 'android', platform: 'android' });
13+
14+
expect(summarizeSnapshotDiagnostics(session)).toEqual({
15+
stats: {
16+
count: 2,
17+
p50Ms: 400,
18+
p95Ms: 2_100,
19+
maxMs: 2_100,
20+
slowThresholdMs: 1_500,
21+
platform: 'android',
22+
backends: { android: 2 },
23+
},
24+
warning: expect.stringContaining('p95 2100ms over 2 captures'),
25+
});
26+
});
27+
28+
test('merges snapshot diagnostics without inflating capture count', () => {
29+
const merged = mergeSnapshotDiagnostics([
30+
{
31+
stats: {
32+
count: 1,
33+
p50Ms: 300,
34+
p95Ms: 300,
35+
maxMs: 300,
36+
slowThresholdMs: 1_500,
37+
platform: 'android',
38+
},
39+
},
40+
{
41+
stats: {
42+
count: 2,
43+
p50Ms: 500,
44+
p95Ms: 1_900,
45+
maxMs: 1_900,
46+
slowThresholdMs: 1_500,
47+
platform: 'android',
48+
},
49+
},
50+
]);
51+
52+
expect(merged?.stats).toMatchObject({
53+
count: 3,
54+
p50Ms: 500,
55+
p95Ms: 1_900,
56+
maxMs: 1_900,
57+
platform: 'android',
58+
});
59+
expect(merged?.warning).toContain('p95 1900ms over 3 captures');
60+
});

src/backend.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import type { ClickButton } from './core/click-button.ts';
1515
import type { DeviceRotation } from './core/device-rotation.ts';
1616
import type { ScrollDirection } from './core/scroll-gesture.ts';
1717
import type { SessionSurface } from './core/session-surface.ts';
18+
import type { SnapshotDiagnosticsSummary } from './snapshot-diagnostics.ts';
1819
import type {
1920
SnapshotCaptureAnalysis,
2021
SnapshotCaptureAnnotations,
@@ -49,6 +50,7 @@ export type BackendSnapshotResult = {
4950
snapshot?: SnapshotState;
5051
appName?: string;
5152
appBundleId?: string;
53+
snapshotDiagnostics?: SnapshotDiagnosticsSummary;
5254
} & SnapshotCaptureAnnotations;
5355

5456
export type BackendSnapshotOptions = SnapshotOptions & {

src/client-shared.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,7 @@ export function serializeSnapshotResult(result: CaptureSnapshotResult): Record<s
178178
...(result.visibility ? { visibility: result.visibility } : {}),
179179
...publicSnapshotCaptureAnnotations(snapshotResultAnnotations(result)),
180180
...(result.unchanged ? { unchanged: result.unchanged } : {}),
181+
...(result.snapshotDiagnostics ? { snapshotDiagnostics: result.snapshotDiagnostics } : {}),
181182
};
182183
}
183184

src/client-types.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import type { PublicSnapshotCaptureAnnotations } from './snapshot-capture-annotations.ts';
2+
import type { SnapshotDiagnosticsSummary } from './snapshot-diagnostics.ts';
23
import type {
34
DaemonResponseData,
45
DaemonInstallSource,
@@ -336,6 +337,7 @@ export type CaptureSnapshotResult = {
336337
appBundleId?: string;
337338
visibility?: SnapshotVisibility;
338339
unchanged?: SnapshotUnchanged;
340+
snapshotDiagnostics?: SnapshotDiagnosticsSummary;
339341
identifiers: AgentDeviceIdentifiers;
340342
} & PublicSnapshotCaptureAnnotations;
341343

src/client.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ import type {
4343
MetroPrepareOptions,
4444
} from './client-types.ts';
4545
import { readSerializedSnapshotCaptureAnnotations } from './snapshot-capture-annotations.ts';
46+
import { readSnapshotDiagnosticsSummary } from './snapshot-diagnostics.ts';
4647

4748
export function createAgentDeviceClient(
4849
config: AgentDeviceClientConfig = {},
@@ -343,15 +344,22 @@ function optionalSnapshotResponseFields(
343344
): Partial<
344345
Pick<
345346
CaptureSnapshotResult,
346-
'androidSnapshot' | 'unchanged' | 'visibility' | 'warnings' | 'snapshotQuality'
347+
| 'androidSnapshot'
348+
| 'unchanged'
349+
| 'visibility'
350+
| 'warnings'
351+
| 'snapshotQuality'
352+
| 'snapshotDiagnostics'
347353
>
348354
> {
349355
const visibility = readObject(data.visibility);
350356
const unchanged = readObject(data.unchanged);
357+
const snapshotDiagnostics = readSnapshotDiagnosticsSummary(data.snapshotDiagnostics);
351358
return {
352359
...(visibility ? { visibility: visibility as CaptureSnapshotResult['visibility'] } : {}),
353360
...readSerializedSnapshotCaptureAnnotations(data),
354361
...(unchanged ? { unchanged: unchanged as CaptureSnapshotResult['unchanged'] } : {}),
362+
...(snapshotDiagnostics ? { snapshotDiagnostics } : {}),
355363
};
356364
}
357365

src/commands/capture/index.test.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
waitCliReader,
1313
waitDaemonWriter,
1414
} from './index.ts';
15+
import { snapshotCliOutput } from './output.ts';
1516

1617
function flags(overrides: Partial<CliFlags> = {}): CliFlags {
1718
return overrides as CliFlags;
@@ -50,6 +51,32 @@ describe('capture command interface', () => {
5051
});
5152
});
5253

54+
test('routes snapshot diagnostics warning to stderr output', () => {
55+
const output = snapshotCliOutput({
56+
result: {
57+
nodes: [],
58+
truncated: false,
59+
identifiers: {},
60+
snapshotDiagnostics: {
61+
stats: {
62+
count: 2,
63+
p50Ms: 400,
64+
p95Ms: 1_900,
65+
maxMs: 1_900,
66+
slowThresholdMs: 1_500,
67+
platform: 'ios',
68+
},
69+
warning: 'Warning: ios snapshots are slow in this run: p95 1900ms over 2 captures.',
70+
},
71+
},
72+
});
73+
74+
expect(output.stderr).toBe(
75+
'Warning: ios snapshots are slow in this run: p95 1900ms over 2 captures.\n',
76+
);
77+
expect(output.text).not.toContain('snapshots are slow');
78+
});
79+
5380
test('reads screenshot path and writes screenshot flags', () => {
5481
const input = screenshotCliReader(
5582
['page.png'],

src/commands/capture/output.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@ export function snapshotCliOutput(params: {
1616
data,
1717
// Programmatic SDK callers can see `unchanged`; CLI --json hides it for schema compatibility.
1818
jsonData: withoutUnchanged(data),
19+
stderr: params.result.snapshotDiagnostics?.warning
20+
? `${params.result.snapshotDiagnostics.warning}\n`
21+
: undefined,
1922
text: formatSnapshotText(data, {
2023
raw: params.raw,
2124
flatten: params.interactiveOnly,

src/commands/capture/runtime/snapshot.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import type { BackendSnapshotResult } from '../../../backend.ts';
2+
import type { SnapshotDiagnosticsSummary } from '../../../snapshot-diagnostics.ts';
23
import type { AgentDeviceRuntime, CommandSessionRecord } from '../../../runtime-contract.ts';
34
import {
45
publicSnapshotCaptureAnnotations,
@@ -38,6 +39,7 @@ export type SnapshotCommandResult = {
3839
appBundleId?: string;
3940
visibility?: SnapshotVisibility;
4041
unchanged?: SnapshotUnchanged;
42+
snapshotDiagnostics?: SnapshotDiagnosticsSummary;
4143
} & PublicSnapshotCaptureAnnotations;
4244

4345
export type DiffSnapshotCommandResult = {
@@ -84,6 +86,9 @@ export const snapshotCommand: RuntimeCommand<
8486
warnings: capture.warnings,
8587
}),
8688
...(unchanged ? { unchanged } : {}),
89+
...(capture.result.snapshotDiagnostics
90+
? { snapshotDiagnostics: capture.result.snapshotDiagnostics }
91+
: {}),
8792
...snapshotAppFields(capture),
8893
};
8994
};

src/daemon/handlers/__tests__/session-replay-vars.test.ts

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { runCmdBackground, type ExecBackgroundResult } from '../../../utils/exec
88
import type { DaemonInvokeFn, DaemonRequest, DaemonResponse, SessionAction } from '../../types.ts';
99
import type { CommandFlags } from '../../../core/dispatch.ts';
1010
import { SessionStore } from '../../session-store.ts';
11+
import { makeIosSession } from '../../../__tests__/test-utils/index.ts';
1112
import {
1213
buildReplayVarScope,
1314
collectReplayShellEnv,
@@ -475,6 +476,110 @@ test('runReplayScriptFile dispatches resolved literals with file env overridden
475476
}
476477
});
477478

479+
test('runReplayScriptFile reports snapshot diagnostics from per-action session samples', async () => {
480+
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-replay-snapshot-samples-'));
481+
const scriptPath = path.join(root, 'flow.ad');
482+
fs.writeFileSync(scriptPath, ['snapshot', 'snapshot', ''].join('\n'));
483+
const sessionStore = new SessionStore(path.join(root, 'state'));
484+
sessionStore.set(
485+
's',
486+
makeIosSession('s', {
487+
snapshotDiagnostics: { samples: [] },
488+
}),
489+
);
490+
let captures = 0;
491+
492+
const response = await runReplayScriptFile({
493+
req: {
494+
token: 't',
495+
session: 's',
496+
command: 'replay',
497+
positionals: [scriptPath],
498+
meta: { cwd: root },
499+
},
500+
sessionName: 's',
501+
logPath: path.join(root, 'log'),
502+
sessionStore,
503+
invoke: async (): Promise<DaemonResponse> => {
504+
captures += 1;
505+
const session = sessionStore.get('s');
506+
session?.snapshotDiagnostics?.samples.push({
507+
durationMs: captures === 1 ? 400 : 1_900,
508+
backend: 'xctest',
509+
platform: 'ios',
510+
});
511+
return {
512+
ok: true,
513+
data: {
514+
snapshotDiagnostics: {
515+
stats: {
516+
count: captures,
517+
p50Ms: captures === 1 ? 400 : 1_900,
518+
p95Ms: captures === 1 ? 400 : 1_900,
519+
maxMs: captures === 1 ? 400 : 1_900,
520+
slowThresholdMs: 1_500,
521+
platform: 'ios',
522+
},
523+
},
524+
},
525+
};
526+
},
527+
});
528+
529+
assert.equal(response.ok, true);
530+
const diagnostics = response.data?.snapshotDiagnostics as
531+
| { stats?: { count?: number }; warning?: string }
532+
| undefined;
533+
assert.equal(diagnostics?.stats?.count, 2);
534+
assert.match(String(diagnostics?.warning), /p95 1900ms over 2 captures/);
535+
});
536+
537+
test('runReplayScriptFile reports snapshot diagnostics on replay failure', async () => {
538+
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-replay-snapshot-failure-'));
539+
const scriptPath = path.join(root, 'flow.ad');
540+
fs.writeFileSync(scriptPath, ['snapshot', 'click "Missing"', ''].join('\n'));
541+
const sessionStore = new SessionStore(path.join(root, 'state'));
542+
sessionStore.set(
543+
's',
544+
makeIosSession('s', {
545+
snapshotDiagnostics: { samples: [] },
546+
}),
547+
);
548+
let captures = 0;
549+
550+
const response = await runReplayScriptFile({
551+
req: {
552+
token: 't',
553+
session: 's',
554+
command: 'replay',
555+
positionals: [scriptPath],
556+
meta: { cwd: root },
557+
},
558+
sessionName: 's',
559+
logPath: path.join(root, 'log'),
560+
sessionStore,
561+
invoke: async (): Promise<DaemonResponse> => {
562+
captures += 1;
563+
const session = sessionStore.get('s');
564+
session?.snapshotDiagnostics?.samples.push({
565+
durationMs: captures === 1 ? 450 : 2_100,
566+
backend: 'xctest',
567+
platform: 'ios',
568+
});
569+
if (captures === 1) return { ok: true, data: {} };
570+
return { ok: false, error: { code: 'COMMAND_FAILED', message: 'button missing' } };
571+
},
572+
});
573+
574+
assert.equal(response.ok, false);
575+
const diagnostics = response.error.details?.snapshotDiagnostics as
576+
| { stats?: { count?: number; p95Ms?: number }; warning?: string }
577+
| undefined;
578+
assert.equal(diagnostics?.stats?.count, 2);
579+
assert.equal(diagnostics?.stats?.p95Ms, 2_100);
580+
assert.match(String(diagnostics?.warning), /p95 2100ms over 2 captures/);
581+
});
582+
478583
test('runReplayScriptFile applies CLI env overrides before Maestro compat mapping', async () => {
479584
const { response, calls } = await runReplayFixture({
480585
label: 'maestro-env',

0 commit comments

Comments
 (0)