From a68f7a080c762fef18c68a831a67719e91953d35 Mon Sep 17 00:00:00 2001 From: Gregory Schier Date: Fri, 6 Nov 2026 09:57:34 -0700 Subject: [PATCH] Only show a version comparison for the response it belongs to The comparison query kept its last answer as placeholder data across version changes, so switching to a response with no version, or to one whose comparison was still loading, could leave the Request Changed dropdown offering another response's diff and restore. Placeholder data now carries over only within the same version, and the hook returns a comparison only when it is for the version asked about. Co-Authored-By: Claude Opus 5.5 --- .../components/RequestVersionDropdown.tsx | 2 +- .../hooks/useRequestVersion.test.tsx | 133 ++++++++++++++++++ apps/yaak-client/hooks/useRequestVersion.ts | 17 ++- 3 files changed, 148 insertions(+), 4 deletions(-) create mode 100644 apps/yaak-client/hooks/useRequestVersion.test.tsx diff --git a/apps/yaak-client/components/RequestVersionDropdown.tsx b/apps/yaak-client/components/RequestVersionDropdown.tsx index 4336d511a..cc8e97677 100644 --- a/apps/yaak-client/components/RequestVersionDropdown.tsx +++ b/apps/yaak-client/components/RequestVersionDropdown.tsx @@ -21,7 +21,7 @@ interface Props { * existed have no version and stay quiet forever. */ export function RequestVersionDropdown({ response }: Props) { - const { data: comparison } = useRequestVersion(response.versionId, response.requestId); + const comparison = useRequestVersion(response.versionId, response.requestId); if (comparison == null || !comparison.differs) { return null; } diff --git a/apps/yaak-client/hooks/useRequestVersion.test.tsx b/apps/yaak-client/hooks/useRequestVersion.test.tsx new file mode 100644 index 000000000..831db4e00 --- /dev/null +++ b/apps/yaak-client/hooks/useRequestVersion.test.tsx @@ -0,0 +1,133 @@ +// @vitest-environment happy-dom +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import type { RequestVersionComparison } from "@yaakapp-internal/models"; +import { atom, getDefaultStore } from "jotai"; +import { act } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { afterEach, beforeEach, describe, expect, test, vi } from "vite-plus/test"; + +const mocks = vi.hoisted(() => ({ + rpc: vi.fn<(cmd: string, args: { versionId: string }) => Promise>(), +})); + +vi.mock("../lib/rpc", () => ({ rpc: mocks.rpc })); +vi.mock("./useAllRequests", async () => { + const { atom } = await import("jotai"); + return { allRequestsAtom: atom([{ id: "rq_1", updatedAt: "t1" }]) }; +}); + +function comparison(versionId: string): RequestVersionComparison { + return { + version: { id: versionId, modelId: "rq_1" }, + currentDocument: {}, + differs: true, + } as unknown as RequestVersionComparison; +} + +/** Each version's comparison resolves only when the test says so. */ +function deferredRpc() { + const pending = new Map void>(); + mocks.rpc.mockImplementation( + (_cmd, { versionId }) => + new Promise((resolve) => pending.set(versionId, () => resolve(comparison(versionId)))), + ); + return async (versionId: string) => { + await vi.waitFor(() => expect(pending.has(versionId)).toBe(true)); + await act(async () => pending.get(versionId)?.()); + }; +} + +describe("useRequestVersion", () => { + let container: HTMLDivElement; + let root: Root; + let seen: (RequestVersionComparison | null)[]; + let useRequestVersion: typeof import("./useRequestVersion").useRequestVersion; + let allRequestsAtom: ReturnType>; + const queryClient = () => new QueryClient({ defaultOptions: { queries: { retry: false } } }); + let client: QueryClient; + + function Probe({ versionId }: { versionId: string | null }) { + seen.push(useRequestVersion(versionId, "rq_1")); + return null; + } + + async function show(versionId: string | null) { + await act(async () => + root.render( + + + , + ), + ); + } + + const current = () => seen[seen.length - 1]; + + /** Wait for the comparison for a version to reach the component. */ + async function answered(versionId: string) { + await vi.waitFor(() => expect(current()?.version.id).toBe(versionId)); + } + + beforeEach(async () => { + (globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + ({ useRequestVersion } = await import("./useRequestVersion")); + ({ allRequestsAtom } = (await import("./useAllRequests")) as unknown as { + allRequestsAtom: typeof allRequestsAtom; + }); + getDefaultStore().set(allRequestsAtom, [{ id: "rq_1", updatedAt: "t1" }]); + client = queryClient(); + seen = []; + container = document.createElement("div"); + root = createRoot(container); + }); + + afterEach(async () => { + await act(async () => root.unmount()); + client.clear(); + mocks.rpc.mockReset(); + }); + + test("a response with no version has no comparison, even right after one that had", async () => { + const resolve = deferredRpc(); + await show("mv_a"); + await resolve("mv_a"); + await answered("mv_a"); + + seen = []; + await show(null); + expect(seen.length).toBeGreaterThan(0); + expect(seen.every((c) => c == null)).toBe(true); + }); + + test("switching versions never shows the previous version's comparison", async () => { + const resolve = deferredRpc(); + await show("mv_a"); + await resolve("mv_a"); + await answered("mv_a"); + + seen = []; + await show("mv_b"); + // Still loading: nothing, rather than mv_a's answer + expect(seen.length).toBeGreaterThan(0); + expect(seen.every((c) => c == null)).toBe(true); + + await resolve("mv_b"); + await answered("mv_b"); + }); + + test("an edit to the live request keeps the answer while it refetches", async () => { + const resolve = deferredRpc(); + await show("mv_a"); + await resolve("mv_a"); + await answered("mv_a"); + + seen = []; + await act(async () => + getDefaultStore().set(allRequestsAtom, [{ id: "rq_1", updatedAt: "t2" }]), + ); + // The refetch is pending, but the dropdown does not flicker away meanwhile + expect(seen.length).toBeGreaterThan(0); + expect(seen.every((c) => c?.version.id === "mv_a")).toBe(true); + expect(mocks.rpc).toHaveBeenCalledTimes(2); + }); +}); diff --git a/apps/yaak-client/hooks/useRequestVersion.ts b/apps/yaak-client/hooks/useRequestVersion.ts index b2d3fd232..7de286522 100644 --- a/apps/yaak-client/hooks/useRequestVersion.ts +++ b/apps/yaak-client/hooks/useRequestVersion.ts @@ -11,16 +11,27 @@ import { rpc } from "../lib/rpc"; * Refetches when the live request is written, which is what keeps the "has this * changed?" answer honest while someone edits. The comparison itself is the * backend's — the frontend never hashes anything. + * + * Only ever answers for the version asked about: a response with no version + * gets nothing, and switching responses never shows the last one's comparison + * while the new one loads. */ -export function useRequestVersion(versionId: string | null | undefined, requestId: string | null) { +export function useRequestVersion( + versionId: string | null | undefined, + requestId: string | null, +): RequestVersionComparison | null { const requests = useAtomValue(allRequestsAtom); const liveUpdatedAt = requests.find((r) => r.id === requestId)?.updatedAt; - return useQuery({ - placeholderData: (prev) => prev, + const { data } = useQuery({ + // Keep the last answer across edits to the live request, but never across + // versions, or a response would borrow another response's comparison + placeholderData: (prev, prevQuery) => (prevQuery?.queryKey[1] === versionId ? prev : undefined), queryKey: ["request_version", versionId, liveUpdatedAt], enabled: versionId != null, queryFn: () => rpc("models_request_version", { versionId: versionId! }), }); + + return versionId != null && data?.version.id === versionId ? data : null; }