diff --git a/docs/browser-flags.md b/docs/browser-flags.md index 57af6968..e830417f 100644 --- a/docs/browser-flags.md +++ b/docs/browser-flags.md @@ -12,7 +12,7 @@ We migrated from Puppeteer to Playwright because: ## Source of Truth -The authoritative reference for Chrome flags is maintained by the Chrome team: +The Chrome team maintains the reference for Chrome flags: - **Primary**: [Chrome Flags for Tools](https://github.com/GoogleChrome/chrome-launcher/blob/main/docs/chrome-flags-for-tools.md) - Curated list for automation tools - **Complete list**: [peter.sh/experiments/chromium-command-line-switches](https://peter.sh/experiments/chromium-command-line-switches/) - All Chromium switches diff --git a/docs/json-output.md b/docs/json-output.md index 27ff4d26..5702f0e0 100644 --- a/docs/json-output.md +++ b/docs/json-output.md @@ -105,7 +105,7 @@ With `--wait`, includes comparison results: "changed": 1, "identical": 12 }, - "approvalStatus": "pending", + "visual_review": { "state": "pending" }, "contextCommand": "vizzly context build abc123-def456 --agent --json --source cloud", "exitCode": 1 } @@ -289,13 +289,13 @@ Compact agent JSON: "build": { "id": "abc123", "status": "completed", - "approval_status": "pending" + "review_state": "pending" }, "baseline": { "selected": { "id": "baseline-build", "name": "Approved Main", - "approval_status": "approved" + "review_state": "approved" }, "selection_reason": "latest approved build" }, @@ -319,7 +319,7 @@ Compact agent JSON: "id": "cmp-1", "screenshot_name": "Dashboard", "result": "changed", - "approval_status": "pending", + "review_state": "pending", "needs_review": true, "screenshot": { "id": "current-1", @@ -390,13 +390,13 @@ Full build context JSON: "build": { "id": "abc123", "status": "completed", - "approval_status": "pending" + "review_state": "pending" }, "baseline": { "selected": { "id": "baseline-build", "name": "Approved Main", - "approval_status": "approved" + "review_state": "approved" }, "selection_reason": "common_ancestor", "comparison_baseline_build_ids": ["baseline-build"] @@ -469,7 +469,7 @@ Agent comparison JSON: "id": "cmp-1", "screenshot_name": "Dashboard", "result": "changed", - "approval_status": "pending", + "review_state": "pending", "screenshot": { "url": "https://.../current.png" }, @@ -523,16 +523,12 @@ Raw comparison JSON remains available without `--agent`: "result": "changed", "analysis": { "diff_image_url": ".vizzly/diffs/dashboard.png", - "diff_regions": [], - "confirmed_regions": [] + "diff_regions": [] } }, "history": { "similar_by_fingerprint": [], - "recent_by_name": [], - "hotspot_analysis": { - "confidence": "no_data" - } + "recent_by_name": [] } } ``` @@ -551,11 +547,6 @@ vizzly context screenshot Dashboard --source local --json "screenshot": { "name": "Dashboard" }, - "confirmed_regions": [ - { - "label": "Known header copy band" - } - ], "history": { "recent_comparisons": [] } @@ -643,7 +634,7 @@ vizzly builds --branch main --status completed --limit 10 --json "changed": 1, "identical": 12 }, - "approvalStatus": "approved", + "visual_review": { "state": "approved" }, "createdAt": "2025-01-15T10:30:00Z", "completedAt": "2025-01-15T10:32:00Z" } @@ -687,9 +678,7 @@ vizzly comparisons --build --status changed --json "status": "completed", "result": "changed", "diffPercentage": 0.042, - "approvalStatus": "pending", - "reviewState": "pending", - "visualReview": { "state": "pending" }, + "visual_review": { "state": "pending" }, "viewport": { "width": 1920, "height": 1080 }, "browser": "chromium", "urls": { @@ -700,7 +689,7 @@ vizzly comparisons --build --status changed --json "honeydiff": { "fingerprintHash": "00000000001ec127", "regionCount": 12, - "projection": { + "details": { "clusters": { "count": 12, "average_density": 0.81 } } } @@ -717,9 +706,8 @@ vizzly comparisons --build --status changed --json ``` `status` preserves the processing value returned by the API. Use `result` for -the visual outcome (`identical`, `changed`, or `new`) and `reviewState` for the -current review decision. Older responses that only provide `status` and -`approvalStatus` keep working with the same fields. +the visual outcome (`identical`, `changed`, or `new`) and `visual_review.state` +for the current review state. Search by name across builds: @@ -913,7 +901,7 @@ vizzly upload ./screenshots --wait --json "failed": 2, "new": 1 }, - "approvalStatus": "pending", + "visual_review": { "state": "pending" }, "executionTimeMs": 9876 } } @@ -955,9 +943,9 @@ vizzly approve --json { "status": "data", "data": { - "approved": true, - "comparisonId": "comp_123", - "comparison": { /* updated comparison */ } + "eventId": "event_123", + "idempotent": false, + "review": { /* visual review */ } } } ``` @@ -972,10 +960,9 @@ vizzly reject --reason "Unexpected regression" --json { "status": "data", "data": { - "rejected": true, - "comparisonId": "comp_123", - "reason": "Unexpected regression", - "comparison": { /* updated comparison */ } + "eventId": "event_124", + "idempotent": false, + "review": { /* visual review */ } } } ``` @@ -1040,15 +1027,7 @@ vizzly status --json "newComparisons": 2, "changedComparisons": 1, "identicalComparisons": 12, - "reviewState": "pending", - "review": { - "pending": 3, - "approved": 12, - "rejected": 0, - "auto_approved": 0 - }, - "reviewFlow": "cricket", - "visualReview": { "state": "pending" }, + "visual_review": { "state": "pending" }, "executionTime": 4500, "scope": { "organization": { "id": "org-1", "slug": "acme" }, @@ -1080,9 +1059,7 @@ vizzly status --json The status command reads the API status bundle directly. Processing counts, comparison counts, conclusion, and review state stay separate; a pending review is never treated as unfinished screenshot processing. Fields the API does not -provide are omitted instead of becoming client-authored zeroes. Legacy review -responses continue to expose `approvalStatus`, and legacy build links continue -to use the project-ID route when slug scope is unavailable. +provide are omitted instead of becoming client-authored zeroes. ### `vizzly init` diff --git a/skills/vizzly/SKILL.md b/skills/vizzly/SKILL.md index b731ecee..eb9c2466 100644 --- a/skills/vizzly/SKILL.md +++ b/skills/vizzly/SKILL.md @@ -1,6 +1,6 @@ --- name: vizzly -description: Inspect and explain Vizzly visual regression evidence, verify UI changes, troubleshoot local or cloud builds, adjust screenshot capture, or configure Vizzly CI. Use when a task mentions Vizzly, screenshot baselines, comparisons, Honeydiff, visual review, dynamic regions, or an existing Vizzly workflow. +description: Inspect and explain Vizzly visual regression evidence, verify UI changes, troubleshoot local or cloud builds, adjust screenshot capture, or configure Vizzly CI. Use when a task mentions Vizzly, screenshot baselines, comparisons, Honeydiff, visual review, or an existing Vizzly workflow. --- # Vizzly @@ -47,8 +47,8 @@ test workflow in charge of how the UI is exercised. values. - Do not approve, reject, comment on, publish, or replace evidence unless the task explicitly asks for that mutation. -- Preserve thresholds, cluster sizes, signature properties, and dynamic-region - behavior unless repeated evidence justifies a change. +- Preserve thresholds, cluster sizes, signature properties, and other capture + settings unless repeated evidence justifies a change. - Prefer deterministic fixtures and existing user journeys over hiding a diff with broader tolerances or a new screenshot-only test. - Report the comparison or screenshot identity, observed evidence, access @@ -60,5 +60,5 @@ test workflow in charge of how the UI is exercised. discovery, drill-downs, images, and TDD lifecycle. - [SDK capture](references/sdks.md): add or change screenshot capture code. - [Dynamic content](references/dynamic-content.md): investigate unstable - content, hotspots, or confirmed regions. + content and screenshot-specific tolerances. - [Setup and CI](references/setup-ci.md): initialize Vizzly or change CI. diff --git a/skills/vizzly/references/dynamic-content.md b/skills/vizzly/references/dynamic-content.md index 1ce3230d..64c1ba93 100644 --- a/skills/vizzly/references/dynamic-content.md +++ b/skills/vizzly/references/dynamic-content.md @@ -14,8 +14,8 @@ can cause a diff. They are possible explanations, not conclusions. 2. Record the visible region, recurrence, render metadata, and whether the same change appears across builds or variants. 3. Prefer deterministic fixtures when they can remove irrelevant variation. -4. Treat hotspots and confirmed regions as server- or user-authored evidence. - This CLI exposes them for inspection, not authoring. +4. Use diff regions only to locate changed pixels; they do not approve a + comparison or explain why it changed. 5. Preserve `threshold` and `minClusterSize` unless repeated evidence justifies a change. diff --git a/src/api/endpoints.js b/src/api/endpoints.js index 30435f71..4dbb22cb 100644 --- a/src/api/endpoints.js +++ b/src/api/endpoints.js @@ -44,7 +44,7 @@ export async function getBuild(client, buildId, include = null) { * * @param {Object} client - API client * @param {string} buildId - Build ID - * @returns {Promise} Canonical build status bundle + * @returns {Promise} Build status response */ export async function getBuildStatus(client, buildId) { return client.request(`/api/sdk/builds/${buildId}/status`); @@ -375,52 +375,6 @@ export async function searchComparisons(client, name, filters = {}) { return client.request(endpoint); } -// ============================================================================ -// Hotspot Endpoints -// ============================================================================ - -/** - * Get hotspot analysis for a single screenshot - * @param {Object} client - API client - * @param {string} screenshotName - Screenshot name - * @param {Object} options - Optional settings - * @returns {Promise} Hotspot analysis data - */ -export async function getScreenshotHotspots( - client, - screenshotName, - options = {} -) { - let { windowSize = 20 } = options; - let encodedName = encodeURIComponent(screenshotName); - let endpoint = buildEndpointWithParams( - `/api/sdk/screenshots/${encodedName}/hotspots`, - { - windowSize: String(windowSize), - } - ); - return client.request(endpoint); -} - -/** - * Batch get hotspot analysis for multiple screenshots - * @param {Object} client - API client - * @param {string[]} screenshotNames - Array of screenshot names - * @param {Object} options - Optional settings - * @returns {Promise} Hotspots keyed by screenshot name - */ -export async function getBatchHotspots(client, screenshotNames, options = {}) { - let { windowSize = 20 } = options; - return client.request('/api/sdk/screenshots/hotspots', { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ - screenshot_names: screenshotNames, - windowSize, - }), - }); -} - // ============================================================================ // Auth/Token Endpoints // ============================================================================ diff --git a/src/api/index.js b/src/api/index.js index b699e347..957d813a 100644 --- a/src/api/index.js +++ b/src/api/index.js @@ -42,7 +42,6 @@ export { createBuild, finalizeBuild, finalizeParallelBuild, - getBatchHotspots, getBuild, getBuildContext, getBuildStatus, @@ -52,7 +51,6 @@ export { getPreviewInfo, getReviewQueueContext, getScreenshotContext, - getScreenshotHotspots, getSimilarFingerprintContext, getTddBaselines, getTokenContext, diff --git a/src/cli.js b/src/cli.js index ebb69a0f..b706e2c4 100644 --- a/src/cli.js +++ b/src/cli.js @@ -1059,11 +1059,6 @@ contextCmd 'Maximum recent comparisons to return (1-50)', Number ) - .option( - '--window-size ', - 'Historical hotspot analysis window size (1-50)', - Number - ) .addHelpText( 'after', ` @@ -1181,7 +1176,7 @@ program .argument('', 'API endpoint (e.g., /api/sdk/builds)') .option( '-X, --method ', - 'HTTP method (GET or POST for approve/reject/comment)', + 'HTTP method (GET or POST for build comments)', 'GET' ) .option('-d, --data ', 'Request body (JSON)') @@ -1202,10 +1197,10 @@ Examples: $ vizzly api /api/sdk/builds # List builds $ vizzly api /api/sdk/builds -q limit=5 # With query params $ vizzly api /api/sdk/builds/abc123 # Get specific build - $ vizzly api /api/sdk/comparisons/abc123/approve -X POST + $ vizzly api /api/sdk/builds/abc123/comments -X POST -d '{"content":"Looks good"}' $ vizzly api /api/sdk/builds/abc123/comments -X POST -d '{"content":"Nice!"}' -Note: POST is restricted to approve, reject, and comment endpoints. +Note: POST is restricted to build comment endpoints. Use dedicated approve/reject commands for review decisions. Most operations have dedicated commands (builds, comparisons, approve, etc.). ` ) diff --git a/src/commands/api.js b/src/commands/api.js index 9632afec..4b3ed295 100644 --- a/src/commands/api.js +++ b/src/commands/api.js @@ -6,11 +6,7 @@ import { createApiClient as defaultCreateApiClient } from '../api/index.js'; import { loadConfig as defaultLoadConfig } from '../utils/config-loader.js'; import * as defaultOutput from '../utils/output.js'; -let ALLOWED_POST_ENDPOINTS = [ - /^\/api\/sdk\/comparisons\/[^/]+\/approve$/, - /^\/api\/sdk\/comparisons\/[^/]+\/reject$/, - /^\/api\/sdk\/builds\/[^/]+\/comments$/, -]; +let ALLOWED_POST_ENDPOINTS = [/^\/api\/sdk\/builds\/[^/]+\/comments$/]; function createApiCommandDeps(deps = {}) { return { @@ -89,14 +85,14 @@ export function validateApiRequest({ endpoint, method, hasData = false }) { if (method !== 'GET' && method !== 'POST') { errors.push( - `Method ${method} not allowed. Use GET for queries or POST for approve/reject/comment.` + `Method ${method} not allowed. Use GET for queries or POST for build comments.` ); return errors; } if (method === 'POST' && !isAllowedPostEndpoint(endpoint)) { errors.push( - `POST not allowed for ${endpoint}. Only approve, reject, and comment endpoints support POST.` + `POST not allowed for ${endpoint}. Only build comment endpoints support POST.` ); } diff --git a/src/commands/builds.js b/src/commands/builds.js index 85332b18..8987c1ca 100644 --- a/src/commands/builds.js +++ b/src/commands/builds.js @@ -132,14 +132,13 @@ export async function buildsCommand( let statusColor = getStatusColor(colors, build.status); let statusBadge = statusColor(build.status.toUpperCase()); - // Approval badge - let approvalBadge = ''; - if (build.approval_status && build.status === 'completed') { - approvalBadge = ` ${getApprovalBadge(colors, build.approval_status)}`; + let reviewBadge = ''; + if (build.visual_review?.state && build.status === 'completed') { + reviewBadge = ` ${getReviewBadge(colors, build.visual_review.state)}`; } output.print( - ` ${colors.bold(build.name || build.id)} ${statusBadge}${approvalBadge}` + ` ${colors.bold(build.name || build.id)} ${statusBadge}${reviewBadge}` ); let details = []; @@ -207,7 +206,7 @@ function formatBuildForJson(build, includeComparisons = false) { changed: build.changed_comparisons || 0, identical: build.identical_comparisons || 0, }, - approvalStatus: build.approval_status, + visual_review: build.visual_review || null, createdAt: build.created_at, completedAt: build.completed_at, }; @@ -233,7 +232,7 @@ function formatBuildForJson(build, includeComparisons = false) { name: c.name || c.current_name, status: c.status, diffPercentage: c.diff_percentage, - approvalStatus: c.approval_status, + visual_review: c.visual_review || null, urls: { baseline: c.baseline_screenshot?.original_url || @@ -301,8 +300,8 @@ function displayBuild(output, build, verbose) { output.labelValue('Comparisons', stats.join(colors.dim(' · '))); } - if (build.approval_status) { - output.labelValue('Approval', build.approval_status); + if (build.visual_review?.state) { + output.labelValue('Review', build.visual_review.state); } output.blank(); @@ -364,19 +363,18 @@ function getStatusColor(colors, status) { } /** - * Get colored approval badge + * Get colored review badge */ -function getApprovalBadge(colors, approvalStatus) { - switch (approvalStatus) { +function getReviewBadge(colors, reviewState) { + switch (reviewState) { case 'approved': - case 'auto_approved': return colors.brand.success('APPROVED'); case 'rejected': return colors.brand.error('REJECTED'); case 'pending': return colors.brand.warning('PENDING'); default: - return colors.dim(approvalStatus?.toUpperCase() || ''); + return colors.dim(reviewState?.toUpperCase() || ''); } } diff --git a/src/commands/comparisons.js b/src/commands/comparisons.js index 27b5c981..8fa55d43 100644 --- a/src/commands/comparisons.js +++ b/src/commands/comparisons.js @@ -16,7 +16,6 @@ import { getComparisonName, getComparisonResult, getComparisonViewport, - getVisualReviewState, } from '../utils/visual-context-normalizers.js'; /** @@ -215,17 +214,13 @@ function formatComparisonForJson(comparison) { let gmsdScore = comparison.gmsd_score ?? diffImage.gmsd_score ?? null; let fingerprintHash = comparison.fingerprint_hash || diffImage.fingerprint_hash || null; - let projection = - comparison.analysis_projection || - comparison.projection || - diffImage.analysis_projection || - diffImage.projection || - null; + let analysisDetails = + comparison.analysis_details || diffImage.analysis_details || null; let diffRegions = comparison.diff_regions ?? diffImage.diff_regions ?? null; let regionCount = comparison.region_count ?? diffImage.region_count ?? - projection?.clusters?.count ?? + analysisDetails?.clusters?.count ?? (Array.isArray(diffRegions) ? diffRegions.length : null); let hasHoneydiff = @@ -233,7 +228,7 @@ function formatComparisonForJson(comparison) { ssimScore != null || gmsdScore != null || fingerprintHash || - projection || + analysisDetails || regionCount != null; return { @@ -242,9 +237,7 @@ function formatComparisonForJson(comparison) { status: comparison.status, result: getComparisonResult(comparison), diffPercentage: comparison.diff_percentage ?? null, - approvalStatus: comparison.approval_status, - reviewState: getVisualReviewState(comparison), - visualReview: comparison.visual_review || null, + visual_review: comparison.visual_review || null, viewport: getComparisonViewport(comparison), browser: getComparisonBrowser(comparison), urls: { @@ -272,7 +265,7 @@ function formatComparisonForJson(comparison) { clusterMetadata, fingerprintHash, regionCount, - projection, + details: analysisDetails, diffRegions, diffLines: comparison.diff_lines ?? diffImage.diff_lines ?? null, fingerprintData: @@ -300,7 +293,7 @@ function displayComparison(output, comparison, verbose) { comparison.diff_percentage != null ? `${(comparison.diff_percentage * 100).toFixed(2)}%` : 'N/A', - Review: formatted.reviewState || 'unknown', + Review: formatted.visual_review?.state || 'unknown', }); output.blank(); diff --git a/src/commands/context.js b/src/commands/context.js index 029144bd..a30c5f33 100644 --- a/src/commands/context.js +++ b/src/commands/context.js @@ -836,14 +836,6 @@ function formatNeedsReview(status = {}) { return details.length > 0 ? `yes · ${details.join(' · ')}` : 'yes'; } -function formatConfirmedRegionLabels(regions = []) { - return regions - .map(region => region.label) - .filter(Boolean) - .slice(0, 3) - .join(' · '); -} - function printComparisonList(output, comparisons = [], { limit = 5 } = {}) { let colors = output.getColors(); @@ -1102,9 +1094,6 @@ function displayComparisonContext(output, context) { let screenshotName = context.comparison.screenshot?.name || context.comparison.id; let analysis = context.comparison.analysis || {}; - let confirmedRegionLabels = formatConfirmedRegionLabels( - context.history.confirmed_regions - ); output.print( ` ${colors.bold(screenshotName)} ${statusTone(displayState.toUpperCase())}` @@ -1120,7 +1109,7 @@ function displayComparisonContext(output, context) { ); output.labelValue( 'Memory', - `${context.history.similar_by_fingerprint.length} similar · ${context.history.recent_by_name.length} recent · ${context.history.confirmed_regions.length} confirmed regions` + `${context.history.similar_by_fingerprint.length} similar · ${context.history.recent_by_name.length} recent` ); output.labelValue( 'Review', @@ -1131,10 +1120,6 @@ function displayComparisonContext(output, context) { output.labelValue('Fingerprint', analysis.fingerprint_hash); } - if (confirmedRegionLabels) { - output.labelValue('Known Regions', confirmedRegionLabels); - } - if (context.links?.comparison_url) { output.labelValue('Comparison URL', context.links.comparison_url); } @@ -1150,10 +1135,6 @@ function displayScreenshotContext(output, context) { output.header('context', 'screenshot'); let colors = output.getColors(); - let confirmedRegionLabels = formatConfirmedRegionLabels( - context.confirmed_regions - ); - let hotspotAnalysis = context.hotspot_analysis; output.print(` ${colors.bold(context.screenshot.name)}`); output.print( @@ -1163,20 +1144,8 @@ function displayScreenshotContext(output, context) { output.labelValue( 'Memory', - `${context.history.recent_comparisons.length} recent comparisons · ${context.confirmed_regions.length} confirmed regions` + `${context.history.recent_comparisons.length} recent comparisons` ); - if (hotspotAnalysis) { - output.labelValue( - 'Hotspots', - `${hotspotAnalysis.total_builds_analyzed ?? 'unknown'} builds analyzed · ${hotspotAnalysis.confidence ?? 'unknown'}` - ); - } else { - output.labelValue('Hotspots', 'unavailable'); - } - - if (confirmedRegionLabels) { - output.labelValue('Known Regions', confirmedRegionLabels); - } if (context.history.recent_comparisons.length > 0) { output.blank(); @@ -1414,7 +1383,6 @@ export async function contextScreenshotCommand( } let query = buildScopeQuery(options, { recentLimit: options.recentLimit, - windowSize: options.windowSize, }); output.startSpinner('Fetching screenshot context...'); @@ -1572,11 +1540,6 @@ export function validateContextScreenshotOptions(options = {}) { max: 50, }) ); - errors.push( - ...validateLimitRange(options.windowSize, '--window-size', { - max: 50, - }) - ); return errors; } diff --git a/src/commands/review.js b/src/commands/review.js index c02f63b6..d49ebd20 100644 --- a/src/commands/review.js +++ b/src/commands/review.js @@ -2,6 +2,8 @@ * Review commands - approve, reject, and comment on comparisons/builds */ +import { randomUUID as defaultRandomUUID } from 'node:crypto'; + import { createApiClient as defaultCreateApiClient } from '../api/index.js'; import { loadConfig as defaultLoadConfig } from '../utils/config-loader.js'; import { getAccessToken as defaultGetAccessToken } from '../utils/global-config.js'; @@ -12,6 +14,7 @@ function createReviewDeps(deps = {}) { loadConfig: deps.loadConfig || defaultLoadConfig, createApiClient: deps.createApiClient || defaultCreateApiClient, getAccessToken: deps.getAccessToken || defaultGetAccessToken, + randomUUID: deps.randomUUID || defaultRandomUUID, output: deps.output || defaultOutput, exit: deps.exit || (code => process.exit(code)), }; @@ -143,11 +146,11 @@ async function runReviewMutation({ } export function createApprovalBody(options = {}) { - return options.comment ? { comment: options.comment } : {}; + return options.comment ? { annotation: options.comment.trim() } : {}; } export function createRejectionBody(options = {}) { - return { reason: options.reason }; + return { annotation: options.reason.trim() }; } export function createCommentBody(message, options = {}) { @@ -157,6 +160,129 @@ export function createCommentBody(message, options = {}) { }; } +function requireReviewLocator(response) { + let locator = response?.review_locator; + if ( + !locator?.comparison_id || + !locator?.build_id || + !locator?.organization_slug + ) { + let error = new Error( + 'Comparison response did not include review location details' + ); + error.code = 'INVALID_REVIEW_CONTEXT'; + throw error; + } + return locator; +} + +async function runComparisonDecision({ + command, + comparisonId, + decision, + annotation, + failureMessage, + globalOptions, + options, + spinnerMessage, + writeHumanSuccess, + deps, + configure = true, +}) { + let { + loadConfig, + createApiClient, + getAccessToken, + randomUUID, + output, + exit, + } = createReviewDeps(deps); + + if (configure) { + configureOutput(output, globalOptions); + } + + try { + let config = await loadReviewConfig({ loadConfig, options, globalOptions }); + let token = await getReviewToken(config, getAccessToken); + if (!token) { + output.error('User login required for review actions'); + output.hint('Run "vizzly login" to approve, reject, or comment'); + output.cleanup(); + exit(1); + return; + } + + output.startSpinner(spinnerMessage); + let client = createReviewClient({ + createApiClient, + config, + command, + token, + }); + let comparisonResponse = await client.request( + `/api/sdk/comparisons/${comparisonId}` + ); + let locator = requireReviewLocator(comparisonResponse); + if (locator.comparison_id !== comparisonId) { + let error = new Error( + 'Comparison response did not match the requested comparison' + ); + error.code = 'INVALID_REVIEW_CONTEXT'; + throw error; + } + + let organizationHeaders = { + 'X-Organization': locator.organization_slug, + }; + let commandId = randomUUID(); + let response = await client.request( + `/api/visual-review/builds/${locator.build_id}/comparisons/${comparisonId}/decision`, + { + method: 'POST', + headers: { + ...organizationHeaders, + 'Content-Type': 'application/json', + }, + body: JSON.stringify({ + commandId, + decision, + ...(annotation ? { annotation } : {}), + }), + } + ); + + output.stopSpinner(); + if (globalOptions.json) { + output.data({ + eventId: response.eventId, + idempotent: response.idempotent, + review: response.review, + }); + output.cleanup(); + return; + } + + writeHumanSuccess(output); + output.cleanup(); + } catch (error) { + output.stopSpinner(); + if (globalOptions.json) { + output.data({ + comparisonId, + error: { message: error.message, code: error.code }, + }); + output.cleanup(); + exit(1); + return; + } + + output.error(failureMessage, error); + output.cleanup(); + exit(1); + } +} + /** * Approve a comparison * @param {string} comparisonId - Comparison ID to approve @@ -170,26 +296,15 @@ export async function approveCommand( globalOptions = {}, deps = {} ) { - return await runReviewMutation({ + return await runComparisonDecision({ command: 'approve', - endpoint: `/api/sdk/comparisons/${comparisonId}/approve`, + comparisonId, + decision: 'approved', + annotation: createApprovalBody(options).annotation, failureMessage: 'Failed to approve comparison', globalOptions, options, - requestBody: createApprovalBody(options), spinnerMessage: 'Approving comparison...', - writeJsonError: (output, error) => - output.data({ - approved: false, - comparisonId, - error: { message: error.message, code: error.code }, - }), - writeJsonSuccess: (output, response) => - output.data({ - approved: true, - comparisonId, - comparison: response.comparison, - }), writeHumanSuccess: output => { output.complete(`Comparison ${comparisonId} approved`); if (options.comment) { @@ -224,27 +339,15 @@ export async function rejectCommand( return; } - return await runReviewMutation({ + return await runComparisonDecision({ command: 'reject', - endpoint: `/api/sdk/comparisons/${comparisonId}/reject`, + comparisonId, + decision: 'rejected', + annotation: createRejectionBody(options).annotation, failureMessage: 'Failed to reject comparison', globalOptions, options, - requestBody: createRejectionBody(options), spinnerMessage: 'Rejecting comparison...', - writeJsonError: (output, error) => - output.data({ - rejected: false, - comparisonId, - error: { message: error.message, code: error.code }, - }), - writeJsonSuccess: (output, response) => - output.data({ - rejected: true, - comparisonId, - reason: options.reason, - comparison: response.comparison, - }), writeHumanSuccess: output => { output.complete(`Comparison ${comparisonId} rejected`); output.hint(`Reason: "${options.reason}"`); diff --git a/src/commands/run.js b/src/commands/run.js index ee4cabc0..1cc43b8c 100644 --- a/src/commands/run.js +++ b/src/commands/run.js @@ -544,9 +544,7 @@ export async function runCommand( if (Object.keys(comparisons).length > 0) { jsonResult.comparisons = comparisons; } - if (buildResult.approvalStatus !== undefined) { - jsonResult.approvalStatus = buildResult.approvalStatus; - } + jsonResult.visual_review = buildResult.visual_review; if (buildResult.build?.conclusion !== undefined) { jsonResult.conclusion = buildResult.build.conclusion; } diff --git a/src/commands/status.js b/src/commands/status.js index a1e5d0d2..b5145328 100644 --- a/src/commands/status.js +++ b/src/commands/status.js @@ -56,87 +56,38 @@ async function fetchOptionalPreviewInfo(getPreviewInfo, client, buildId) { } export function normalizeBuildStatus(buildStatus) { - return buildStatus.build || buildStatus; + return buildStatus.build; } /** - * Read exact processing counts from canonical or legacy status responses. + * Read the processing counts from the status response. * - * Legacy `pending_screenshots` is review state, not queue state, so it is - * deliberately excluded. Missing processing fields remain missing instead of - * becoming client-authored zeroes. - * - * @param {Object} status - Canonical status bundle or legacy build record. - * @returns {Object|undefined} API-provided processing facts when available. + * @param {Object} status - Status response. + * @returns {Object} API-provided processing facts. */ export function getProcessingStatus(status = {}) { - if (status.processing) { - return status.processing; - } - - let build = normalizeBuildStatus(status); - let processing = {}; - let fields = [ - ['total', build.screenshot_count], - ['completed', build.completed_jobs], - ['failed', build.failed_jobs], - ['active', build.processing_screenshots], - ]; - - for (let [name, value] of fields) { - if (value != null) { - processing[name] = value; - } - } - - return Object.keys(processing).length > 0 ? processing : undefined; + return status.processing; } /** * Read comparison totals without deriving them from screenshot counts. * - * @param {Object} status - Canonical status bundle or legacy build record. - * @returns {Object|undefined} API-provided comparison facts when available. + * @param {Object} status - Status response. + * @returns {Object} API-provided comparison facts. */ export function getComparisonStatus(status = {}) { - if (status.comparisons) { - return status.comparisons; - } - - let build = normalizeBuildStatus(status); - let comparisons = {}; - let fields = [ - ['total', build.total_comparisons], - ['new', build.new_comparisons], - ['changed', build.changed_comparisons], - ['identical', build.identical_comparisons], - ]; - - for (let [name, value] of fields) { - if (value != null) { - comparisons[name] = value; - } - } - - return Object.keys(comparisons).length > 0 ? comparisons : undefined; + return status.comparisons; } /** - * Prefer canonical Cricket review state while retaining the legacy fallback. + * Read the build review state without mixing it with processing status. * - * Processing status stays separate: a pending review never means a screenshot - * is still being processed. - * - * @param {Object} status - Canonical status bundle or legacy build record. + * @param {Object} status - Status response. * @returns {string|null} API-provided review state when available. */ export function getBuildReviewState(status = {}) { let build = normalizeBuildStatus(status); - return ( - getVisualReviewState(build) || - getVisualReviewState(status.visualReview?.build || status.visual_review) || - null - ); + return getVisualReviewState(build); } /** @@ -163,13 +114,10 @@ export function createStatusSuggestedCommands(build = {}) { } /** - * Preserve the established JSON fields while exposing canonical status facts. - * * Every lifecycle, processing, comparison, and review value comes directly - * from the API. Undefined values intentionally disappear during JSON encoding - * rather than being presented as false certainty. + * from the API. * - * @param {Object} status - Canonical status bundle or legacy build record. + * @param {Object} status - Status response. * @param {Object|null} previewInfo - Optional preview response. * @returns {Object} Machine-readable status payload. */ @@ -177,8 +125,6 @@ export function createStatusData(status, previewInfo = null) { let build = normalizeBuildStatus(status); let processing = getProcessingStatus(status); let comparisons = getComparisonStatus(status); - let visualReview = - build.visual_review || status.visualReview?.build || status.visual_review; return { resource: status.resource, @@ -194,18 +140,14 @@ export function createStatusData(status, previewInfo = null) { branch: build.branch, commit: build.commit_sha, commitMessage: build.commit_message, - screenshotsTotal: processing?.total ?? build.screenshot_count, + screenshotsTotal: processing.total, processing, - comparisonsTotal: comparisons?.total ?? build.total_comparisons, + comparisonsTotal: comparisons.total, comparisons, - newComparisons: comparisons?.new ?? build.new_comparisons, - changedComparisons: comparisons?.changed ?? build.changed_comparisons, - identicalComparisons: comparisons?.identical ?? build.identical_comparisons, - reviewState: getBuildReviewState(status), - review: status.review, - reviewFlow: status.reviewFlow, - visualReview, - approvalStatus: build.approval_status, + newComparisons: comparisons.new, + changedComparisons: comparisons.changed, + identicalComparisons: comparisons.identical, + visual_review: build.visual_review || null, executionTime: build.execution_time_ms, isBaseline: build.is_baseline, userAgent: build.user_agent, @@ -242,19 +184,18 @@ export function createBuildInfo(build) { } /** - * Format API comparison outcomes without filling missing buckets with zeroes. + * Format API comparison outcomes. * - * @param {Object} status - Canonical status bundle or legacy build record. + * @param {Object} status - Status response. * @param {Object} colors - Output color helpers. * @returns {string} Human-readable comparison summary. */ export function createComparisonStats(status, colors) { - let build = normalizeBuildStatus(status); - let comparisons = getComparisonStatus(status) || {}; + let comparisons = getComparisonStatus(status); let stats = []; - let newCount = comparisons.new ?? build.new_comparisons; - let changedCount = comparisons.changed ?? build.changed_comparisons; - let identicalCount = comparisons.identical ?? build.identical_comparisons; + let newCount = comparisons.new; + let changedCount = comparisons.changed; + let identicalCount = comparisons.identical; if (newCount > 0) { stats.push(`${colors.brand.info(newCount)} new`); @@ -270,14 +211,11 @@ export function createComparisonStats(status, colors) { } /** - * Build the best available legacy link when the API did not return one. - * - * Slug routes match the current app, while the project-ID route remains as a - * compatibility fallback for older build responses. + * Build the scoped app link when the API did not return one. * * @param {string} baseUrl - API or app base URL. * @param {Object} build - Build record. - * @param {Object|null} scope - Canonical organization and project scope. + * @param {Object|null} scope - Organization and project details. * @returns {string|null} Build URL when the response has enough identity. */ export function createBuildUrl(baseUrl, build, scope = null) { @@ -285,29 +223,24 @@ export function createBuildUrl(baseUrl, build, scope = null) { return null; } - let organizationSlug = - scope?.organization?.slug || build.organization_slug || build.org_slug; - let projectSlug = scope?.project?.slug || build.project_slug; + let organizationSlug = scope?.organization?.slug; + let projectSlug = scope?.project?.slug; let appBaseUrl = getAppBaseUrl(baseUrl); if (organizationSlug && projectSlug) { return `${appBaseUrl}/${organizationSlug}/${projectSlug}/builds/${build.id}`; } - if (build.project_id) { - return `${appBaseUrl}/projects/${build.project_id}/builds/${build.id}`; - } - return null; } /** - * Preserve status failure behavior while accepting the canonical conclusion. + * Preserve status failure behavior while using the server's conclusion. * * Review-required and rejected builds keep their existing successful command * exit behavior. Only build or processing failures produce a failing status. * - * @param {Object} status - Canonical status bundle or legacy build record. + * @param {Object} status - Status response. * @returns {boolean} Whether human status should exit non-zero. */ export function shouldFailStatus(status) { @@ -316,7 +249,7 @@ export function shouldFailStatus(status) { return ( build.status === 'failed' || ['build_failed', 'processing_failed'].includes(status.conclusion) || - (processing?.failed ?? 0) > 0 + processing.failed > 0 ); } @@ -365,7 +298,7 @@ function writeSuggestedCommands({ build, output }) { /** * Add optional API facts without inventing placeholders for missing metadata. * - * @param {Object} status - Canonical status bundle or legacy build record. + * @param {Object} status - Status response. * @returns {Object} Available verbose fields. */ function createVerboseInfo(status) { @@ -412,7 +345,7 @@ function createVerboseInfo(status) { /** * Format only the processing facts supplied by the API. * - * @param {Object|undefined} processing - Canonical processing counts. + * @param {Object|undefined} processing - Processing counts. * @returns {string} Human-readable processing summary. */ function formatProcessingStatus(processing) { @@ -457,7 +390,7 @@ function writeHumanStatus({ let colors = output.getColors(); let comparisonStats = createComparisonStats(status, colors); let processing = getProcessingStatus(status); - let screenshotsTotal = processing?.total ?? build.screenshot_count; + let screenshotsTotal = processing.total; let processingSummary = formatProcessingStatus(processing); if (screenshotsTotal != null) { diff --git a/src/commands/upload.js b/src/commands/upload.js index 280b33f1..49c1643b 100644 --- a/src/commands/upload.js +++ b/src/commands/upload.js @@ -310,7 +310,7 @@ export async function uploadCommand( failed: buildResult.failedComparisons || 0, new: buildResult.newComparisons || 0, }, - approvalStatus: buildResult.approvalStatus || 'pending', + visual_review: buildResult.visual_review, executionTimeMs, }); output.cleanup(); diff --git a/src/context/local-workspace-provider.js b/src/context/local-workspace-provider.js index bfb5d7b8..d80d3793 100644 --- a/src/context/local-workspace-provider.js +++ b/src/context/local-workspace-provider.js @@ -53,7 +53,7 @@ function mapComparisonResult(status) { return status || 'unknown'; } -function mapApprovalStatus(status) { +function mapReviewState(status) { if (status === 'failed' || status === 'new') { return 'pending'; } @@ -110,67 +110,6 @@ function resolveAssetReference(assetPath, snapshot) { return assetPath; } -function normalizeConfirmedRegions(regions = []) { - return regions.map((region, index) => ({ - id: region.id || `local-region-${index}`, - x1: region.x1 ?? region.x ?? null, - y1: region.y1 ?? region.y ?? null, - x2: - region.x2 ?? - (region.x != null && region.width != null - ? region.x + region.width - : null), - y2: - region.y2 ?? - (region.y != null && region.height != null - ? region.y + region.height - : null), - label: region.label || null, - })); -} - -function mergeConfirmedRegions(snapshot, comparisonName, details = {}) { - let workspaceRegions = snapshot.regions?.[comparisonName]?.confirmed || []; - let detailRegions = details.confirmedRegions || []; - let merged = [...workspaceRegions, ...detailRegions]; - let seen = new Set(); - - return normalizeConfirmedRegions(merged).filter(region => { - let key = `${region.label || ''}:${region.x1}:${region.y1}:${region.x2}:${region.y2}`; - - if (seen.has(key)) { - return false; - } - - seen.add(key); - return true; - }); -} - -function buildHotspotAnalysis(snapshot, comparisonName, details = {}) { - let hotspotMetadata = snapshot.hotspots?.[comparisonName] || null; - let hotspotAnalysis = details.hotspotAnalysis || null; - - if (!hotspotMetadata && !hotspotAnalysis) { - return { - regions: [], - total_builds_analyzed: 0, - confidence: 'no_data', - confidence_score: null, - data_source: 'local_workspace', - }; - } - - return { - regions: hotspotMetadata?.regions || [], - total_builds_analyzed: 1, - confidence: - hotspotAnalysis?.confidence || hotspotMetadata?.confidence || 'workspace', - confidence_score: hotspotAnalysis?.confidenceScore || null, - data_source: 'local_workspace', - }; -} - function buildComparisonLinks(snapshot, comparisonId) { if (!snapshot.serverInfo?.port) { return {}; @@ -201,7 +140,7 @@ function buildBuildSnapshot(snapshot) { branch: 'local', commit_sha: null, commit_message: null, - approval_status: snapshot.serverInfo ? 'pending' : 'approved', + review_state: snapshot.serverInfo ? 'pending' : 'approved', status: snapshot.serverInfo ? 'running' : 'completed', created_at: null, }; @@ -225,7 +164,7 @@ function buildBaselineSnapshot(snapshot) { branch: metadata.branch || 'local', commit_sha: metadata.buildInfo?.commitSha || null, commit_message: metadata.buildInfo?.commitMessage || null, - approval_status: metadata.buildInfo?.approvalStatus || 'approved', + review_state: metadata.buildInfo?.visual_review?.state || 'approved', status: metadata.buildInfo?.completedAt ? 'completed' : 'local', created_at: metadata.createdAt || null, completed_at: metadata.buildInfo?.completedAt || null, @@ -238,7 +177,7 @@ function buildBaselineSnapshot(snapshot) { function buildReviewState(build, reviewSummary) { let reasons = []; - if (build.approval_status === 'pending') { + if (build.review_state === 'pending') { reasons.push('build_pending_approval'); } @@ -257,16 +196,10 @@ function buildReviewState(build, reviewSummary) { function mapLocalComparison(snapshot, comparison) { let details = snapshot.comparisonDetails[comparison.id] || {}; let comparisonName = comparison.originalName || comparison.name; - let confirmedRegions = mergeConfirmedRegions( - snapshot, - comparisonName, - details - ); - let hotspotAnalysis = buildHotspotAnalysis(snapshot, comparisonName, details); let properties = comparison.properties || {}; let buildSnapshot = buildBuildSnapshot(snapshot); let result = mapComparisonResult(comparison.status); - let approvalStatus = mapApprovalStatus(comparison.status); + let reviewState = mapReviewState(comparison.status); let baselineBuildId = snapshot.baselineMetadata?.buildId || null; let diffImageUrl = resolveAssetReference(comparison.diff, snapshot); let diffRegions = details.diffClusters || []; @@ -277,9 +210,9 @@ function mapLocalComparison(snapshot, comparison) { screenshot_name: comparisonName, status: comparison.status, result, - approval_status: approvalStatus, + review_state: reviewState, needs_review: - approvalStatus === 'pending' && ['changed', 'new'].includes(result), + reviewState === 'pending' && ['changed', 'new'].includes(result), build_id: buildSnapshot.id, build_name: buildSnapshot.name, build_branch: buildSnapshot.branch, @@ -337,14 +270,11 @@ function mapLocalComparison(snapshot, comparison) { diff_lines: null, fingerprint_hash: null, fingerprint_data: null, - hotspot_analysis: hotspotAnalysis, - region_analysis: details.regionAnalysis || null, - confirmed_regions: confirmedRegions, }, }; } -function projectLocalScreenshot(screenshot, baseline) { +function formatLocalScreenshot(screenshot, baseline) { if (!screenshot) return null; return { @@ -372,8 +302,8 @@ function projectLocalScreenshot(screenshot, baseline) { }; } -function projectLocalDiff(diff, includeDiffs) { - let projected = { +function formatLocalDiff(diff, includeDiffs) { + let formatted = { percentage: diff?.percentage ?? null, changed_pixels: diff?.changed_pixels ?? null, total_pixels: diff?.total_pixels ?? null, @@ -384,38 +314,38 @@ function projectLocalDiff(diff, includeDiffs) { }; if (includeDiffs) { - projected.regions = diff?.regions || []; - projected.cluster_metadata = diff?.cluster_metadata ?? null; - projected.fingerprint_data = diff?.fingerprint_data ?? null; - projected.diff_lines = diff?.diff_lines ?? []; + formatted.regions = diff?.regions || []; + formatted.cluster_metadata = diff?.cluster_metadata ?? null; + formatted.fingerprint_data = diff?.fingerprint_data ?? null; + formatted.diff_lines = diff?.diff_lines ?? []; } - return projected; + return formatted; } -function projectLocalEvidence(comparison, includeDiffs) { +function formatLocalEvidence(comparison, includeDiffs) { return { type: 'comparison', id: comparison.id, screenshot_name: comparison.screenshot_name, status: comparison.status, result: comparison.result, - approval_status: comparison.approval_status, + review_state: comparison.review_state, needs_review: comparison.needs_review, build_id: comparison.build_id, build_name: comparison.build_name, build_branch: comparison.build_branch, build_commit_sha: comparison.build_commit_sha, build_created_at: comparison.build_created_at, - screenshot: projectLocalScreenshot( + screenshot: formatLocalScreenshot( comparison.screenshot, comparison.baseline ), - diff: projectLocalDiff(comparison.diff, includeDiffs), + diff: formatLocalDiff(comparison.diff, includeDiffs), }; } -function projectLocalFocusedComparison(comparison, includeDiffs) { +function formatLocalFocusedComparison(comparison, includeDiffs) { if (includeDiffs) return comparison; let analysis = comparison.analysis || {}; @@ -429,12 +359,12 @@ function projectLocalFocusedComparison(comparison, includeDiffs) { return { ...comparison, - diff: projectLocalDiff(comparison.diff, false), + diff: formatLocalDiff(comparison.diff, false), analysis: summaryAnalysis, }; } -function projectLocalHistoryItem(comparison) { +function formatLocalHistoryItem(comparison) { return { id: comparison.id, screenshot_name: comparison.screenshot_name, @@ -445,7 +375,7 @@ function projectLocalHistoryItem(comparison) { build_name: comparison.build_name, build_branch: comparison.build_branch, build_created_at: comparison.build_created_at, - screenshot: projectLocalScreenshot( + screenshot: formatLocalScreenshot( comparison.screenshot, comparison.baseline ), @@ -467,41 +397,12 @@ function compactLocalCollection(items) { }; } -function createLocalDynamicRegions(comparison = null) { - let confirmedRegions = comparison?.analysis?.confirmed_regions; - let hotspotAnalysis = comparison?.analysis?.hotspot_analysis; - - return { - decision: null, - patterns: compactLocalCollection([]), - confirmed_regions: compactLocalCollection(confirmedRegions), - exclusions: { - total: null, - included: false, - details_available: false, - }, - hotspot_analysis: hotspotAnalysis - ? { - total_builds_analyzed: hotspotAnalysis.total_builds_analyzed ?? null, - confidence: hotspotAnalysis.confidence ?? null, - confidence_score: hotspotAnalysis.confidence_score ?? null, - data_source: hotspotAnalysis.data_source ?? null, - coverage: hotspotAnalysis.coverage ?? null, - confirmed_region_coverage: - hotspotAnalysis.confirmed_region_coverage ?? null, - } - : null, - }; -} - function createLocalSnapshotRevision(snapshot) { let revisionInput = { serverInfo: snapshot.serverInfo, reportData: snapshot.reportData, comparisonDetails: snapshot.comparisonDetails, baselineMetadata: snapshot.baselineMetadata, - hotspotFile: snapshot.hotspotFile, - regionFile: snapshot.regionFile, }; return createHash('sha256') @@ -511,13 +412,13 @@ function createLocalSnapshotRevision(snapshot) { function buildReviewSummary(comparisons = []) { let approved = comparisons.filter( - comparison => mapApprovalStatus(comparison.status) === 'approved' + comparison => mapReviewState(comparison.status) === 'approved' ).length; let rejected = comparisons.filter( - comparison => mapApprovalStatus(comparison.status) === 'rejected' + comparison => mapReviewState(comparison.status) === 'rejected' ).length; let pending = comparisons.filter( - comparison => mapApprovalStatus(comparison.status) === 'pending' + comparison => mapReviewState(comparison.status) === 'pending' ).length; return { @@ -567,12 +468,7 @@ export function createLocalWorkspaceContextProvider(options = {}, deps = {}) { comparisonDetails: readJson(join(vizzlyDir, 'comparison-details.json')) || {}, baselineMetadata: readJson(join(vizzlyDir, 'baselines', 'metadata.json')), - hotspotFile: readJson(join(vizzlyDir, 'hotspots.json')), - regionFile: readJson(join(vizzlyDir, 'regions.json')), }; - - snapshotCache.hotspots = snapshotCache.hotspotFile?.hotspots || null; - snapshotCache.regions = snapshotCache.regionFile?.regions || null; return snapshotCache; } @@ -621,9 +517,7 @@ export function createLocalWorkspaceContextProvider(options = {}, deps = {}) { return Boolean( snapshot.reportData.comparisons.some( comparison => (comparison.originalName || comparison.name) === target - ) || - snapshot.regions?.[target] || - snapshot.hotspots?.[target] + ) ); } @@ -760,7 +654,7 @@ export function createLocalWorkspaceContextProvider(options = {}, deps = {}) { mapLocalComparison(snapshot, comparison) ); let mappedScreenshots = mappedComparisons.map(comparison => - projectLocalScreenshot(comparison.screenshot, comparison.baseline) + formatLocalScreenshot(comparison.screenshot, comparison.baseline) ); let reviewSummary = buildReviewSummary(snapshot.reportData.comparisons); let reviewState = buildReviewState(resolvedBuild, reviewSummary); @@ -816,7 +710,7 @@ export function createLocalWorkspaceContextProvider(options = {}, deps = {}) { ); let includeDiffs = query.details === 'diffs'; let evidence = mappedComparisons.map(comparison => - projectLocalEvidence(comparison, includeDiffs) + formatLocalEvidence(comparison, includeDiffs) ); return { @@ -829,14 +723,6 @@ export function createLocalWorkspaceContextProvider(options = {}, deps = {}) { summary: context.summary, preview: null, signature_properties: [], - dynamic_regions: { - exclusions: { - total: null, - included: false, - details_available: false, - }, - item_details_included: false, - }, evidence: createLocalPage(evidence, { limit, offset: cursor?.offset || 0, @@ -883,16 +769,6 @@ export function createLocalWorkspaceContextProvider(options = {}, deps = {}) { history: { similar_by_fingerprint: [], recent_by_name: history, - hotspot_analysis: buildHotspotAnalysis( - snapshot, - comparisonName, - snapshot.comparisonDetails[comparison.id] || {} - ), - confirmed_regions: mergeConfirmedRegions( - snapshot, - comparisonName, - snapshot.comparisonDetails[comparison.id] || {} - ), }, review: { review_summary: { @@ -936,11 +812,10 @@ export function createLocalWorkspaceContextProvider(options = {}, deps = {}) { scope: context.scope, build: context.build, signature_properties: [], - comparison: projectLocalFocusedComparison( + comparison: formatLocalFocusedComparison( context.comparison, query.details === 'diffs' ), - dynamic_regions: createLocalDynamicRegions(context.comparison), history: { active_stream: activeStream || null, similar_by_fingerprint: createLocalPage([], { @@ -952,7 +827,7 @@ export function createLocalWorkspaceContextProvider(options = {}, deps = {}) { stream: 'similar_by_fingerprint', revision, }), - recent_by_name: createLocalPage(history.map(projectLocalHistoryItem), { + recent_by_name: createLocalPage(history.map(formatLocalHistoryItem), { limit, offset: recentOffset, resource: 'comparison_context', @@ -984,11 +859,7 @@ export function createLocalWorkspaceContextProvider(options = {}, deps = {}) { ) .map(comparison => mapLocalComparison(snapshot, comparison)); - if ( - matches.length === 0 && - !snapshot.regions?.[screenshotName] && - !snapshot.hotspots?.[screenshotName] - ) { + if (matches.length === 0) { throw createLocalWorkspaceError( `No local screenshot context found for "${screenshotName}"` ); @@ -1001,8 +872,6 @@ export function createLocalWorkspaceContextProvider(options = {}, deps = {}) { screenshot: { name: screenshotName, }, - hotspot_analysis: buildHotspotAnalysis(snapshot, screenshotName), - confirmed_regions: mergeConfirmedRegions(snapshot, screenshotName), history: { recent_comparisons: matches, }, diff --git a/src/reporter/src/components/comparison/fullscreen-viewer.jsx b/src/reporter/src/components/comparison/fullscreen-viewer.jsx index 45b40544..032efd20 100644 --- a/src/reporter/src/components/comparison/fullscreen-viewer.jsx +++ b/src/reporter/src/components/comparison/fullscreen-viewer.jsx @@ -20,7 +20,6 @@ import { ExclamationTriangleIcon, InformationCircleIcon, ListBulletIcon, - MapPinIcon, } from '@heroicons/react/24/outline'; import { Badge } from '@vizzly-testing/bear-den'; import { @@ -104,7 +103,6 @@ function FullscreenViewerInner({ let [showInspector, setShowInspector] = useState(false); let [queueFilter, setQueueFilter] = useState('needs-review'); let [showBaseline, setShowBaseline] = useState(true); - let [showRegions, setShowRegions] = useState(false); let { zoom, setZoom } = useZoom('fit'); let { isActive: isReviewMode } = useReviewMode(); @@ -119,7 +117,7 @@ function FullscreenViewerInner({ // Transform comparisons for queue display // Map CLI status to BearDen result format - // QueueItem expects: name, result, approval_status, diff_percentage, status + // Queue items use the local review result, independent of cloud review state. let queueItems = useMemo(() => { return comparisons.map(comp => ({ ...comp, @@ -129,8 +127,7 @@ function FullscreenViewerInner({ result: mapStatusToResult(comp.status), // Keep original status so QueueItem can check both formats status: comp.status, - // Map approval_status from userAction - approval_status: + review_state: comp.userAction === 'accepted' ? 'approved' : comp.userAction === 'rejected' @@ -178,8 +175,8 @@ function FullscreenViewerInner({ 'needs-review': item => { return ( (item.result === 'changed' || item.result === 'new') && - item.approval_status !== 'approved' && - item.approval_status !== 'rejected' + item.review_state !== 'approved' && + item.review_state !== 'rejected' ); }, changes: item => item.result === 'changed', @@ -356,12 +353,6 @@ function FullscreenViewerInner({ toggleInspector(); } break; - case 'g': - if (!e.metaKey && !e.ctrlKey) { - e.preventDefault(); - setShowRegions(prev => !prev); - } - break; } }; @@ -414,9 +405,9 @@ function FullscreenViewerInner({ ); } - // Determine result and approval status for current comparison + // Determine the local review result for the current comparison. let result = mapStatusToResult(comparison.status); - let approvalStatus = + let reviewState = comparison.userAction === 'accepted' ? 'approved' : comparison.userAction === 'rejected' @@ -517,7 +508,7 @@ function FullscreenViewerInner({ ) : null} - {/* Regions toggle - only show if comparison has regions */} - {(comparison?.confirmedRegions?.length > 0 || - comparison?.hasConfirmedRegions) && ( - - )} - - {/* Regions toggle - mobile */} - {(comparison?.confirmedRegions?.length > 0 || - comparison?.hasConfirmedRegions) && ( - - )} - {canDelete && onDelete && (