From 9b9828e816f022d5c24cbdac50d1558474b5cb26 Mon Sep 17 00:00:00 2001 From: Heinrich Xiao <74563446+Heinrich-XIAO@users.noreply.github.com> Date: Wed, 11 Mar 2026 21:29:54 -0400 Subject: [PATCH 1/3] Stop retaining diff search param in chat thread route - Remove `retainSearchParams` middleware for `diff` on `/_chat/$threadId` - Let diff query state clear naturally so collapsed diff panel does not persist unexpectedly - Clean up now-unused router imports/types --- apps/web/src/routes/_chat.$threadId.tsx | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/apps/web/src/routes/_chat.$threadId.tsx b/apps/web/src/routes/_chat.$threadId.tsx index 5e05b51fbd..0bc0bc207c 100644 --- a/apps/web/src/routes/_chat.$threadId.tsx +++ b/apps/web/src/routes/_chat.$threadId.tsx @@ -1,14 +1,10 @@ import { ThreadId } from "@t3tools/contracts"; -import { createFileRoute, retainSearchParams, useNavigate } from "@tanstack/react-router"; +import { createFileRoute, useNavigate } from "@tanstack/react-router"; import { Suspense, lazy, type ReactNode, useCallback, useEffect } from "react"; import ChatView from "../components/ChatView"; import { useComposerDraftStore } from "../composerDraftStore"; -import { - type DiffRouteSearch, - parseDiffRouteSearch, - stripDiffSearchParams, -} from "../diffRouteSearch"; +import { parseDiffRouteSearch, stripDiffSearchParams } from "../diffRouteSearch"; import { useMediaQuery } from "../hooks/useMediaQuery"; import { useStore } from "../store"; import { Sheet, SheetPopup } from "../components/ui/sheet"; @@ -226,8 +222,5 @@ function ChatThreadRouteView() { export const Route = createFileRoute("/_chat/$threadId")({ validateSearch: (search) => parseDiffRouteSearch(search), - search: { - middlewares: [retainSearchParams(["diff"])], - }, component: ChatThreadRouteView, }); From 9b2e8093fb89931210b03e5c0856f8bbecb50ce7 Mon Sep 17 00:00:00 2001 From: Heinrich Xiao <74563446+Heinrich-XIAO@users.noreply.github.com> Date: Wed, 11 Mar 2026 21:43:12 -0400 Subject: [PATCH 2/3] Fix diff close with retained search params --- apps/web/src/components/ChatView.tsx | 10 +++- apps/web/src/diffRouteSearch.test.ts | 62 ++++++++++++++++++- apps/web/src/diffRouteSearch.ts | 79 +++++++++++++++++++++++-- apps/web/src/routes/_chat.$threadId.tsx | 53 +++++++++++++++-- 4 files changed, 191 insertions(+), 13 deletions(-) diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx index 0a9c0371a1..cca51d89d0 100644 --- a/apps/web/src/components/ChatView.tsx +++ b/apps/web/src/components/ChatView.tsx @@ -36,7 +36,11 @@ import { gitBranchesQueryOptions, gitCreateWorktreeMutationOptions } from "~/lib import { projectSearchEntriesQueryOptions } from "~/lib/projectReactQuery"; import { serverConfigQueryOptions, serverQueryKeys } from "~/lib/serverReactQuery"; import { isElectron } from "../env"; -import { parseDiffRouteSearch, stripDiffSearchParams } from "../diffRouteSearch"; +import { + closeDiffSearchParams, + parseDiffRouteSearch, + stripDiffSearchParams, +} from "../diffRouteSearch"; import { type ComposerTrigger, detectComposerTrigger, @@ -1027,7 +1031,9 @@ export default function ChatView({ threadId }: ChatViewProps) { replace: true, search: (previous) => { const rest = stripDiffSearchParams(previous); - return diffOpen ? { ...rest, diff: undefined } : { ...rest, diff: "1" }; + return diffOpen + ? { ...rest, clearDiff: "1" as const } + : { ...rest, diff: "1" }; }, }); }, [diffOpen, navigate, threadId]); diff --git a/apps/web/src/diffRouteSearch.test.ts b/apps/web/src/diffRouteSearch.test.ts index ef00874bd2..2cad77ffd4 100644 --- a/apps/web/src/diffRouteSearch.test.ts +++ b/apps/web/src/diffRouteSearch.test.ts @@ -1,6 +1,14 @@ +import { stripSearchParams } from "@tanstack/react-router"; +import { TurnId } from "@t3tools/contracts"; import { describe, expect, it } from "vitest"; -import { parseDiffRouteSearch } from "./diffRouteSearch"; +import { + closeDiffSearchParams, + type DiffRouteSearchNavigation, + parseDiffRouteSearch, + parseDiffRouteSearchNavigation, + retainDiffSearchParams, +} from "./diffRouteSearch"; describe("parseDiffRouteSearch", () => { it("parses valid diff search values", () => { @@ -71,4 +79,56 @@ describe("parseDiffRouteSearch", () => { diff: "1", }); }); + + it("builds an explicit close navigation payload", () => { + expect( + closeDiffSearchParams({ + diff: "1", + diffTurnId: TurnId.makeUnsafe("turn-1"), + diffFilePath: "src/app.ts", + }), + ).toEqual({ + clearDiff: "1", + }); + }); + + it("parses route control flags for navigation middleware", () => { + expect( + parseDiffRouteSearchNavigation({ + clearDiff: "1", + }), + ).toEqual({ + clearDiff: "1", + }); + }); + + it("retains diff across navigations that do not touch diff state", () => { + expect( + retainDiffSearchParams({ + search: { + diff: "1", + }, + next: () => ({}), + }), + ).toEqual({ + diff: "1", + }); + }); + + it("does not reinsert diff when navigation explicitly clears it", () => { + const stripped = stripSearchParams(["clearDiff"]); + + const nextSearch = stripped({ + search: { + diff: "1", + }, + next: (search) => + retainDiffSearchParams({ + search, + next: () => closeDiffSearchParams(search), + }), + }); + + expect(nextSearch).toEqual({}); + }); }); diff --git a/apps/web/src/diffRouteSearch.ts b/apps/web/src/diffRouteSearch.ts index d7de23e348..25f5ecc0b3 100644 --- a/apps/web/src/diffRouteSearch.ts +++ b/apps/web/src/diffRouteSearch.ts @@ -6,10 +6,20 @@ export interface DiffRouteSearch { diffFilePath?: string | undefined; } +export interface DiffRouteSearchControls { + clearDiff?: "1"; +} + +export type DiffRouteSearchNavigation = DiffRouteSearch & DiffRouteSearchControls; + function isDiffOpenValue(value: unknown): boolean { return value === "1" || value === 1 || value === true; } +function isClearDiffValue(value: unknown): boolean { + return value === "1" || value === 1 || value === true; +} + function normalizeSearchString(value: unknown): string | undefined { if (typeof value !== "string") { return undefined; @@ -18,18 +28,42 @@ function normalizeSearchString(value: unknown): string | undefined { return normalized.length > 0 ? normalized : undefined; } -export function stripDiffSearchParams>( +export function stripDiffSearchParams( params: T, ): Omit { - const { diff: _diff, diffTurnId: _diffTurnId, diffFilePath: _diffFilePath, ...rest } = params; + const { + diff: _diff, + diffTurnId: _diffTurnId, + diffFilePath: _diffFilePath, + ...rest + } = params as T & { + diff?: unknown; + diffTurnId?: unknown; + diffFilePath?: unknown; + }; return rest as Omit; } -export function parseDiffRouteSearch(search: Record): DiffRouteSearch { - const diff = isDiffOpenValue(search.diff) ? "1" : undefined; - const diffTurnIdRaw = diff ? normalizeSearchString(search.diffTurnId) : undefined; +export function closeDiffSearchParams( + params: T, +): Omit & DiffRouteSearchControls { + return { + ...stripDiffSearchParams(params), + clearDiff: "1", + }; +} + +export function parseDiffRouteSearch(search: object): DiffRouteSearch { + const rawSearch = search as { + diff?: unknown; + diffTurnId?: unknown; + diffFilePath?: unknown; + }; + const diff = isDiffOpenValue(rawSearch.diff) ? "1" : undefined; + const diffTurnIdRaw = diff ? normalizeSearchString(rawSearch.diffTurnId) : undefined; const diffTurnId = diffTurnIdRaw ? TurnId.makeUnsafe(diffTurnIdRaw) : undefined; - const diffFilePath = diff && diffTurnId ? normalizeSearchString(search.diffFilePath) : undefined; + const diffFilePath = + diff && diffTurnId ? normalizeSearchString(rawSearch.diffFilePath) : undefined; return { ...(diff ? { diff } : {}), @@ -37,3 +71,36 @@ export function parseDiffRouteSearch(search: Record): DiffRoute ...(diffFilePath ? { diffFilePath } : {}), }; } + +export function parseDiffRouteSearchNavigation(search: object): DiffRouteSearchNavigation { + const rawSearch = search as { + clearDiff?: unknown; + }; + const parsed = parseDiffRouteSearch(search); + const clearDiff = isClearDiffValue(rawSearch.clearDiff) ? "1" : undefined; + + return { + ...parsed, + ...(clearDiff ? { clearDiff } : {}), + }; +} + +export function retainDiffSearchParams({ + search, + next, +}: { + search: DiffRouteSearchNavigation; + next: (newSearch: DiffRouteSearchNavigation) => DiffRouteSearchNavigation; +}): DiffRouteSearchNavigation { + const result = next(search); + + if (result.clearDiff === "1") { + return result; + } + + if ("diff" in result) { + return result; + } + + return search.diff ? { ...result, diff: search.diff } : result; +} diff --git a/apps/web/src/routes/_chat.$threadId.tsx b/apps/web/src/routes/_chat.$threadId.tsx index 0bc0bc207c..8d4dcf8351 100644 --- a/apps/web/src/routes/_chat.$threadId.tsx +++ b/apps/web/src/routes/_chat.$threadId.tsx @@ -1,10 +1,14 @@ import { ThreadId } from "@t3tools/contracts"; -import { createFileRoute, useNavigate } from "@tanstack/react-router"; +import { createFileRoute, stripSearchParams, useNavigate } from "@tanstack/react-router"; import { Suspense, lazy, type ReactNode, useCallback, useEffect } from "react"; import ChatView from "../components/ChatView"; import { useComposerDraftStore } from "../composerDraftStore"; -import { parseDiffRouteSearch, stripDiffSearchParams } from "../diffRouteSearch"; +import { + type DiffRouteSearch, + parseDiffRouteSearch, + stripDiffSearchParams, +} from "../diffRouteSearch"; import { useMediaQuery } from "../hooks/useMediaQuery"; import { useStore } from "../store"; import { Sheet, SheetPopup } from "../components/ui/sheet"; @@ -16,6 +20,36 @@ const DIFF_INLINE_SIDEBAR_WIDTH_STORAGE_KEY = "chat_diff_sidebar_width"; const DIFF_INLINE_DEFAULT_WIDTH = "clamp(28rem,48vw,44rem)"; const DIFF_INLINE_SIDEBAR_MIN_WIDTH = 26 * 16; const COMPOSER_COMPACT_MIN_LEFT_CONTROLS_WIDTH_PX = 208; +const CLEAR_DIFF_SEARCH_VALUE = "1"; + +type ChatThreadRouteSearch = DiffRouteSearch & { + clearDiff?: typeof CLEAR_DIFF_SEARCH_VALUE; +}; + +function shouldClearDiff(value: unknown): boolean { + return value === CLEAR_DIFF_SEARCH_VALUE || value === 1 || value === true; +} + +function parseChatThreadRouteSearch(search: Record): ChatThreadRouteSearch { + const parsed = parseDiffRouteSearch(search); + return shouldClearDiff(search.clearDiff) + ? { ...parsed, clearDiff: CLEAR_DIFF_SEARCH_VALUE } + : parsed; +} + +function retainDiffUnlessCleared({ + search, + next, +}: { + search: ChatThreadRouteSearch; + next: (newSearch: ChatThreadRouteSearch) => ChatThreadRouteSearch; +}): ChatThreadRouteSearch { + const result = next(search); + if (result.clearDiff === CLEAR_DIFF_SEARCH_VALUE || "diff" in result) { + return result; + } + return search.diff ? { ...result, diff: search.diff } : result; +} const DiffPanelSheet = (props: { children: ReactNode; @@ -166,7 +200,12 @@ function ChatThreadRouteView() { void navigate({ to: "/$threadId", params: { threadId }, - search: { diff: undefined }, + search: (previous) => { + return { + ...stripDiffSearchParams(previous), + clearDiff: CLEAR_DIFF_SEARCH_VALUE, + }; + }, }); }, [navigate, threadId]); const openDiff = useCallback(() => { @@ -221,6 +260,12 @@ function ChatThreadRouteView() { } export const Route = createFileRoute("/_chat/$threadId")({ - validateSearch: (search) => parseDiffRouteSearch(search), + validateSearch: (search) => parseChatThreadRouteSearch(search), + search: { + middlewares: [ + retainDiffUnlessCleared, + stripSearchParams(["clearDiff"]), + ], + }, component: ChatThreadRouteView, }); From b71fc43f2360b7a0bcff7a145d688a43f2ba1ac3 Mon Sep 17 00:00:00 2001 From: Heinrich Xiao <74563446+Heinrich-XIAO@users.noreply.github.com> Date: Thu, 12 Mar 2026 07:49:16 -0400 Subject: [PATCH 3/3] Simplify diff close fix surface area --- apps/web/src/components/ChatView.tsx | 6 +-- apps/web/src/diffRouteSearch.test.ts | 62 +--------------------- apps/web/src/diffRouteSearch.ts | 79 +++------------------------- 3 files changed, 8 insertions(+), 139 deletions(-) diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx index cca51d89d0..38ddb15e05 100644 --- a/apps/web/src/components/ChatView.tsx +++ b/apps/web/src/components/ChatView.tsx @@ -36,11 +36,7 @@ import { gitBranchesQueryOptions, gitCreateWorktreeMutationOptions } from "~/lib import { projectSearchEntriesQueryOptions } from "~/lib/projectReactQuery"; import { serverConfigQueryOptions, serverQueryKeys } from "~/lib/serverReactQuery"; import { isElectron } from "../env"; -import { - closeDiffSearchParams, - parseDiffRouteSearch, - stripDiffSearchParams, -} from "../diffRouteSearch"; +import { parseDiffRouteSearch, stripDiffSearchParams } from "../diffRouteSearch"; import { type ComposerTrigger, detectComposerTrigger, diff --git a/apps/web/src/diffRouteSearch.test.ts b/apps/web/src/diffRouteSearch.test.ts index 2cad77ffd4..ef00874bd2 100644 --- a/apps/web/src/diffRouteSearch.test.ts +++ b/apps/web/src/diffRouteSearch.test.ts @@ -1,14 +1,6 @@ -import { stripSearchParams } from "@tanstack/react-router"; -import { TurnId } from "@t3tools/contracts"; import { describe, expect, it } from "vitest"; -import { - closeDiffSearchParams, - type DiffRouteSearchNavigation, - parseDiffRouteSearch, - parseDiffRouteSearchNavigation, - retainDiffSearchParams, -} from "./diffRouteSearch"; +import { parseDiffRouteSearch } from "./diffRouteSearch"; describe("parseDiffRouteSearch", () => { it("parses valid diff search values", () => { @@ -79,56 +71,4 @@ describe("parseDiffRouteSearch", () => { diff: "1", }); }); - - it("builds an explicit close navigation payload", () => { - expect( - closeDiffSearchParams({ - diff: "1", - diffTurnId: TurnId.makeUnsafe("turn-1"), - diffFilePath: "src/app.ts", - }), - ).toEqual({ - clearDiff: "1", - }); - }); - - it("parses route control flags for navigation middleware", () => { - expect( - parseDiffRouteSearchNavigation({ - clearDiff: "1", - }), - ).toEqual({ - clearDiff: "1", - }); - }); - - it("retains diff across navigations that do not touch diff state", () => { - expect( - retainDiffSearchParams({ - search: { - diff: "1", - }, - next: () => ({}), - }), - ).toEqual({ - diff: "1", - }); - }); - - it("does not reinsert diff when navigation explicitly clears it", () => { - const stripped = stripSearchParams(["clearDiff"]); - - const nextSearch = stripped({ - search: { - diff: "1", - }, - next: (search) => - retainDiffSearchParams({ - search, - next: () => closeDiffSearchParams(search), - }), - }); - - expect(nextSearch).toEqual({}); - }); }); diff --git a/apps/web/src/diffRouteSearch.ts b/apps/web/src/diffRouteSearch.ts index 25f5ecc0b3..d7de23e348 100644 --- a/apps/web/src/diffRouteSearch.ts +++ b/apps/web/src/diffRouteSearch.ts @@ -6,20 +6,10 @@ export interface DiffRouteSearch { diffFilePath?: string | undefined; } -export interface DiffRouteSearchControls { - clearDiff?: "1"; -} - -export type DiffRouteSearchNavigation = DiffRouteSearch & DiffRouteSearchControls; - function isDiffOpenValue(value: unknown): boolean { return value === "1" || value === 1 || value === true; } -function isClearDiffValue(value: unknown): boolean { - return value === "1" || value === 1 || value === true; -} - function normalizeSearchString(value: unknown): string | undefined { if (typeof value !== "string") { return undefined; @@ -28,42 +18,18 @@ function normalizeSearchString(value: unknown): string | undefined { return normalized.length > 0 ? normalized : undefined; } -export function stripDiffSearchParams( +export function stripDiffSearchParams>( params: T, ): Omit { - const { - diff: _diff, - diffTurnId: _diffTurnId, - diffFilePath: _diffFilePath, - ...rest - } = params as T & { - diff?: unknown; - diffTurnId?: unknown; - diffFilePath?: unknown; - }; + const { diff: _diff, diffTurnId: _diffTurnId, diffFilePath: _diffFilePath, ...rest } = params; return rest as Omit; } -export function closeDiffSearchParams( - params: T, -): Omit & DiffRouteSearchControls { - return { - ...stripDiffSearchParams(params), - clearDiff: "1", - }; -} - -export function parseDiffRouteSearch(search: object): DiffRouteSearch { - const rawSearch = search as { - diff?: unknown; - diffTurnId?: unknown; - diffFilePath?: unknown; - }; - const diff = isDiffOpenValue(rawSearch.diff) ? "1" : undefined; - const diffTurnIdRaw = diff ? normalizeSearchString(rawSearch.diffTurnId) : undefined; +export function parseDiffRouteSearch(search: Record): DiffRouteSearch { + const diff = isDiffOpenValue(search.diff) ? "1" : undefined; + const diffTurnIdRaw = diff ? normalizeSearchString(search.diffTurnId) : undefined; const diffTurnId = diffTurnIdRaw ? TurnId.makeUnsafe(diffTurnIdRaw) : undefined; - const diffFilePath = - diff && diffTurnId ? normalizeSearchString(rawSearch.diffFilePath) : undefined; + const diffFilePath = diff && diffTurnId ? normalizeSearchString(search.diffFilePath) : undefined; return { ...(diff ? { diff } : {}), @@ -71,36 +37,3 @@ export function parseDiffRouteSearch(search: object): DiffRouteSearch { ...(diffFilePath ? { diffFilePath } : {}), }; } - -export function parseDiffRouteSearchNavigation(search: object): DiffRouteSearchNavigation { - const rawSearch = search as { - clearDiff?: unknown; - }; - const parsed = parseDiffRouteSearch(search); - const clearDiff = isClearDiffValue(rawSearch.clearDiff) ? "1" : undefined; - - return { - ...parsed, - ...(clearDiff ? { clearDiff } : {}), - }; -} - -export function retainDiffSearchParams({ - search, - next, -}: { - search: DiffRouteSearchNavigation; - next: (newSearch: DiffRouteSearchNavigation) => DiffRouteSearchNavigation; -}): DiffRouteSearchNavigation { - const result = next(search); - - if (result.clearDiff === "1") { - return result; - } - - if ("diff" in result) { - return result; - } - - return search.diff ? { ...result, diff: search.diff } : result; -}