From 5bc83179a6f109e6e9962d26ac519c12c3e0230f Mon Sep 17 00:00:00 2001 From: Robert DeLuca Date: Sat, 23 May 2026 13:18:15 -0500 Subject: [PATCH 1/3] =?UTF-8?q?=E2=9C=A8=20Align=20local=20TDD=20dynamic?= =?UTF-8?q?=20content=20filtering?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Match local confirmed-region filtering to cloud behavior by using center-based region matching, pixel-weighted coverage, and SSIM gates. Persist bundled and fallback dynamic metadata during baseline downloads so local TDD can use the same context as cloud review. --- src/tdd/core/region-coverage.js | 158 +++++++++++++-- src/tdd/services/comparison-service.js | 17 +- src/tdd/tdd-service.js | 118 ++++++++---- tests/tdd/core/region-coverage.test.js | 108 ++++++++--- tests/tdd/services/comparison-service.test.js | 71 +++++++ tests/tdd/tdd-service.test.js | 182 ++++++++++++++++++ 6 files changed, 569 insertions(+), 85 deletions(-) diff --git a/src/tdd/core/region-coverage.js b/src/tdd/core/region-coverage.js index eb503e71..0e56f135 100644 --- a/src/tdd/core/region-coverage.js +++ b/src/tdd/core/region-coverage.js @@ -9,6 +9,8 @@ * 2D boxes that users have manually confirmed via the cloud UI. */ +let REGION_CENTER_TOLERANCE = 10; + /** * Check if a diff cluster intersects with a region (2D box intersection) * @@ -43,12 +45,102 @@ export function clusterIntersectsRegion(cluster, region) { return !noOverlap; } +function normalizeBoundingBox(cluster) { + let box = cluster?.boundingBox || cluster; + if (!box) { + return null; + } + + if ( + Number.isFinite(box.x) && + Number.isFinite(box.y) && + Number.isFinite(box.width) && + Number.isFinite(box.height) + ) { + return { + x1: box.x, + y1: box.y, + x2: box.x + box.width, + y2: box.y + box.height, + width: Math.abs(box.width), + height: Math.abs(box.height), + }; + } + + if ( + Number.isFinite(box.x1) && + Number.isFinite(box.y1) && + Number.isFinite(box.x2) && + Number.isFinite(box.y2) + ) { + let width = Math.abs(box.x2 - box.x1); + let height = Math.abs(box.y2 - box.y1); + return { + x1: box.x1, + y1: box.y1, + x2: box.x2, + y2: box.y2, + width, + height, + }; + } + + return null; +} + +function getBoundingBoxCenter(box) { + return { + x: (box.x1 + box.x2) / 2, + y: (box.y1 + box.y2) / 2, + }; +} + +function getRegionLabel(region) { + return region.id || region.label || null; +} + +function clusterMatchesRegion(cluster, region) { + let clusterBox = normalizeBoundingBox(cluster); + let regionBox = normalizeBoundingBox(region); + + if (!clusterBox || !regionBox) { + return false; + } + + let clusterCenter = getBoundingBoxCenter(clusterBox); + let regionCenter = getBoundingBoxCenter(regionBox); + let distance = Math.sqrt( + (clusterCenter.x - regionCenter.x) ** 2 + + (clusterCenter.y - regionCenter.y) ** 2 + ); + + return distance <= REGION_CENTER_TOLERANCE; +} + +function estimateClusterPixels(cluster) { + if (Number.isFinite(cluster?.pixelCount)) { + return cluster.pixelCount; + } + + let box = normalizeBoundingBox(cluster); + if (!box) { + return 0; + } + + return box.width * box.height * 0.5; +} + /** - * Calculate what percentage of diff clusters fall within region boxes + * Calculate what percentage of changed pixels match confirmed regions. + * + * This mirrors cloud dynamic-region approval: clusters match confirmed regions + * by center proximity, then coverage is weighted by changed pixels. A huge + * unmatched change should not be hidden just because several tiny clusters + * match confirmed regions. * * @param {Array} diffClusters - Array of diff clusters from honeydiff * @param {Array} regions - Array of confirmed regions { x1, y1, x2, y2 } - * @returns {{ coverage: number, clustersInRegions: number, totalClusters: number, matchedRegions: string[] }} + * @returns {{ coverage: number, clustersInRegions: number, totalClusters: number, matchedRegions: string[], pixelsInRegions: number, totalPixels: number }} */ export function calculateRegionCoverage(diffClusters, regions) { if (!diffClusters || diffClusters.length === 0) { @@ -57,6 +149,8 @@ export function calculateRegionCoverage(diffClusters, regions) { clustersInRegions: 0, totalClusters: 0, matchedRegions: [], + pixelsInRegions: 0, + totalPixels: 0, }; } @@ -66,40 +160,48 @@ export function calculateRegionCoverage(diffClusters, regions) { clustersInRegions: 0, totalClusters: diffClusters.length, matchedRegions: [], + pixelsInRegions: 0, + totalPixels: 0, }; } let clustersInRegions = 0; let matchedRegionIds = new Set(); + let pixelsInRegions = 0; + let totalPixels = 0; for (let cluster of diffClusters) { - // Check if this cluster intersects any region - let intersectsAnyRegion = false; + let pixelCount = estimateClusterPixels(cluster); + totalPixels += pixelCount; + + let matchedAnyRegion = false; for (let region of regions) { - if (clusterIntersectsRegion(cluster, region)) { - intersectsAnyRegion = true; - // Track which regions were matched (for debugging/display) - if (region.id) { - matchedRegionIds.add(region.id); - } else if (region.label) { - matchedRegionIds.add(region.label); + if (clusterMatchesRegion(cluster, region)) { + matchedAnyRegion = true; + + let regionLabel = getRegionLabel(region); + if (regionLabel) { + matchedRegionIds.add(regionLabel); } } } - if (intersectsAnyRegion) { + if (matchedAnyRegion) { clustersInRegions++; + pixelsInRegions += pixelCount; } } - let coverage = clustersInRegions / diffClusters.length; + let coverage = totalPixels > 0 ? pixelsInRegions / totalPixels : 0; return { coverage, clustersInRegions, totalClusters: diffClusters.length, matchedRegions: [...matchedRegionIds], + pixelsInRegions, + totalPixels, }; } @@ -107,17 +209,39 @@ export function calculateRegionCoverage(diffClusters, regions) { * Determine if a comparison should auto-pass based on region coverage * * Unlike hotspots which require confidence scoring, user-defined regions - * are already confirmed by humans, so we only need the 80% threshold. + * are already confirmed by humans. Cloud still requires strong structural + * similarity so a layout shift cannot hide inside a confirmed box. * * @param {Array} regions - Confirmed regions (already filtered to confirmed status) * @param {{ coverage: number }} coverageResult - Result from calculateRegionCoverage + * @param {Object} options - Approval thresholds + * @param {number|null} options.ssimScore - Honeydiff SSIM/perceptual score + * @param {number} options.coverageThreshold - Required region coverage + * @param {number} options.ssimThreshold - Required structural similarity * @returns {boolean} True if diff should auto-pass as region-filtered */ -export function shouldAutoApproveFromRegions(regions, coverageResult) { +export function shouldAutoApproveFromRegions( + regions, + coverageResult, + options = {} +) { if (!regions || regions.length === 0 || !coverageResult) { return false; } - // Need at least 80% of diff clusters in confirmed regions - return coverageResult.coverage >= 0.8; + let { + ssimScore = null, + coverageThreshold = 0.9, + ssimThreshold = 0.95, + } = options; + + if (coverageResult.coverage < coverageThreshold) { + return false; + } + + if (ssimScore === null || ssimScore === undefined) { + return false; + } + + return ssimScore >= ssimThreshold; } diff --git a/src/tdd/services/comparison-service.js b/src/tdd/services/comparison-service.js index 1bed3efb..e3527930 100644 --- a/src/tdd/services/comparison-service.js +++ b/src/tdd/services/comparison-service.js @@ -38,6 +38,9 @@ export async function compareImages( diffPath, overwrite: true, includeClusters: true, + includeSSIM: true, + includeGMSD: true, + clusterMerge: true, minClusterSize, }); } @@ -144,6 +147,11 @@ export function buildFailedComparison(params) { } = params; let diffClusters = honeydiffResult.diffClusters || []; + let ssimScore = + honeydiffResult.perceptualScore ?? + honeydiffResult.ssimScore ?? + honeydiffResult.ssim_score ?? + null; let isFiltered = false; let filterReason = 'pixel-diff'; @@ -155,7 +163,11 @@ export function buildFailedComparison(params) { if (confirmedRegions.length > 0 && diffClusters.length > 0) { regionCoverage = calculateRegionCoverage(diffClusters, confirmedRegions); - if (shouldAutoApproveFromRegions(confirmedRegions, regionCoverage)) { + if ( + shouldAutoApproveFromRegions(confirmedRegions, regionCoverage, { + ssimScore, + }) + ) { isRegionFiltered = true; isFiltered = true; filterReason = 'region-filtered'; @@ -210,8 +222,11 @@ export function buildFailedComparison(params) { coverage: regionCoverage.coverage, clustersInRegions: regionCoverage.clustersInRegions, totalClusters: regionCoverage.totalClusters, + pixelsInRegions: regionCoverage.pixelsInRegions, + totalPixels: regionCoverage.totalPixels, matchedRegions: regionCoverage.matchedRegions, confirmedCount: confirmedRegions.length, + ssimScore, isFiltered: isRegionFiltered, } : null, diff --git a/src/tdd/tdd-service.js b/src/tdd/tdd-service.js index 44a1e174..9e07c80f 100644 --- a/src/tdd/tdd-service.js +++ b/src/tdd/tdd-service.js @@ -19,6 +19,7 @@ import { getBatchHotspots as defaultGetBatchHotspots, getBuilds as defaultGetBuilds, getComparison as defaultGetComparison, + getComparisonContext as defaultGetComparisonContext, getTddBaselines as defaultGetTddBaselines, } from '../api/index.js'; import { NetworkError } from '../errors/vizzly-error.js'; @@ -132,6 +133,7 @@ export class TddService { getTddBaselines: defaultGetTddBaselines, getBuilds: defaultGetBuilds, getComparison: defaultGetComparison, + getComparisonContext: defaultGetComparisonContext, getBatchHotspots: defaultGetBatchHotspots, fetchWithTimeout: defaultFetchWithTimeout, getDefaultBranch: defaultGetDefaultBranch, @@ -252,6 +254,7 @@ export class TddService { getTddBaselines, getBuilds, getComparison, + getComparisonContext, clearBaselineData, generateScreenshotSignature, generateBaselineFilename, @@ -321,10 +324,25 @@ export class TddService { } baselineBuild.screenshots = apiResponse.screenshots; + this.saveDynamicMetadataFromBaselineResponse(apiResponse); } else if (comparisonId) { // Handle specific comparison download output.info(`Using comparison: ${comparisonId}`); let comparison = await getComparison(this.client, comparisonId); + let comparisonContext = null; + if (!comparison.dynamic_content && getComparisonContext) { + try { + comparisonContext = await getComparisonContext( + this.client, + comparisonId + ); + } catch (error) { + output.debug( + 'tdd', + `comparison context unavailable for ${comparisonId}: ${error.message}` + ); + } + } if (!comparison.baseline_screenshot) { throw new Error( @@ -370,6 +388,11 @@ export class TddService { let screenshotName = comparison.baseline_name || comparison.current_name; + let dynamicContent = comparison.dynamic_content || { + hotspot_analysis: comparisonContext?.history?.hotspot_analysis, + confirmed_regions: comparisonContext?.history?.confirmed_regions, + }; + this.saveDynamicMetadataForScreenshot(screenshotName, dynamicContent); let signature = generateScreenshotSignature( screenshotName, screenshotProperties, @@ -430,30 +453,7 @@ export class TddService { baselineBuild = apiResponse.build; baselineBuild.screenshots = apiResponse.screenshots; - - // Store bundled hotspots and regions from API response - if ( - apiResponse.hotspots && - Object.keys(apiResponse.hotspots).length > 0 - ) { - this.hotspotData = apiResponse.hotspots; - saveHotspotMetadata( - this.workingDir, - apiResponse.hotspots, - apiResponse.summary - ); - } - if ( - apiResponse.regions && - Object.keys(apiResponse.regions).length > 0 - ) { - this.regionData = apiResponse.regions; - saveRegionMetadata( - this.workingDir, - apiResponse.regions, - apiResponse.summary - ); - } + this.saveDynamicMetadataFromBaselineResponse(apiResponse); } let buildDetails = baselineBuild; @@ -684,6 +684,57 @@ export class TddService { } } + saveDynamicMetadataFromBaselineResponse(apiResponse = {}) { + let { saveHotspotMetadata, saveRegionMetadata } = this._deps; + let hasHotspots = Object.keys(apiResponse.hotspots || {}).length > 0; + let hasRegions = Object.keys(apiResponse.regions || {}).length > 0; + + if (hasHotspots) { + this.hotspotData = apiResponse.hotspots; + saveHotspotMetadata( + this.workingDir, + apiResponse.hotspots, + apiResponse.summary + ); + } + + if (hasRegions) { + this.regionData = apiResponse.regions; + saveRegionMetadata( + this.workingDir, + apiResponse.regions, + apiResponse.summary + ); + } + } + + saveDynamicMetadataForScreenshot(screenshotName, dynamicContent = {}) { + if (!screenshotName) { + return; + } + + let hotspotAnalysis = dynamicContent.hotspot_analysis; + let confirmedRegions = dynamicContent.confirmed_regions || []; + let hasHotspotAnalysis = hotspotAnalysis?.total_builds_analyzed > 0; + let hasConfirmedRegions = confirmedRegions.length > 0; + let summary = { + screenshots: 1, + hotspots: hasHotspotAnalysis ? 1 : 0, + regions: hasConfirmedRegions ? 1 : 0, + total_regions: confirmedRegions.length, + }; + + this.saveDynamicMetadataFromBaselineResponse({ + hotspots: hasHotspotAnalysis ? { [screenshotName]: hotspotAnalysis } : {}, + regions: hasConfirmedRegions + ? { + [screenshotName]: { confirmed: confirmedRegions, candidates: [] }, + } + : {}, + summary, + }); + } + /** * Process already-fetched baseline data (for use when caller handles auth) * This allows the baseline router to fetch with a project token and pass the response here @@ -728,24 +779,7 @@ export class TddService { let baselineBuild = apiResponse.build; - // Store bundled hotspots and regions from API response - let { saveHotspotMetadata, saveRegionMetadata } = this._deps; - if (apiResponse.hotspots && Object.keys(apiResponse.hotspots).length > 0) { - this.hotspotData = apiResponse.hotspots; - saveHotspotMetadata( - this.workingDir, - apiResponse.hotspots, - apiResponse.summary - ); - } - if (apiResponse.regions && Object.keys(apiResponse.regions).length > 0) { - this.regionData = apiResponse.regions; - saveRegionMetadata( - this.workingDir, - apiResponse.regions, - apiResponse.summary - ); - } + this.saveDynamicMetadataFromBaselineResponse(apiResponse); if (baselineBuild.status === 'failed') { output.warn( diff --git a/tests/tdd/core/region-coverage.test.js b/tests/tdd/core/region-coverage.test.js index 0664e69f..1fae94a8 100644 --- a/tests/tdd/core/region-coverage.test.js +++ b/tests/tdd/core/region-coverage.test.js @@ -104,6 +104,8 @@ describe('tdd/core/region-coverage', () => { clustersInRegions: 0, totalClusters: 0, matchedRegions: [], + pixelsInRegions: 0, + totalPixels: 0, }); }); @@ -118,6 +120,8 @@ describe('tdd/core/region-coverage', () => { clustersInRegions: 0, totalClusters: 0, matchedRegions: [], + pixelsInRegions: 0, + totalPixels: 0, }); }); @@ -141,13 +145,13 @@ describe('tdd/core/region-coverage', () => { assert.strictEqual(result.totalClusters, 1); }); - it('calculates 100% coverage when all clusters in region', () => { + it('calculates 100% coverage when all clusters match confirmed region centers', () => { let result = calculateRegionCoverage( [ - { boundingBox: { x: 10, y: 10, width: 5, height: 5 } }, - { boundingBox: { x: 20, y: 20, width: 5, height: 5 } }, + { boundingBox: { x: 45, y: 45, width: 10, height: 10 } }, + { boundingBox: { x: 50, y: 45, width: 10, height: 10 } }, ], - [{ x1: 0, y1: 0, x2: 100, y2: 100 }] + [{ x1: 45, y1: 45, x2: 55, y2: 55 }] ); assert.strictEqual(result.coverage, 1); @@ -155,7 +159,7 @@ describe('tdd/core/region-coverage', () => { assert.strictEqual(result.totalClusters, 2); }); - it('calculates 0% coverage when no clusters in region', () => { + it('calculates 0% coverage when no clusters match confirmed region centers', () => { let result = calculateRegionCoverage( [ { boundingBox: { x: 200, y: 200, width: 5, height: 5 } }, @@ -169,13 +173,24 @@ describe('tdd/core/region-coverage', () => { assert.strictEqual(result.totalClusters, 2); }); - it('calculates 50% coverage when half clusters in region', () => { + it('calculates 0% coverage when a cluster intersects but center does not match', () => { + let result = calculateRegionCoverage( + [{ boundingBox: { x: 90, y: 90, width: 20, height: 20 } }], + [{ x1: 0, y1: 0, x2: 100, y2: 100 }] + ); + + assert.strictEqual(result.coverage, 0); + assert.strictEqual(result.clustersInRegions, 0); + assert.strictEqual(result.totalClusters, 1); + }); + + it('calculates 50% coverage when half the changed pixels match region centers', () => { let result = calculateRegionCoverage( [ - { boundingBox: { x: 10, y: 10, width: 5, height: 5 } }, // Inside - { boundingBox: { x: 200, y: 200, width: 5, height: 5 } }, // Outside + { boundingBox: { x: 45, y: 45, width: 10, height: 10 } }, + { boundingBox: { x: 200, y: 200, width: 10, height: 10 } }, ], - [{ x1: 0, y1: 0, x2: 100, y2: 100 }] + [{ x1: 45, y1: 45, x2: 55, y2: 55 }] ); assert.strictEqual(result.coverage, 0.5); @@ -183,15 +198,37 @@ describe('tdd/core/region-coverage', () => { assert.strictEqual(result.totalClusters, 2); }); + it('weights coverage by changed pixels instead of cluster count', () => { + let result = calculateRegionCoverage( + [ + { + boundingBox: { x: 45, y: 45, width: 10, height: 10 }, + pixelCount: 100, + }, + { + boundingBox: { x: 200, y: 200, width: 100, height: 100 }, + pixelCount: 900, + }, + ], + [{ x1: 45, y1: 45, x2: 55, y2: 55 }] + ); + + assert.strictEqual(result.clustersInRegions, 1); + assert.strictEqual(result.totalClusters, 2); + assert.strictEqual(result.pixelsInRegions, 100); + assert.strictEqual(result.totalPixels, 1000); + assert.strictEqual(result.coverage, 0.1); + }); + it('handles multiple regions', () => { let result = calculateRegionCoverage( [ - { boundingBox: { x: 10, y: 10, width: 5, height: 5 } }, // In region 1 - { boundingBox: { x: 210, y: 210, width: 5, height: 5 } }, // In region 2 + { boundingBox: { x: 45, y: 45, width: 10, height: 10 } }, + { boundingBox: { x: 245, y: 245, width: 10, height: 10 } }, ], [ - { id: 'region-1', x1: 0, y1: 0, x2: 100, y2: 100 }, - { id: 'region-2', x1: 200, y1: 200, x2: 300, y2: 300 }, + { id: 'region-1', x1: 45, y1: 45, x2: 55, y2: 55 }, + { id: 'region-2', x1: 245, y1: 245, x2: 255, y2: 255 }, ] ); @@ -205,8 +242,8 @@ describe('tdd/core/region-coverage', () => { it('tracks matched region ids', () => { let result = calculateRegionCoverage( - [{ boundingBox: { x: 10, y: 10, width: 5, height: 5 } }], - [{ id: 'timestamp-region', x1: 0, y1: 0, x2: 100, y2: 100 }] + [{ boundingBox: { x: 45, y: 45, width: 10, height: 10 } }], + [{ id: 'timestamp-region', x1: 45, y1: 45, x2: 55, y2: 55 }] ); assert.deepStrictEqual(result.matchedRegions, ['timestamp-region']); @@ -214,8 +251,8 @@ describe('tdd/core/region-coverage', () => { it('tracks matched region labels when no id', () => { let result = calculateRegionCoverage( - [{ boundingBox: { x: 10, y: 10, width: 5, height: 5 } }], - [{ label: 'avatar', x1: 0, y1: 0, x2: 100, y2: 100 }] + [{ boundingBox: { x: 45, y: 45, width: 10, height: 10 } }], + [{ label: 'avatar', x1: 45, y1: 45, x2: 55, y2: 55 }] ); assert.deepStrictEqual(result.matchedRegions, ['avatar']); @@ -241,34 +278,55 @@ describe('tdd/core/region-coverage', () => { assert.strictEqual(result, false); }); - it('returns false when coverage below 80%', () => { + it('returns false when coverage is below cloud threshold', () => { + let result = shouldAutoApproveFromRegions( + [{ x1: 0, y1: 0, x2: 100, y2: 100 }], + { coverage: 0.89 }, + { ssimScore: 0.99 } + ); + assert.strictEqual(result, false); + }); + + it('returns false when SSIM is below cloud threshold', () => { + let result = shouldAutoApproveFromRegions( + [{ x1: 0, y1: 0, x2: 100, y2: 100 }], + { coverage: 1 }, + { ssimScore: 0.94 } + ); + assert.strictEqual(result, false); + }); + + it('returns false when SSIM is missing', () => { let result = shouldAutoApproveFromRegions( [{ x1: 0, y1: 0, x2: 100, y2: 100 }], - { coverage: 0.79 } + { coverage: 1 } ); assert.strictEqual(result, false); }); - it('returns true when coverage exactly 80%', () => { + it('returns true when coverage and SSIM meet cloud thresholds', () => { let result = shouldAutoApproveFromRegions( [{ x1: 0, y1: 0, x2: 100, y2: 100 }], - { coverage: 0.8 } + { coverage: 0.9 }, + { ssimScore: 0.95 } ); assert.strictEqual(result, true); }); - it('returns true when coverage above 80%', () => { + it('returns true when coverage and SSIM are above cloud thresholds', () => { let result = shouldAutoApproveFromRegions( [{ x1: 0, y1: 0, x2: 100, y2: 100 }], - { coverage: 0.95 } + { coverage: 0.95 }, + { ssimScore: 0.99 } ); assert.strictEqual(result, true); }); - it('returns true when coverage is 100%', () => { + it('supports custom thresholds', () => { let result = shouldAutoApproveFromRegions( [{ x1: 0, y1: 0, x2: 100, y2: 100 }], - { coverage: 1.0 } + { coverage: 0.8 }, + { ssimScore: 0.9, coverageThreshold: 0.8, ssimThreshold: 0.9 } ); assert.strictEqual(result, true); }); diff --git a/tests/tdd/services/comparison-service.test.js b/tests/tdd/services/comparison-service.test.js index defa3132..afdc0889 100644 --- a/tests/tdd/services/comparison-service.test.js +++ b/tests/tdd/services/comparison-service.test.js @@ -224,6 +224,77 @@ describe('tdd/services/comparison-service', () => { assert.strictEqual(result.status, 'passed'); assert.strictEqual(result.reason, 'hotspot-filtered'); }); + + it('filters as passed when confirmed region coverage and SSIM match cloud rules', () => { + let result = buildFailedComparison({ + name: 'timestamp', + signature: 'timestamp|1920|chrome', + baselinePath: '/baselines/timestamp.png', + currentPath: '/current/timestamp.png', + diffPath: '/diffs/timestamp.png', + properties: {}, + threshold: 2.0, + minClusterSize: 2, + honeydiffResult: { + diffPercentage: 1.0, + diffPixels: 1000, + totalPixels: 1000000, + perceptualScore: 0.98, + diffClusters: [ + { + boundingBox: { x: 20, y: 20, width: 100, height: 20 }, + pixelCount: 900, + }, + { + boundingBox: { x: 500, y: 500, width: 10, height: 10 }, + pixelCount: 100, + }, + ], + }, + regionData: { + confirmed: [{ id: 'region-1', x1: 15, y1: 15, x2: 125, y2: 45 }], + }, + }); + + assert.strictEqual(result.status, 'passed'); + assert.strictEqual(result.reason, 'region-filtered'); + assert.strictEqual(result.regionAnalysis.coverage, 0.9); + assert.strictEqual(result.regionAnalysis.ssimScore, 0.98); + assert.strictEqual(result.regionAnalysis.isFiltered, true); + }); + + it('keeps confirmed-region diffs failed when SSIM indicates structural change', () => { + let result = buildFailedComparison({ + name: 'layout-shift', + signature: 'layout-shift|1920|chrome', + baselinePath: '/baselines/layout.png', + currentPath: '/current/layout.png', + diffPath: '/diffs/layout.png', + properties: {}, + threshold: 2.0, + minClusterSize: 2, + honeydiffResult: { + diffPercentage: 1.0, + diffPixels: 1000, + totalPixels: 1000000, + perceptualScore: 0.9, + diffClusters: [ + { + boundingBox: { x: 20, y: 20, width: 100, height: 20 }, + pixelCount: 1000, + }, + ], + }, + regionData: { + confirmed: [{ id: 'region-1', x1: 15, y1: 15, x2: 125, y2: 45 }], + }, + }); + + assert.strictEqual(result.status, 'failed'); + assert.strictEqual(result.reason, 'pixel-diff'); + assert.strictEqual(result.regionAnalysis.coverage, 1); + assert.strictEqual(result.regionAnalysis.isFiltered, false); + }); }); describe('buildErrorComparison', () => { diff --git a/tests/tdd/tdd-service.test.js b/tests/tdd/tdd-service.test.js index 46db6d9e..151667c1 100644 --- a/tests/tdd/tdd-service.test.js +++ b/tests/tdd/tdd-service.test.js @@ -40,6 +40,7 @@ function createMockDeps(overrides = {}) { getTddBaselines: async () => null, getBuilds: async () => ({ data: [] }), getComparison: async () => null, + getComparisonContext: async () => null, getBatchHotspots: async () => ({ hotspots: {} }), fetchWithTimeout: async () => ({ ok: true, @@ -60,6 +61,8 @@ function createMockDeps(overrides = {}) { upsertScreenshotInMetadata: () => {}, loadHotspotMetadata: () => null, saveHotspotMetadata: () => {}, + loadRegionMetadata: () => null, + saveRegionMetadata: () => {}, }; let defaultBaseline = { @@ -1234,6 +1237,185 @@ describe('tdd/tdd-service', () => { assert.ok(tddCall, 'Should call injected getTddBaselines'); assert.strictEqual(tddCall.buildId, 'build-123'); }); + + it('saves bundled dynamic metadata when downloading by buildId', async () => { + let savedHotspots = null; + let savedRegions = null; + let mockDeps = createMockDeps({ + api: { + getDefaultBranch: async () => 'main', + getTddBaselines: async () => ({ + build: { + id: 'build-123', + name: 'Test Build', + status: 'completed', + }, + screenshots: [], + signatureProperties: [], + hotspots: { + Dashboard: { + regions: [{ y1: 10, y2: 30 }], + confidence: 'high', + }, + }, + regions: { + Dashboard: { + confirmed: [{ id: 'region-1', x1: 0, y1: 0, x2: 100, y2: 30 }], + candidates: [], + }, + }, + summary: { + screenshots: 1, + hotspots: 1, + regions: 1, + total_regions: 1, + }, + }), + }, + baseline: { + clearBaselineData: () => {}, + }, + metadata: { + saveHotspotMetadata: (_dir, hotspots) => { + savedHotspots = hotspots; + }, + saveRegionMetadata: (_dir, regions) => { + savedRegions = regions; + }, + }, + }); + let service = new TddService({}, '/test', false, null, mockDeps); + + await service.downloadBaselines('test', null, 'build-123', null); + + assert.deepStrictEqual(savedHotspots.Dashboard.regions, [ + { y1: 10, y2: 30 }, + ]); + assert.strictEqual(savedRegions.Dashboard.confirmed[0].id, 'region-1'); + assert.strictEqual(service.hotspotData, savedHotspots); + assert.strictEqual(service.regionData, savedRegions); + }); + + it('saves dynamic metadata when downloading by comparisonId', async () => { + let savedHotspots = null; + let savedRegions = null; + let mockDeps = createMockDeps({ + api: { + getDefaultBranch: async () => 'main', + getComparison: async () => ({ + id: 'comparison-123', + current_name: 'Dashboard', + baseline_screenshot: { + id: 'screenshot-1', + build_id: 'build-123', + original_url: 'https://cdn.test/baseline.png', + }, + current_viewport_width: 1280, + current_viewport_height: 720, + current_browser: 'chromium', + dynamic_content: { + hotspot_analysis: { + regions: [{ y1: 20, y2: 40 }], + total_builds_analyzed: 4, + confidence: 'high', + }, + confirmed_regions: [ + { id: 'region-1', x1: 0, y1: 20, x2: 200, y2: 40 }, + ], + }, + }), + }, + baseline: { + clearBaselineData: () => {}, + }, + metadata: { + saveHotspotMetadata: (_dir, hotspots) => { + savedHotspots = hotspots; + }, + saveRegionMetadata: (_dir, regions) => { + savedRegions = regions; + }, + }, + }); + let service = new TddService({}, '/test', false, null, mockDeps); + + await service.downloadBaselines('test', null, null, 'comparison-123'); + + assert.deepStrictEqual(savedHotspots.Dashboard.regions, [ + { y1: 20, y2: 40 }, + ]); + assert.strictEqual(savedRegions.Dashboard.confirmed[0].id, 'region-1'); + assert.strictEqual(service.baselineData.buildId, 'build-123'); + }); + + it('falls back to comparison context dynamic metadata when comparison is older shape', async () => { + let savedHotspots = null; + let savedRegions = null; + let contextCalls = 0; + let mockDeps = createMockDeps({ + api: { + getDefaultBranch: async () => 'main', + getComparison: async () => ({ + id: 'comparison-123', + current_name: 'Dashboard', + baseline_screenshot: { + id: 'screenshot-1', + build_id: 'build-123', + original_url: 'https://cdn.test/baseline.png', + }, + current_viewport_width: 1280, + current_viewport_height: 720, + current_browser: 'chromium', + }), + getComparisonContext: async (_client, comparisonId) => { + contextCalls++; + assert.strictEqual(comparisonId, 'comparison-123'); + return { + history: { + hotspot_analysis: { + regions: [{ y1: 32, y2: 48 }], + total_builds_analyzed: 3, + confidence: 'high', + }, + confirmed_regions: [ + { + id: 'region-from-context', + x1: 10, + y1: 32, + x2: 220, + y2: 48, + }, + ], + }, + }; + }, + }, + baseline: { + clearBaselineData: () => {}, + }, + metadata: { + saveHotspotMetadata: (_dir, hotspots) => { + savedHotspots = hotspots; + }, + saveRegionMetadata: (_dir, regions) => { + savedRegions = regions; + }, + }, + }); + let service = new TddService({}, '/test', false, null, mockDeps); + + await service.downloadBaselines('test', null, null, 'comparison-123'); + + assert.strictEqual(contextCalls, 1); + assert.deepStrictEqual(savedHotspots.Dashboard.regions, [ + { y1: 32, y2: 48 }, + ]); + assert.strictEqual( + savedRegions.Dashboard.confirmed[0].id, + 'region-from-context' + ); + assert.strictEqual(service.baselineData.buildId, 'build-123'); + }); }); describe('createNewBaseline', () => { From 5e969891a040f1aa1174d5a3da656e85ed3479c1 Mon Sep 17 00:00:00 2001 From: Robert DeLuca Date: Sat, 23 May 2026 13:29:21 -0500 Subject: [PATCH 2/3] =?UTF-8?q?=E2=AC=86=EF=B8=8F=20Upgrade=20honeydiff=20?= =?UTF-8?q?to=20v0.10.3?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use the latest released honeydiff package for local TDD comparisons so the CLI runs against the newest diff metadata behavior. --- package-lock.json | 8 ++++---- package.json | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/package-lock.json b/package-lock.json index 651551ae..8d1d8693 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9,7 +9,7 @@ "version": "0.31.0", "license": "MIT", "dependencies": { - "@vizzly-testing/honeydiff": "^0.10.0", + "@vizzly-testing/honeydiff": "^0.10.3", "ansis": "^4.2.0", "commander": "^14.0.0", "cosmiconfig": "^9.0.0", @@ -3019,9 +3019,9 @@ } }, "node_modules/@vizzly-testing/honeydiff": { - "version": "0.10.1", - "resolved": "https://registry.npmjs.org/@vizzly-testing/honeydiff/-/honeydiff-0.10.1.tgz", - "integrity": "sha512-XKvW4WkHx9UaDs1cQCR83RC5KR1xmnyune4C7Gifb51+KVhdmZzl/hbRNbzInrK0oQij294xwn+tv8riuNusYg==", + "version": "0.10.3", + "resolved": "https://registry.npmjs.org/@vizzly-testing/honeydiff/-/honeydiff-0.10.3.tgz", + "integrity": "sha512-gzcd2Ryr0l9trrNb4PL3oo51HzBFFDQBiXTTF56+3IHi/yHo52qCvKpJQAPZwauWOuBXr53dIa7OelkTxkyeCA==", "license": "MIT", "engines": { "node": ">=22.0.0" diff --git a/package.json b/package.json index 960f8be9..ba2cc25c 100644 --- a/package.json +++ b/package.json @@ -90,7 +90,7 @@ "registry": "https://registry.npmjs.org/" }, "dependencies": { - "@vizzly-testing/honeydiff": "^0.10.0", + "@vizzly-testing/honeydiff": "^0.10.3", "ansis": "^4.2.0", "commander": "^14.0.0", "cosmiconfig": "^9.0.0", From 0e9914f834e21532a0dfd3f00ca2e4ed6e3a4fb0 Mon Sep 17 00:00:00 2001 From: Robert DeLuca Date: Sat, 23 May 2026 14:45:21 -0500 Subject: [PATCH 3/3] =?UTF-8?q?=F0=9F=90=9B=20Ship=20static=20report=20run?= =?UTF-8?q?time=20dependencies?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Keep React and React DOM in the published dependency set because the packed CLI loads the SSR report renderer at runtime. --- package-lock.json | 9 +++------ package.json | 4 ++-- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/package-lock.json b/package-lock.json index 8d1d8693..b9bfeb05 100644 --- a/package-lock.json +++ b/package-lock.json @@ -16,7 +16,9 @@ "dotenv": "^17.2.1", "form-data": "^4.0.0", "glob": "^13.0.0", - "zod": "^4.1.12" + "zod": "^4.1.12", + "react": "^19.1.1", + "react-dom": "^19.1.1" }, "bin": { "vizzly": "bin/vizzly.js" @@ -40,8 +42,6 @@ "babel-plugin-react-compiler": "^1.0.0", "babel-plugin-transform-remove-console": "^6.9.4", "postcss": "^8.5.6", - "react": "^19.1.1", - "react-dom": "^19.1.1", "rimraf": "^6.0.1", "tailwindcss": "^4.1.13", "tsd": "^0.33.0", @@ -6389,7 +6389,6 @@ "version": "19.2.5", "resolved": "https://registry.npmjs.org/react/-/react-19.2.5.tgz", "integrity": "sha512-llUJLzz1zTUBrskt2pwZgLq59AemifIftw4aB7JxOqf1HY2FDaGDxgwpAPVzHU1kdWabH7FauP4i1oEeer2WCA==", - "dev": true, "license": "MIT", "engines": { "node": ">=0.10.0" @@ -6399,7 +6398,6 @@ "version": "19.2.5", "resolved": "https://registry.npmjs.org/react-dom/-/react-dom-19.2.5.tgz", "integrity": "sha512-J5bAZz+DXMMwW/wV3xzKke59Af6CHY7G4uYLN1OvBcKEsWOs4pQExj86BBKamxl/Ik5bx9whOrvBlSDfWzgSag==", - "dev": true, "license": "MIT", "dependencies": { "scheduler": "^0.27.0" @@ -6837,7 +6835,6 @@ "version": "0.27.0", "resolved": "https://registry.npmjs.org/scheduler/-/scheduler-0.27.0.tgz", "integrity": "sha512-eNv+WrVbKu1f3vbYJT/xtiF5syA5HPIMtf9IgY/nKg0sWqzAUEvqY/xm7OcZc/qafLx/iO9FgOmeSAp4v5ti/Q==", - "dev": true, "license": "MIT" }, "node_modules/semver": { diff --git a/package.json b/package.json index ba2cc25c..da75ade8 100644 --- a/package.json +++ b/package.json @@ -97,6 +97,8 @@ "dotenv": "^17.2.1", "form-data": "^4.0.0", "glob": "^13.0.0", + "react": "^19.1.1", + "react-dom": "^19.1.1", "zod": "^4.1.12" }, "tsd": { @@ -128,8 +130,6 @@ "babel-plugin-react-compiler": "^1.0.0", "babel-plugin-transform-remove-console": "^6.9.4", "postcss": "^8.5.6", - "react": "^19.1.1", - "react-dom": "^19.1.1", "rimraf": "^6.0.1", "tailwindcss": "^4.1.13", "tsd": "^0.33.0",