From 4db89cbe3f81e39ac03cb8254c51a9557874f5c2 Mon Sep 17 00:00:00 2001 From: Youhai Date: Fri, 31 Jul 2026 20:20:58 +0800 Subject: [PATCH] fix(web): show file summary after task completion (#143) Co-authored-by: Youhai <185432073+Youhai020616@users.noreply.github.com> Co-authored-by: Claude Fable 5 --- .../2026-07-30-file-summary-task-boundary.md | 3 + changelog/unreleased/README.md | 2 + packages/web/src/features/chat/chat-page.tsx | 55 ++++++++++--------- .../src/features/chat/message-files-card.tsx | 29 +++++----- .../web/src/features/chat/message-item.tsx | 30 +++++++--- packages/web/test/stream-controller.test.ts | 24 +++++++- packages/web/test/stream-model.test.ts | 16 ++++++ 7 files changed, 108 insertions(+), 51 deletions(-) create mode 100644 changelog/unreleased/2026-07-30-file-summary-task-boundary.md diff --git a/changelog/unreleased/2026-07-30-file-summary-task-boundary.md b/changelog/unreleased/2026-07-30-file-summary-task-boundary.md new file mode 100644 index 0000000..784e15a --- /dev/null +++ b/changelog/unreleased/2026-07-30-file-summary-task-boundary.md @@ -0,0 +1,3 @@ +# File summaries wait for the Task to finish + +The main conversation's file summary no longer appears after an intermediate assistant message while tools and later model turns are still running. It now appears once at the completed Task boundary and scans all assistant text from that Task, so paths mentioned before a tool call are still available in the final summary. Nested agent conversations keep their existing per-message summaries because their embedded stream does not expose the parent view's Task footer. File-existence caching also stops retaining negative results, allowing a later Task to create and surface a path that was previously absent. Existence checks go out in server-sized batches, so a long Task referencing more paths than one files/stat call accepts still gets its summary instead of silently losing it. diff --git a/changelog/unreleased/README.md b/changelog/unreleased/README.md index 6d89054..1a34750 100644 --- a/changelog/unreleased/README.md +++ b/changelog/unreleased/README.md @@ -1,3 +1,5 @@ # Unreleased Changes since v0.1.5. The version number is assigned at release, when this folder is renamed. + +- [2026-07-30] Web App: the main conversation's file summary moves to the completed Task boundary — one card per Task scanning all of its assistant text, nested agent conversations keep their per-message summaries, and file-existence caching stops retaining negative results so a later Task can surface a newly created path. ([details](2026-07-30-file-summary-task-boundary.md)) diff --git a/packages/web/src/features/chat/chat-page.tsx b/packages/web/src/features/chat/chat-page.tsx index d445abc..581ef98 100644 --- a/packages/web/src/features/chat/chat-page.tsx +++ b/packages/web/src/features/chat/chat-page.tsx @@ -215,6 +215,13 @@ function headerStats( */ export const DRAFT_SESSION_ID = "new"; +/** + * Server-enforced ceiling on paths per files/stat call (STAT_MAX_PATHS in the sessions routes, + * which 400s above it). A Task-level summary aggregates candidates across the whole Task and can + * exceed it, so cache misses are checked in chunks of this size. + */ +const STAT_PATHS_PER_REQUEST = 100; + export function ChatPage() { const navigate = useNavigate(); const params = useParams<{ sessionId?: string }>(); @@ -451,10 +458,10 @@ export function ChatPage() { } }, [stream.taskState, reloadSessions, reloadAgents]); - // Existence cache for message file cards (session-level): normalized relative path -> whether - // it exists; while a lookup is in flight, the cache shares a single Promise, so a batch of - // concurrent mounts only issues one files/stat call. - const statCacheRef = useRef(new Map>()); + // Positive-only existence cache for file summary cards (session-level): normalized relative + // path -> true, or the shared in-flight lookup. Missing files aren't retained — a later Task may + // create the same path, so its summary must re-check instead of inheriting stale false state. + const statCacheRef = useRef(new Map>()); // Session switch: resets the cost, the file-card existence cache, and the per-turn thinking // level (it's per-session UI state), avoiding stale data from the previous Session (Files @@ -466,34 +473,30 @@ export function ChatPage() { statCacheRef.current = new Map(); }, [routeSessionId]); - // Batched existence check (message file cards): merges only the cache-miss paths into a single - // files/stat call, and the result lands in the session-level cache — during streaming, the - // candidate set is re-checked on every change, and the cache ensures only new paths trigger a - // request. On request failure, the placeholder is cleared (don't permanently cache a "couldn't - // find" as "doesn't exist"), returned as not-existing this time, and re-checked on the next mount. + // Batched existence check for file summaries: cache stable positive results and share in-flight + // requests, but never retain a negative result. Each pending lookup mutates the cache only while + // it is still the current entry, so an old request can't delete or overwrite a newer one. const statFiles = useCallback( async (paths: string[]): Promise> => { const sessionId = selected?.sessionId ?? null; const cache = statCacheRef.current; const misses = sessionId === null ? [] : paths.filter((p) => !cache.has(p)); if (sessionId !== null && misses.length > 0) { - const batch = api - .statSessionFiles(sessionId, misses) - .then((res) => new Set(res.existing)) - .catch(() => null); - for (const p of misses) { - cache.set( - p, - batch.then((existing) => { - if (existing === null) { - cache.delete(p); - return false; - } - const exists = existing.has(p); - cache.set(p, exists); - return exists; - }), - ); + for (let i = 0; i < misses.length; i += STAT_PATHS_PER_REQUEST) { + const chunk = misses.slice(i, i + STAT_PATHS_PER_REQUEST); + const batch = api + .statSessionFiles(sessionId, chunk) + .then((res) => new Set(res.existing)) + .catch(() => null); + for (const p of chunk) { + const pending = batch.then((existing) => existing?.has(p) ?? false); + cache.set(p, pending); + void pending.then((exists) => { + if (cache.get(p) !== pending) return; + if (exists) cache.set(p, true); + else cache.delete(p); + }); + } } } const result = new Set(); diff --git a/packages/web/src/features/chat/message-files-card.tsx b/packages/web/src/features/chat/message-files-card.tsx index 0a6b728..7a2c32e 100644 --- a/packages/web/src/features/chat/message-files-card.tsx +++ b/packages/web/src/features/chat/message-files-card.tsx @@ -1,19 +1,16 @@ /** - * Message-level file summary card (visual reference: Codex's "files changed" card): extracts - * file paths from inline code in the assistant's text (heuristic via isFilePathLike), normalizes - * them to Workspace-relative paths, confirms they actually exist via files/stat, and aggregates - * them into a unified card at the end of the message — a light-background single-line header bar - * ("N files") + a list of file rows inside the card; each row's path is split into a "faded - * directory / bold filename" pair, with a "Preview" label at the end of the row making the action - * explicit, and clicking the whole row navigates to the Files panel preview for that relative - * path via onOpenFile. Collapses when there are more than 3 rows. - * The whole card doesn't render until the stat result comes back (to avoid a flash-then-disappear); - * it also doesn't render if none of the candidates exist — the heuristic extraction inevitably - * matches error message examples, external paths, and other strings that can't actually be - * opened, so this card is only responsible for "if you click it, it really opens". - * Doesn't include diff stats — file writes may happen inside opaque exec_command shells, so - * the protocol has no reliable structured edit signal; this is just an aggregated view of - * text references, hence the neutral "N files" title. + * File summary card for a supplied assistant-text scope (visual reference: Codex's "files + * changed" card): the root conversation passes a completed Task's aggregated assistant text, + * while nested conversations — which don't produce task_stats — pass one settled assistant + * message to preserve their existing behavior. Extracts inline-code paths heuristically via + * isFilePathLike, normalizes them to Workspace-relative paths, confirms they actually exist via + * files/stat, and renders a light-background "N files" card whose rows open the Files panel. + * Collapses when there are more than 3 rows. + * + * The card waits for stat results and doesn't render when no candidate exists. It intentionally + * does not claim these files were changed: opaque exec_command shells provide no reliable + * structured edit signal, so this is only an aggregated view of text references that are + * currently openable. */ import { useEffect, useMemo, useState } from "react"; import { S } from "../../lib/strings"; @@ -61,7 +58,7 @@ export function MessageFilesCard({ statFiles, onOpenFile, }: { - /** Raw Markdown text of the assistant message. */ + /** Raw Markdown from the assistant scope being summarized (a root Task or one nested message). */ text: string; /** Absolute Workspace path of the current Session (used to normalize absolute paths found in the text). */ workspace: string | null; diff --git a/packages/web/src/features/chat/message-item.tsx b/packages/web/src/features/chat/message-item.tsx index 6bc179c..2f3a196 100644 --- a/packages/web/src/features/chat/message-item.tsx +++ b/packages/web/src/features/chat/message-item.tsx @@ -328,8 +328,8 @@ export function MessageItem({ item, ctx }: { item: ChatItem; ctx: StreamRenderCo {item.stopReason && item.stopReason !== "completed" && ( [{item.stopReason}] )} - {/* File summary card (Codex-style): aggregates file references in the text once streaming ends (lists only ones confirmed to exist). */} - {!item.streaming && ctx.onOpenFile && ctx.statFiles && ( + {/* Nested models don't produce task_stats, so preserve their existing message-level file summaries. The root conversation renders one aggregated card from task_stats instead. */} + {ctx.origin.length > 0 && !item.streaming && ctx.onOpenFile && ctx.statFiles && ( ; case "task_stats": return ( - + <> + {/* Root-session file references are a Task-level summary: task_stats is emitted only after the Task closes, and assistantText aggregates every assistant segment in that Task. */} + {ctx.origin.length === 0 && + ctx.onOpenFile && + ctx.statFiles && + item.assistantText.trim() !== "" && ( + + )} + + ); } } diff --git a/packages/web/test/stream-controller.test.ts b/packages/web/test/stream-controller.test.ts index 6c06f77..7ac0805 100644 --- a/packages/web/test/stream-controller.test.ts +++ b/packages/web/test/stream-controller.test.ts @@ -21,7 +21,7 @@ import type { MessagesLiveTail, ServerEvent, SessionStatus } from "@prismshadow/ import { createStreamController } from "../src/lib/omni/stream-controller"; import type { StreamController } from "../src/lib/omni/stream-controller"; import { approvalKey, findToolCard } from "../src/lib/omni/stream-model"; -import type { AssistantTextItem, ToolCallItem } from "../src/lib/omni/stream-model"; +import type { AssistantTextItem, TaskStatsItem, ToolCallItem } from "../src/lib/omni/stream-model"; /** Override a message timestamp (constructor defaults to the current time). */ function at(msg: M, ts: string): M { @@ -172,6 +172,28 @@ describe("in-stream task_state is the authoritative running state (history-closi // History hasn't returned yet, but state is already reported. expect(h.states).toEqual(["running"]); }); + + it("closes the current Task before an auto-started queued follow-up begins", async () => { + const h = createHarness(); + const p = h.controller.load(); + h.controller.handleServer({ type: "task_state", state: "running", queued: 1 }); + h.resolveLoad(HISTORY_TASK); + await p; + + // Server ordering for a queued follow-up: current run flips idle, then launchTask publishes + // the queued user input before its running state. The first idle must seal Task 1 before that. + h.controller.handleServer({ type: "task_state", state: "idle", queued: 1 }); + h.controller.handleOmni(at(userText("follow-up"), "2026-07-05T00:01:00.000Z")); + h.controller.handleServer({ type: "task_state", state: "running", queued: 0 }); + h.controller.handleOmni(at(assistantText("follow-up answer"), "2026-07-05T00:01:03.000Z")); + h.controller.handleOmni(at(tokenUsage(counts(1400), counts(400)), "2026-07-05T00:01:05.000Z")); + h.controller.handleServer({ type: "task_state", state: "idle", queued: 0 }); + + const stats = h.controller.model.items.filter( + (item) => item.kind === "task_stats", + ) as TaskStatsItem[]; + expect(stats.map((item) => item.assistantText)).toEqual(["answer", "follow-up answer"]); + }); }); describe("approval re-delivery (origin composite key + missing-card backfill)", () => { diff --git a/packages/web/test/stream-model.test.ts b/packages/web/test/stream-model.test.ts index 3517bfe..42b9e16 100644 --- a/packages/web/test/stream-model.test.ts +++ b/packages/web/test/stream-model.test.ts @@ -788,6 +788,22 @@ describe("Task segmentation and stats triggering", () => { expect(stats.stats!.elapsedDeltaMs).toBe(5000); // time span from the first to the last message }); + it("aggregates every assistant text segment in a Task into the footer copy target", () => { + const m = createStreamModel(); + pushMessages(m, [ + at(userText("build it"), "2026-07-05T00:00:00.000Z"), + at(assistantText("Creating `package.json`."), "2026-07-05T00:00:01.000Z"), + at(tokenUsage(counts(400), counts(400)), "2026-07-05T00:00:02.000Z"), + at(assistantText("Installation finished."), "2026-07-05T00:00:03.000Z"), + at(tokenUsage(counts(700), counts(300)), "2026-07-05T00:00:04.000Z"), + ]); + + finalizeHistory(m); + + const stats = items(m).find((i) => i.kind === "task_stats") as TaskStatsItem; + expect(stats.assistantText).toBe("Creating `package.json`.\n\nInstallation finished."); + }); + it("stream end (finalizeHistory) closes the last Task; rounds without usage get no stats figures but still get a footer", () => { const m = createStreamModel(); pushMessages(m, [