Skip to content

Commit 6a6b28b

Browse files
authored
✨ Resolve screenshot SHAs before byte upload (#299)
## Why Screenshot upload is one of Vizzly's hottest SDK paths. When a build captures thousands of screenshots, sending image bytes for files we already have in storage is wasted work for the CLI, API, and object storage. The paired Vizzly API change lets the screenshot create endpoint accept `sha256` without `image_data`, resolve reusable storage server-side, and only ask for bytes when the image is missing. This CLI change moves the per-screenshot upload path to that contract. ## Approach - Compute the screenshot SHA before upload and first call the screenshot create endpoint with name, metadata, and SHA only. - Skip the byte upload when the API returns `upload_required: false`, preserving the existing skipped/fromExisting result shape for callers. - Fall back to the normal byte upload path when SHA resolution is unavailable, returns a server error, or talks to an older server that still requires `image_data`. - Continue surfacing auth and ownership failures instead of retrying them as byte uploads, so the fallback does not hide real access-control errors. - Document that skipped uploads mean Vizzly reused existing storage, and that `--upload-all` still forces screenshots through. This keeps the CLI simple: no SDK-side batching, no client-side cache, and no extra state to recover from. The API stays authoritative about whether a screenshot can reuse existing storage. ## Confidence The API payload helper now has a pure test proving SHA resolution omits image bytes. The upload endpoint tests cover resolve misses, resolve hits, old-server fallback, auth failure, and `--upload-all`. The full CLI test suite and Biome lint pass locally. ## Related - Coordinates with `vizzly-testing/vizzly#543`, which adds the server-side create-with-SHA behavior.
1 parent db168b2 commit 6a6b28b

7 files changed

Lines changed: 178 additions & 40 deletions

File tree

README.md

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -176,9 +176,11 @@ vizzly upload ./screenshots --threshold 2 --min-cluster-size 4 --batch-size 10 -
176176
`--upload-timeout` controls the upload client's timeout, including how long
177177
`--wait` polls for build processing.
178178

179-
CI workflows can force every screenshot through even when the SHA cache says it
180-
already uploaded. Parallel jobs should use the same stable ID, then finalize
181-
that ID after every shard completes:
179+
When `vizzly run` sends screenshots to Vizzly, the CLI first asks the API to
180+
resolve each screenshot by SHA. If Vizzly already has the image bytes, the CLI
181+
creates the screenshot record without re-uploading the image. CI workflows can
182+
force every screenshot through with `--upload-all`. Parallel jobs should use the
183+
same stable ID, then finalize that ID after every shard completes:
182184

183185
```bash
184186
vizzly run "pnpm test" --upload-all --parallel-id "$GITHUB_RUN_ID"

docs/json-output.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -759,6 +759,10 @@ vizzly upload ./screenshots --json
759759
}
760760
```
761761

762+
`stats.skipped` counts screenshots that Vizzly resolved by SHA without sending
763+
image bytes. Use `--upload-all` when you need every screenshot uploaded
764+
regardless of existing storage.
765+
762766
With `--wait`, the payload includes comparison results after Vizzly finishes
763767
processing the build:
764768

src/api/core.js

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,9 @@ export function buildRequestHeaders({
7070
// ============================================================================
7171

7272
/**
73-
* Build payload for screenshot upload
73+
* Build payload for screenshot upload or SHA resolution
7474
* @param {string} name - Screenshot name
75-
* @param {Buffer} buffer - Image data
75+
* @param {Buffer|null} buffer - Image data, or null when resolving by SHA
7676
* @param {Object} metadata - Screenshot metadata (viewport, browser, etc.)
7777
* @param {string|null} sha256 - Pre-computed SHA256 hash (optional)
7878
* @returns {Object} Screenshot upload payload
@@ -85,17 +85,31 @@ export function buildScreenshotPayload(
8585
) {
8686
let payload = {
8787
name,
88-
image_data: buffer.toString('base64'),
8988
properties: metadata ?? {},
9089
};
9190

91+
if (buffer) {
92+
payload.image_data = buffer.toString('base64');
93+
}
94+
9295
if (sha256) {
9396
payload.sha256 = sha256;
9497
}
9598

9699
return payload;
97100
}
98101

102+
/**
103+
* Build payload for server-side screenshot resolution without sending image bytes
104+
* @param {string} name - Screenshot name
105+
* @param {Object} metadata - Screenshot metadata (viewport, browser, etc.)
106+
* @param {string} sha256 - Pre-computed SHA256 hash
107+
* @returns {Object} Screenshot resolution payload
108+
*/
109+
export function buildScreenshotResolvePayload(name, metadata = {}, sha256) {
110+
return buildScreenshotPayload(name, null, metadata, sha256);
111+
}
112+
99113
/**
100114
* Build payload for build creation
101115
* @param {Object} options - Build options

src/api/endpoints.js

Lines changed: 43 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,10 @@ import {
1111
buildBuildPayload,
1212
buildEndpointWithParams,
1313
buildQueryParams,
14-
buildScreenshotCheckObject,
1514
buildScreenshotPayload,
15+
buildScreenshotResolvePayload,
1616
buildShaCheckPayload,
1717
computeSha256,
18-
findScreenshotBySha,
19-
shaExists,
2018
} from './core.js';
2119

2220
// ============================================================================
@@ -156,6 +154,18 @@ export async function checkShas(client, screenshots, buildId) {
156154
}
157155
}
158156

157+
function canUploadBytesAfterShaResolveError(error) {
158+
let status = error?.context?.status;
159+
160+
// Network failures, older servers, and server errors should degrade to the
161+
// slower byte upload path. Auth and ownership errors must still surface.
162+
if (!status) return true;
163+
if (status >= 500) return true;
164+
if (status === 400 && error.message?.includes('image_data')) return true;
165+
166+
return false;
167+
}
168+
159169
/**
160170
* Upload a screenshot with SHA deduplication
161171
* @param {Object} client - API client
@@ -184,24 +194,39 @@ export async function uploadScreenshot(
184194
});
185195
}
186196

187-
// Normal flow with SHA deduplication
197+
// Normal flow with server-side SHA resolution.
188198
let sha256 = computeSha256(buffer);
189-
let checkObj = buildScreenshotCheckObject(sha256, name, metadata);
190-
let checkResult = await checkShas(client, [checkObj], buildId);
191-
192-
if (shaExists(checkResult, sha256)) {
193-
// File already exists, screenshot record was automatically created
194-
let screenshot = findScreenshotBySha(checkResult, sha256);
195-
return {
196-
message: 'Screenshot already exists, skipped upload',
197-
sha256,
198-
skipped: true,
199-
screenshot,
200-
fromExisting: true,
201-
};
199+
let resolvePayload = buildScreenshotResolvePayload(name, metadata, sha256);
200+
try {
201+
let resolveResult = await client.request(
202+
`/api/sdk/builds/${buildId}/screenshots`,
203+
{
204+
method: 'POST',
205+
headers: { 'Content-Type': 'application/json' },
206+
body: JSON.stringify(resolvePayload),
207+
}
208+
);
209+
210+
if (resolveResult?.upload_required === false) {
211+
return {
212+
message: 'Screenshot already exists, skipped upload',
213+
sha256,
214+
skipped: true,
215+
screenshot: resolveResult,
216+
fromExisting: true,
217+
};
218+
}
219+
} catch (error) {
220+
if (!canUploadBytesAfterShaResolveError(error)) {
221+
throw error;
222+
}
223+
224+
output.debug('sha-resolve', 'failed, uploading screenshot bytes', {
225+
error: error.message,
226+
});
202227
}
203228

204-
// File doesn't exist, proceed with upload
229+
// File doesn't exist, or resolve failed; proceed with upload.
205230
let payload = buildScreenshotPayload(name, buffer, metadata, sha256);
206231
return client.request(`/api/sdk/builds/${buildId}/screenshots`, {
207232
method: 'POST',

src/api/index.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ export {
2222
buildRequestHeaders,
2323
buildScreenshotCheckObject,
2424
buildScreenshotPayload,
25+
buildScreenshotResolvePayload,
2526
buildShaCheckPayload,
2627
buildUserAgent,
2728
computeSha256,

tests/api/core.test.js

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
buildRequestHeaders,
1616
buildScreenshotCheckObject,
1717
buildScreenshotPayload,
18+
buildScreenshotResolvePayload,
1819
buildShaCheckPayload,
1920
buildUserAgent,
2021
computeSha256,
@@ -279,6 +280,21 @@ describe('api/core', () => {
279280

280281
assert.deepStrictEqual(result.properties, {});
281282
});
283+
284+
it('builds SHA resolve payload without image bytes', () => {
285+
let result = buildScreenshotResolvePayload(
286+
'homepage',
287+
{ browser: 'firefox' },
288+
'abc123'
289+
);
290+
291+
assert.deepStrictEqual(result, {
292+
name: 'homepage',
293+
properties: { browser: 'firefox' },
294+
sha256: 'abc123',
295+
});
296+
assert.strictEqual(result.image_data, undefined);
297+
});
282298
});
283299

284300
describe('buildBuildPayload', () => {

tests/api/endpoints.test.js

Lines changed: 92 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,21 @@ function createMockClient(responseOrHandler = {}) {
6666
};
6767
}
6868

69+
function createSequencedClient(responses) {
70+
let responseIndex = 0;
71+
72+
return createMockClient(() => {
73+
let response = responses[responseIndex] ?? responses[responses.length - 1];
74+
responseIndex++;
75+
76+
if (response instanceof Error) {
77+
throw response;
78+
}
79+
80+
return response;
81+
});
82+
}
83+
6984
describe('api/endpoints', () => {
7085
describe('getBuild', () => {
7186
it('requests correct endpoint', async () => {
@@ -337,13 +352,11 @@ describe('api/endpoints', () => {
337352
});
338353

339354
describe('uploadScreenshot', () => {
340-
it('uploads screenshot with deduplication', async () => {
341-
let client = createMockClient(endpoint => {
342-
if (endpoint === '/api/sdk/check-shas') {
343-
return { existing: [], missing: ['someSha'], screenshots: [] };
344-
}
345-
return { id: 'screenshot-1' };
346-
});
355+
it('resolves SHA before uploading screenshot bytes', async () => {
356+
let client = createSequencedClient([
357+
{ upload_required: true },
358+
{ id: 'screenshot-1' },
359+
]);
347360

348361
let buffer = Buffer.from('fake png data');
349362
let result = await uploadScreenshot(
@@ -355,22 +368,38 @@ describe('api/endpoints', () => {
355368
);
356369

357370
assert.ok(result);
358-
// Should have called check-shas first, then upload
359371
let calls = client.getCalls();
360372
assert.strictEqual(calls.length, 2);
373+
assert.strictEqual(
374+
calls[0].endpoint,
375+
'/api/sdk/builds/build-123/screenshots'
376+
);
377+
assert.strictEqual(
378+
calls[1].endpoint,
379+
'/api/sdk/builds/build-123/screenshots'
380+
);
381+
let resolveBody = JSON.parse(calls[0].options.body);
382+
let uploadBody = JSON.parse(calls[1].options.body);
383+
let expectedSha = createHash('sha256').update(buffer).digest('hex');
384+
385+
assert.strictEqual(resolveBody.name, 'test-screenshot');
386+
assert.strictEqual(resolveBody.sha256, expectedSha);
387+
assert.deepStrictEqual(resolveBody.properties, { viewport: '1920x1080' });
388+
assert.ok(!resolveBody.image_data);
389+
assert.strictEqual(uploadBody.sha256, expectedSha);
390+
assert.ok(uploadBody.image_data);
361391
});
362392

363-
it('skips upload when SHA exists', async () => {
393+
it('skips byte upload when screenshot create resolves reusable SHA', async () => {
364394
let buffer = Buffer.from('fake png data');
365-
// Compute the actual SHA that will be generated
366395
let sha = createHash('sha256').update(buffer).digest('hex');
367396

368397
let client = createMockClient(endpoint => {
369-
if (endpoint === '/api/sdk/check-shas') {
398+
if (endpoint === '/api/sdk/builds/build-123/screenshots') {
370399
return {
371-
existing: [sha],
372-
missing: [],
373-
screenshots: [{ sha256: sha, id: 'existing-id' }],
400+
id: 'existing-id',
401+
sha256: sha,
402+
upload_required: false,
374403
};
375404
}
376405
return { id: 'screenshot-1' };
@@ -385,9 +414,56 @@ describe('api/endpoints', () => {
385414

386415
assert.strictEqual(result.skipped, true);
387416
assert.strictEqual(result.fromExisting, true);
388-
// Should only call check-shas, not upload
389417
let calls = client.getCalls();
390418
assert.strictEqual(calls.length, 1);
419+
assert.strictEqual(
420+
calls[0].endpoint,
421+
'/api/sdk/builds/build-123/screenshots'
422+
);
423+
let resolveBody = JSON.parse(calls[0].options.body);
424+
assert.strictEqual(resolveBody.sha256, sha);
425+
assert.ok(!resolveBody.image_data);
426+
});
427+
428+
it('uploads bytes when SHA resolution is unavailable', async () => {
429+
let client = createSequencedClient([
430+
new Error('old server'),
431+
{ id: 'screenshot-1' },
432+
]);
433+
434+
let buffer = Buffer.from('fake png data');
435+
let result = await uploadScreenshot(
436+
client,
437+
'build-123',
438+
'test-screenshot',
439+
buffer,
440+
{ viewport: '1920x1080' }
441+
);
442+
443+
assert.deepStrictEqual(result, { id: 'screenshot-1' });
444+
let calls = client.getCalls();
445+
assert.strictEqual(calls.length, 2);
446+
assert.ok(!JSON.parse(calls[0].options.body).image_data);
447+
assert.ok(JSON.parse(calls[1].options.body).image_data);
448+
});
449+
450+
it('does not retry byte upload when SHA resolution fails with auth errors', async () => {
451+
let authError = new Error('Invalid or expired API token');
452+
authError.context = { status: 401 };
453+
let client = createMockClient(() => {
454+
throw authError;
455+
});
456+
457+
let buffer = Buffer.from('fake png data');
458+
459+
await assert.rejects(
460+
() => uploadScreenshot(client, 'build-123', 'test-screenshot', buffer),
461+
/Invalid or expired API token/
462+
);
463+
464+
let calls = client.getCalls();
465+
assert.strictEqual(calls.length, 1);
466+
assert.ok(!JSON.parse(calls[0].options.body).image_data);
391467
});
392468

393469
it('uploads directly when skipDedup is true', async () => {
@@ -404,7 +480,7 @@ describe('api/endpoints', () => {
404480
);
405481

406482
let calls = client.getCalls();
407-
// Should only have one call (upload), not checkShas
483+
// Should only have one call (upload), not SHA resolve
408484
assert.strictEqual(calls.length, 1);
409485
assert.ok(calls[0].endpoint.includes('/screenshots'));
410486
});

0 commit comments

Comments
 (0)