From 16c8d34f31d0727ff7b0b6853e00c30626a77977 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9CVandit?= Date: Tue, 11 Aug 2026 11:56:47 +0530 Subject: [PATCH] fix: clear stale compare sync offsets on version switch offA/offB were calibrated for the previously-selected version pair. Switching either pane's version left them in place, silently misapplying an unrelated sync offset to the new pair and re-thrashing the decoder to hold a wrong alignment. Fixes #182 --- .../__tests__/compare-overlay.test.tsx | 44 ++++++++++++++++++- .../review/compare/compare-overlay.tsx | 21 ++++++++- 2 files changed, 62 insertions(+), 3 deletions(-) diff --git a/apps/web/components/review/compare/__tests__/compare-overlay.test.tsx b/apps/web/components/review/compare/__tests__/compare-overlay.test.tsx index 3e76f977..cfdb2220 100644 --- a/apps/web/components/review/compare/__tests__/compare-overlay.test.tsx +++ b/apps/web/components/review/compare/__tests__/compare-overlay.test.tsx @@ -1,5 +1,5 @@ import { describe, expect, it, vi, beforeEach } from 'vitest' -import { fireEvent, render, screen, waitFor } from '@testing-library/react' +import { fireEvent, render, screen, waitFor, within } from '@testing-library/react' import { useReviewStore } from '@/stores/review-store' const replace = vi.fn() @@ -315,6 +315,48 @@ describe('CompareOverlay marker click', () => { }) }) +describe('CompareOverlay version-switch offset reset (#182)', () => { + it('switching the left version clears offA/offB (calibrated for the OLD pair)', () => { + // Pre-existing sync offsets from calibrating the v1/v3 pair. + searchParamsString = 'compare=v-1&offA=1.5&offB=-0.5' + render( + , + ) + + fireEvent.click(within(screen.getByTestId('compare-select-a')).getByRole('button')) + fireEvent.click(screen.getByRole('option', { name: /^v2$/ })) + + const url = new URL(replace.mock.calls.at(-1)?.[0], 'http://x') + expect(url.searchParams.get('offA')).toBeNull() + expect(url.searchParams.get('offB')).toBeNull() + expect(url.searchParams.get('compare')).toBe('v-2') + }) + + it('switching the right version clears offA/offB too', () => { + searchParamsString = 'compare=v-1&offA=1.5&offB=-0.5' + render( + , + ) + + fireEvent.click(within(screen.getByTestId('compare-select-b')).getByRole('button')) + fireEvent.click(screen.getByRole('option', { name: /^v2$/ })) + + const url = new URL(replace.mock.calls.at(-1)?.[0], 'http://x') + expect(url.searchParams.get('offA')).toBeNull() + expect(url.searchParams.get('offB')).toBeNull() + }) +}) + describe('CompareOverlay per-pane annotation display', () => { const DRAWING = { objects: [], _canvasWidth: 640, _canvasHeight: 360 } diff --git a/apps/web/components/review/compare/compare-overlay.tsx b/apps/web/components/review/compare/compare-overlay.tsx index 1b50f470..2a54f307 100644 --- a/apps/web/components/review/compare/compare-overlay.tsx +++ b/apps/web/components/review/compare/compare-overlay.tsx @@ -279,6 +279,23 @@ export function CompareOverlay({ asset, versions, rightVersion, onClose, canComm [sideB], ) + // Version switch (either pane): offA/offB were calibrated for the OLD pair — + // they no longer describe the new one, so drop them rather than silently + // misapplying a stale sync offset to the new pair (#182). + const handleSwitchLeft = React.useCallback( + (v: AssetVersion) => { + writeParams((p) => { p.set('compare', v.id); p.delete('offA'); p.delete('offB') }) + }, + [writeParams], + ) + const handleSwitchRight = React.useCallback( + (v: AssetVersion) => { + writeParams((p) => { p.delete('offA'); p.delete('offB') }) + setCurrentVersion(v) + }, + [writeParams, setCurrentVersion], + ) + // Shared zoom/pan for image modes const transform = useSharedTransform() @@ -310,7 +327,7 @@ export function CompareOverlay({ asset, versions, rightVersion, onClose, canComm value={left.id} excludeId={right.id} accentClass="text-sky-400" - onChange={(v) => writeParams((p) => p.set('compare', v.id))} + onChange={handleSwitchLeft} /> {/* Center: asset title (flex-1 min-w-0 truncates without pushing either @@ -338,7 +355,7 @@ export function CompareOverlay({ asset, versions, rightVersion, onClose, canComm value={right.id} excludeId={left.id} accentClass="text-emerald-400" - onChange={(v) => setCurrentVersion(v)} + onChange={handleSwitchRight} />