From 71b1288a7a03ae0b4a432b6761be12b2d9eaee4f Mon Sep 17 00:00:00 2001 From: Anish Sarkar <104695310+AnishSarkar22@users.noreply.github.com> Date: Wed, 22 Jul 2026 12:55:36 +0530 Subject: [PATCH] refactor: remove unused thread metadata migration and clean up chat thread queries --- .../175_publish_thread_metadata_to_zero.py | 23 ---- surfsense_backend/app/zero_publication.py | 6 +- surfsense_web/hooks/use-resolved-tabs.test.ts | 20 ++-- surfsense_web/hooks/use-resolved-tabs.ts | 100 ++++++++++++------ surfsense_web/zero/queries/chat.ts | 10 -- surfsense_web/zero/queries/documents.ts | 3 - surfsense_web/zero/queries/index.ts | 3 +- surfsense_web/zero/schema/chat.ts | 6 +- 8 files changed, 82 insertions(+), 89 deletions(-) delete mode 100644 surfsense_backend/alembic/versions/175_publish_thread_metadata_to_zero.py diff --git a/surfsense_backend/alembic/versions/175_publish_thread_metadata_to_zero.py b/surfsense_backend/alembic/versions/175_publish_thread_metadata_to_zero.py deleted file mode 100644 index d4d298a4a..000000000 --- a/surfsense_backend/alembic/versions/175_publish_thread_metadata_to_zero.py +++ /dev/null @@ -1,23 +0,0 @@ -"""publish thread metadata to zero_publication - -Revision ID: 175 -Revises: 174 -""" - -from collections.abc import Sequence - -from alembic import op -from app.zero_publication import apply_publication - -revision: str = "175" -down_revision: str | None = "174" -branch_labels: str | Sequence[str] | None = None -depends_on: str | Sequence[str] | None = None - - -def upgrade() -> None: - apply_publication(op.get_bind()) - - -def downgrade() -> None: - """No-op. Historical publication shapes are immutable.""" diff --git a/surfsense_backend/app/zero_publication.py b/surfsense_backend/app/zero_publication.py index f6dff6cdf..ea4d1c90c 100644 --- a/surfsense_backend/app/zero_publication.py +++ b/surfsense_backend/app/zero_publication.py @@ -57,12 +57,12 @@ AUTOMATION_COLS = [ "workspace_id", ] +# Only the columns Zero needs as the authz *parent* of chat messages/comments/ +# session-state (``whereExists("thread")`` + space constraint). Tab titles and +# visibility are resolved over REST (react-query), not pushed via Zero. NEW_CHAT_THREAD_COLS = [ "id", "workspace_id", - "title", - "visibility", - "created_by_id", ] # Enough to drive the lifecycle UI by push: status, the reviewable brief, and diff --git a/surfsense_web/hooks/use-resolved-tabs.test.ts b/surfsense_web/hooks/use-resolved-tabs.test.ts index 34a51df86..dfad6a5a6 100644 --- a/surfsense_web/hooks/use-resolved-tabs.test.ts +++ b/surfsense_web/hooks/use-resolved-tabs.test.ts @@ -1,30 +1,24 @@ import assert from "node:assert/strict"; import { test } from "node:test"; -import { - getMissingCompleteChatIds, - resolveTabPointers, - type ResolvedTab, -} from "./use-resolved-tabs"; +import { getMissingChatIds, resolveTabPointers, type ResolvedTab } from "./use-resolved-tabs"; // Run with: pnpm exec tsx --test hooks/use-resolved-tabs.test.ts -test("does not prune unresolved chat tabs before Zero completes", () => { - const missing = getMissingCompleteChatIds({ +test("does not prune chat tabs that have not resolved as missing", () => { + const missing = getMissingChatIds({ tabs: [{ id: "chat-42", type: "chat", entityId: 42, workspaceId: 7 }], - threadRows: [], - resultType: "unknown", + notFoundIds: new Set(), }); assert.equal(missing.size, 0); }); -test("prunes unresolved chat tabs only after Zero completes", () => { - const missing = getMissingCompleteChatIds({ +test("prunes only chat tabs whose thread resolved as not found", () => { + const missing = getMissingChatIds({ tabs: [ { id: "chat-42", type: "chat", entityId: 42, workspaceId: 7 }, { id: "chat-43", type: "chat", entityId: 43, workspaceId: 7 }, ], - threadRows: [{ id: 43, title: "Present", visibility: "PRIVATE" }], - resultType: "complete", + notFoundIds: new Set([42]), }); assert.deepEqual([...missing], [42]); diff --git a/surfsense_web/hooks/use-resolved-tabs.ts b/surfsense_web/hooks/use-resolved-tabs.ts index 1b072857a..112d82983 100644 --- a/surfsense_web/hooks/use-resolved-tabs.ts +++ b/surfsense_web/hooks/use-resolved-tabs.ts @@ -1,11 +1,19 @@ "use client"; -import { useQuery } from "@rocicorp/zero/react"; +import { useQueries } from "@tanstack/react-query"; import { useAtomValue, useSetAtom } from "jotai"; import { useEffect, useMemo } from "react"; import { pruneMissingChatTabsAtom, type Tab, tabsAtom } from "@/atoms/tabs/tabs.atom"; -import type { ChatVisibility } from "@/lib/chat/thread-persistence"; -import { queries } from "@/zero/queries"; +import { documentsApiService } from "@/lib/apis/documents-api.service"; +import { type ChatVisibility, getThreadFull } from "@/lib/chat/thread-persistence"; +import { NotFoundError } from "@/lib/error"; +import { cacheKeys } from "@/lib/query-client/cache-keys"; + +// Thread/document metadata is read-only for tabs: the DB is the single source +// of truth and react-query is the cache. Titles/visibility stay fresh because +// the rename/visibility/delete mutations patch these same query caches (see +// lib/chat/thread-cache.ts), so a rename reflects in the tab bar immediately. +const METADATA_STALE_TIME_MS = 60 * 1000; interface ThreadRow { id: number; @@ -38,6 +46,13 @@ function rowById(rows: readonly T[] | undefined): Map< return new Map((rows ?? []).map((row) => [row.id, row])); } +// Retry transient failures (network, 5xx) but never a definitive 404 — a +// missing thread/document is authoritative and should settle immediately so +// the tab can be pruned rather than spun on. +function retryUnlessNotFound(failureCount: number, error: Error): boolean { + return !(error instanceof NotFoundError) && failureCount < 2; +} + export function getChatUrl(workspaceId: number, threadId: number | null): string { return threadId ? `/dashboard/${workspaceId}/new-chat/${threadId}` @@ -59,7 +74,9 @@ export function resolveTabPointers({ return tabs.map((tab) => { if (tab.type === "document") { const title = - tab.entityId === null ? "Document" : (documents.get(tab.entityId)?.title ?? `Document ${tab.entityId}`); + tab.entityId === null + ? "Document" + : (documents.get(tab.entityId)?.title ?? `Document ${tab.entityId}`); return { ...tab, title }; } @@ -73,23 +90,23 @@ export function resolveTabPointers({ }); } -export function getMissingCompleteChatIds({ +/** + * A chat tab is prunable only once its thread metadata fetch has settled as a + * definitive 404 (thread deleted). Transient network/5xx errors are excluded so + * an outage never wrongly closes open tabs. + */ +export function getMissingChatIds({ tabs, - threadRows, - resultType, + notFoundIds, }: { tabs: Tab[]; - threadRows?: readonly ThreadRow[]; - resultType: string; + notFoundIds: Set; }): Set { - if (resultType !== "complete") return new Set(); - - const threadIds = new Set((threadRows ?? []).map((row) => row.id)); return new Set( tabs .filter( (tab): tab is Tab & { type: "chat"; entityId: number } => - tab.type === "chat" && tab.entityId !== null && !threadIds.has(tab.entityId) + tab.type === "chat" && tab.entityId !== null && notFoundIds.has(tab.entityId) ) .map((tab) => tab.entityId) ); @@ -101,29 +118,48 @@ export function useResolvedTabs(): ResolvedTab[] { const chatIds = useMemo(() => uniqueEntityIds(tabs, "chat"), [tabs]); const documentIds = useMemo(() => uniqueEntityIds(tabs, "document"), [tabs]); - const [threadRows, threadResult] = useQuery( - queries.threads.byIds({ ids: chatIds.length > 0 ? chatIds : [-1] }) + + const threadResults = useQueries({ + queries: chatIds.map((id) => ({ + queryKey: cacheKeys.threads.detail(id), + queryFn: () => getThreadFull(id), + staleTime: METADATA_STALE_TIME_MS, + retry: retryUnlessNotFound, + })), + }); + + const documentResults = useQueries({ + queries: documentIds.map((id) => ({ + queryKey: cacheKeys.documents.document(String(id)), + queryFn: () => documentsApiService.getDocument({ id }), + staleTime: METADATA_STALE_TIME_MS, + retry: retryUnlessNotFound, + })), + }); + + const threadRows: ThreadRow[] = threadResults.flatMap((result, index) => + result.data + ? [{ id: chatIds[index], title: result.data.title, visibility: result.data.visibility }] + : [] ); - const [documentRows] = useQuery( - queries.documents.byIds({ ids: documentIds.length > 0 ? documentIds : [-1] }) + const documentRows: DocumentRow[] = documentResults.flatMap((result, index) => + result.data ? [{ id: documentIds[index], title: result.data.title }] : [] ); - const missingChatIds = useMemo( - () => - getMissingCompleteChatIds({ - tabs, - threadRows, - resultType: threadResult.type, - }), - [tabs, threadRows, threadResult.type] - ); + // Stable primitive key of the threads that settled as 404, so the prune + // effect fires only when that set changes — not on every react-query render. + const notFoundChatIdsKey = threadResults + .flatMap((result, index) => (result.error instanceof NotFoundError ? [chatIds[index]] : [])) + .sort((a, b) => a - b) + .join(","); useEffect(() => { - pruneMissingChatTabs(missingChatIds); - }, [missingChatIds, pruneMissingChatTabs]); + const notFoundIds = new Set( + notFoundChatIdsKey ? notFoundChatIdsKey.split(",").map(Number) : [] + ); + const missing = getMissingChatIds({ tabs, notFoundIds }); + if (missing.size > 0) pruneMissingChatTabs(missing); + }, [notFoundChatIdsKey, tabs, pruneMissingChatTabs]); - return useMemo( - () => resolveTabPointers({ tabs, threadRows, documentRows }), - [tabs, threadRows, documentRows] - ); + return resolveTabPointers({ tabs, threadRows, documentRows }); } diff --git a/surfsense_web/zero/queries/chat.ts b/surfsense_web/zero/queries/chat.ts index 06969a4e9..40e09a6ee 100644 --- a/surfsense_web/zero/queries/chat.ts +++ b/surfsense_web/zero/queries/chat.ts @@ -29,13 +29,3 @@ export const chatSessionQueries = { .one() ), }; - -export const threadQueries = { - byIds: defineQuery(z.object({ ids: z.array(z.number()) }), ({ args: { ids }, ctx }) => - constrainToAllowedSpaces(zql.new_chat_threads, ctx) - .where("id", "IN", ids) - .where(({ or, cmp }) => - or(cmp("createdById", ctx?.userId ?? ""), cmp("visibility", "SEARCH_SPACE")) - ) - ), -}; diff --git a/surfsense_web/zero/queries/documents.ts b/surfsense_web/zero/queries/documents.ts index 6c110aee2..d2f483989 100644 --- a/surfsense_web/zero/queries/documents.ts +++ b/surfsense_web/zero/queries/documents.ts @@ -9,9 +9,6 @@ export const documentQueries = { if (!canReadSpace(ctx, workspaceId)) return denySpace(query).orderBy("createdAt", "desc"); return constrainToAllowedSpaces(query, ctx).orderBy("createdAt", "desc"); }), - byIds: defineQuery(z.object({ ids: z.array(z.number()) }), ({ args: { ids }, ctx }) => - constrainToAllowedSpaces(zql.documents, ctx).where("id", "IN", ids) - ), }; export const connectorQueries = { diff --git a/surfsense_web/zero/queries/index.ts b/surfsense_web/zero/queries/index.ts index 2e51bf9a8..45df8fa98 100644 --- a/surfsense_web/zero/queries/index.ts +++ b/surfsense_web/zero/queries/index.ts @@ -1,6 +1,6 @@ import { defineQueries } from "@rocicorp/zero"; import { automationRunQueries } from "./automations"; -import { chatSessionQueries, commentQueries, messageQueries, threadQueries } from "./chat"; +import { chatSessionQueries, commentQueries, messageQueries } from "./chat"; import { connectorQueries, documentQueries } from "./documents"; import { folderQueries } from "./folders"; import { notificationQueries } from "./inbox"; @@ -15,7 +15,6 @@ export const queries = defineQueries({ messages: messageQueries, comments: commentQueries, chatSession: chatSessionQueries, - threads: threadQueries, user: userQueries, automationRuns: automationRunQueries, podcasts: podcastQueries, diff --git a/surfsense_web/zero/schema/chat.ts b/surfsense_web/zero/schema/chat.ts index 81673feaa..c2790d5bc 100644 --- a/surfsense_web/zero/schema/chat.ts +++ b/surfsense_web/zero/schema/chat.ts @@ -20,13 +20,13 @@ export const newChatMessageTable = table("new_chat_messages") }) .primaryKey("id"); +// Published only as the authz parent of chat messages/comments/session-state +// (whereExists("thread") + space constraint). Title/visibility are resolved +// over REST (react-query) for the tab bar, not synced through Zero. export const newChatThreadTable = table("new_chat_threads") .columns({ id: number(), workspaceId: number().from("workspace_id"), - title: string(), - visibility: string(), - createdById: string().optional().from("created_by_id"), }) .primaryKey("id");