From 2e4191e56fdf3c9d0c5c4c4331dac8a29dcf98cb Mon Sep 17 00:00:00 2001 From: Xuehao Xu Date: Sun, 4 Oct 2026 14:40:42 +0800 Subject: [PATCH] fix(frontend): preserve complete artifact editing baselines (#6036) * fix(frontend): preserve complete artifact editing baselines * fix(frontend): keep artifact edit exit available during reloads * refactor(frontend): remove redundant artifact action fragments --- README.md | 8 +- frontend/src/AGENTS.md | 9 +- .../artifacts/artifact-file-detail.tsx | 136 ++++---- .../artifact-file-detail-editing.dom.test.tsx | 293 ++++++++++++++++++ 4 files changed, 388 insertions(+), 58 deletions(-) create mode 100644 frontend/tests/unit/components/workspace/artifacts/artifact-file-detail-editing.dom.test.tsx diff --git a/README.md b/README.md index 0b75733435..aa0e9ca8fb 100644 --- a/README.md +++ b/README.md @@ -1942,8 +1942,12 @@ If the sample cuts a CRLF line ending in half, the preview keeps the earlier com Text artifacts are streamed with HTTP byte-range support. The Web UI initially loads at most 1 MiB, shows the preview size when a file is larger, and waits for an explicit **Load full file** action before fetching the remainder or mounting -the full code editor. Active HTML, XHTML, and SVG artifacts remain forced -downloads at the Gateway boundary. +the full code editor. Editing becomes available once the complete file has loaded; +unsaved-change detection uses the full content, including when reverting edits +or deleting the portion beyond the initial preview. If a reload fails or returns +only a preview, users can still exit editing while retaining their unsaved draft. +Active HTML, XHTML, and SVG artifacts remain forced downloads at the Gateway +boundary. Artifact previews and downloads preserve literal percent sequences in file names: `report%20final.md` and `report final.md` remain distinct files. Markdown links diff --git a/frontend/src/AGENTS.md b/frontend/src/AGENTS.md index d40cd73500..2276232c5d 100644 --- a/frontend/src/AGENTS.md +++ b/frontend/src/AGENTS.md @@ -32,8 +32,13 @@ Language-map membership checks only own properties; inherited names such as Regular artifact text loads request at most the first 1 MiB through an HTTP byte range. A truncated preview must stay lightweight and expose an explicit full-file action; do not mount CodeMirror for that artifact until the user - requests and receives the complete content. The Gateway retains range - ownership and returns 206/416 through `FileResponse`. + requests and receives the complete content. Truncated responses must never + establish or replace an editing baseline, including when discarding a draft: + their ETag can match the full response. Editing and saving require complete + content; retain existing drafts across partial reloads. The Gateway retains + range ownership and returns 206/416 through `FileResponse`. Keep Exit editing + available while editing even if a reload is loading, truncated, or fails; + exiting retains the draft and is disabled only during a pending save. 3. `useThreadHistory` loads persisted conversation pages from `GET /api/threads/{id}/messages/page`, preserving the backend's thread-global event `seq`; rendering overlays checkpoint/live copies at their matching canonical identities (a summarized checkpoint may contain a protected early input plus a recent tail). Context-compaction rescue diffs every retained visible identity rather than slicing at the first anchor, and keeps a run-scoped ledger of committed visible messages so replacement updates and repeated rolling checkpoint windows cannot erase an already displayed step. A checkpoint/transient prefix whose canonical position is still behind an unloaded cursor page is woven in before the first shared anchor, not discarded: both the checkpoint and seq-sorted history place it earlier, so that position is known even when the pages between are not. Position authority is seq-first and lives in `core/threads/message-order.ts` (re-exported through `hooks.ts`): every normalized identity tracks latest visible content and trusted position separately; a valid `deerflow_seq` (positive safe integer, earliest value wins per identity, hidden control copies contribute a position only when no visible copy carries one) joins an ascending skeleton that outranks identity-anchor weaving, which remains the fallback for no-seq segments whose internal order is preserved. Live-only and rescued messages with trusted seq also serve as anchors for adjacent no-seq segments. A trailing segment follows its last live-only positioned anchor within the loaded window; after a shared anchor or a rescued prefix before the window, it stays at the tail. Optimistic messages remain last; the transient bridge inserts positioned rows before weaving so the insertion cannot reverse previously displayed steps. Rescued sequence anchors preserve preceding captured steps even before React has rendered them; only a leading prefix anchored to loaded history requires previously rendered ordering to cross an unloaded cursor gap. Content replacement never drops the known seq, `run_id`, or `turn_duration`, and the compaction transient bridge plus rendered ledger share the same position priority. `deerflow_seq` is server-owned display metadata and is never written back into a checkpoint. It must never be appended to the tail (#4065) — the tail is provably wrong — but suppressing it entirely is how a user's own question vanished from a long thread once the first 50-row history page no longer reached back to it (#4666). A collapsed unloaded gap is recoverable by paging; a dropped message is not. Weaving alone restores the message but not its exact position — after compaction the live window carries too few anchors — so both sides now carry the backend's thread-global `additional_kwargs.deerflow_seq`: `buildVisibleHistoryMessages` copies each row's `seq`, and the Gateway stamps it onto `values` frame messages it has already persisted. A live message whose seq is below the loaded window's lower bound is placed ahead of everything on screen instead of before the nearest anchor, which is what puts a compaction-rescued first user turn back at the head rather than mid-transcript. That split happens _before_ the anchor walk, not inside it: a compacted checkpoint can share no identity at all with the loaded page — it keeps only the current run's recent tail, while the page on screen was fetched turns earlier — and the anchor walk then never runs at all, which is precisely when a rescued turn most needs its seq. Doing the split inside the walk left that case appending the message after the whole window (#4666), the one arrangement #4065 proved wrong. A message without a seq (still streaming, so not in the feed yet) keeps the weaving path — the tail is already its correct position. Optimistic messages are then added without timestamp re-sorting. History invalidation preserves already-loaded pages so their established ordering positions are not discarded. Dynamic context re-keys each submitted user message from the client-generated `local-human-*` identity `X` to the visible server echo `X__user`; UI identity matching normalizes that reserved suffix only for human messages so the optimistic input and checkpoint replacement remain one visible turn. At dispatch, a local-turn anchor snapshots the checkpoint identity baseline, canonical history identities and maximum trusted seq, and any pre-existing transient-bridge identities. Render repair uses `confirmedHistoryIdentities` plus `preSubmitMaxSeq` to restore baseline or history-confirmed messages above that exact human anchor, while moving only speculative non-baseline AI/tool steps behind it; `currentTurnRunIds` keeps already-persisted steps of the active turn below their human. Keep the anchor scoped to its originating thread through finish, stop, and stream error because the SDK's settled frame can retain transient event order; replace it on the next local submit and clear it on thread switch or replay-gap recovery. 4. Stop actions call the LangGraph SDK stream stop path; `core/threads/hooks.ts` invalidates current-thread, thread-history, token-usage, and sidebar/search caches immediately and schedules one follow-up refetch because SDK stop may finish via abort + fire-and-forget cancel before backend title finalization commits 5. TanStack Query manages server state. `UserPreferencesBoundary` preserves diff --git a/frontend/src/components/workspace/artifacts/artifact-file-detail.tsx b/frontend/src/components/workspace/artifacts/artifact-file-detail.tsx index 9166b89fef..b44d7ec322 100644 --- a/frontend/src/components/workspace/artifacts/artifact-file-detail.tsx +++ b/frontend/src/components/workspace/artifacts/artifact-file-detail.tsx @@ -227,19 +227,31 @@ export function ArtifactFileDetail({ (draft) => draft.draftContent !== draft.baselineContent, ); const isEditing = editingPath === filepath; - const canEdit = canEditOpenedArtifact({ - filepath, - isCodeFile, - isWriteFile, - isSkillFile, - isMock: Boolean(isMock), - hasRevision: typeof sha256 === "string" && sha256.length === 64, - isStaticWebsite: env.NEXT_PUBLIC_STATIC_WEBSITE_ONLY === "true", - }); + const canEdit = + content !== undefined && + !truncated && + !isLoading && + !error && + canEditOpenedArtifact({ + filepath, + isCodeFile, + isWriteFile, + isSkillFile, + isMock: Boolean(isMock), + hasRevision: typeof sha256 === "string" && sha256.length === 64, + isStaticWebsite: env.NEXT_PUBLIC_STATIC_WEBSITE_ONLY === "true", + }); const editorContent = isDirty ? activeDraft.draftContent : visibleContent; useEffect(() => { - if (content === undefined || sha256 === undefined || isWriteFile) { + // A byte-range preview can carry the same full-file ETag as the complete + // response. Only complete content can establish an editing baseline. + if ( + content === undefined || + sha256 === undefined || + isWriteFile || + truncated + ) { return; } setDrafts((current) => { @@ -250,7 +262,7 @@ export function ArtifactFileDetail({ } return { ...current, [filepath]: next }; }); - }, [content, filepath, isWriteFile, setDrafts, sha256]); + }, [content, filepath, isWriteFile, setDrafts, sha256, truncated]); const [viewMode, setViewMode] = useState<"code" | "preview">( artifactViewState.initialViewMode, @@ -268,8 +280,14 @@ export function ArtifactFileDetail({ }, [isDirty, t.artifactEditing.discardChanges]); const discardDraft = useCallback(() => { - const latestContent = content ?? activeDraft.baselineContent; - const latestSha256 = sha256 ?? activeDraft.baselineSha256; + const hasCompleteContent = + !truncated && content !== undefined && sha256 !== undefined; + const latestContent = hasCompleteContent + ? content + : activeDraft.baselineContent; + const latestSha256 = hasCompleteContent + ? sha256 + : activeDraft.baselineSha256; setDrafts((current) => ({ ...current, [filepath]: { @@ -281,7 +299,15 @@ export function ArtifactFileDetail({ }, })); setEditingPath(null); - }, [activeDraft, content, filepath, setDrafts, setEditingPath, sha256]); + }, [ + activeDraft, + content, + filepath, + setDrafts, + setEditingPath, + sha256, + truncated, + ]); const handleSave = useCallback(async () => { if ( @@ -487,47 +513,49 @@ export function ArtifactFileDetail({ /> )} {canEdit && isEditing && ( - <> - void handleSave()} + /> + )} + {isEditing && ( + setEditingPath(null)} + /> + )} + {canEdit && isEditing && ( + { + if (confirmDiscard()) { + discardDraft(); } - disabled={ - !isDirty || - isSaving || - thread.isLoading || - activeDraft.conflict - } - onClick={() => void handleSave()} - /> - setEditingPath(null)} - /> - { - if (confirmDiscard()) { - discardDraft(); - } - }} - /> - + }} + /> )} {!isEditing && !isWriteFile && diff --git a/frontend/tests/unit/components/workspace/artifacts/artifact-file-detail-editing.dom.test.tsx b/frontend/tests/unit/components/workspace/artifacts/artifact-file-detail-editing.dom.test.tsx new file mode 100644 index 0000000000..f07c8263bb --- /dev/null +++ b/frontend/tests/unit/components/workspace/artifacts/artifact-file-detail-editing.dom.test.tsx @@ -0,0 +1,293 @@ +import { afterEach, beforeEach, describe, expect, it, rs } from "@rstest/core"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { + cleanup, + fireEvent, + render, + screen, + waitFor, +} from "@testing-library/react"; +import { useState, type PropsWithChildren } from "react"; + +const mocks = rs.hoisted(() => ({ + artifactContent: { + content: undefined as string | undefined, + url: undefined as string | undefined, + sha256: undefined as string | undefined, + truncated: false, + previewBytes: undefined as number | undefined, + totalBytes: undefined as number | undefined, + fullContentRequested: false, + loadFullContent: rs.fn(), + isLoading: false, + error: undefined as unknown, + }, + setSidebarOpen: rs.fn(), +})); + +rs.mock("@/core/artifacts/hooks", () => ({ + useArtifactContent: () => mocks.artifactContent, +})); +rs.mock("@/components/workspace/messages/context", () => ({ + useThread: () => ({ thread: { isLoading: false }, isMock: false }), +})); +rs.mock("@/components/ui/sidebar", () => ({ + useSidebar: () => ({ setOpen: mocks.setSidebarOpen }), +})); +rs.mock("next/navigation", () => ({ + usePathname: () => "/workspace/chats/artifact-editing-test", +})); +rs.mock("@/core/auth/AuthProvider", () => ({ + useAuth: () => ({ user: null }), +})); +rs.mock("@/core/config", () => ({ + getBackendBaseURL: () => "/backend", +})); +rs.mock("@/env", () => ({ + env: { NEXT_PUBLIC_STATIC_WEBSITE_ONLY: "false" }, +})); +rs.mock("@/core/api/fetcher", () => ({ + fetch: rs.fn(), +})); + +// Exercise real React/provider state and editor callbacks without CodeMirror. +rs.mock("@/components/workspace/code-editor", () => ({ + CodeEditor: ({ + value, + readonly, + disabled, + onChange, + }: { + value: string; + readonly?: boolean; + disabled?: boolean; + onChange?: (value: string) => void; + }) => ( +