mirror of
https://github.com/logancyang/obsidian-copilot.git
synced 2026-07-22 07:50:24 +00:00
fix(agent-mode): stop folding the completed trail into "Worked for X" (#2652)
* fix(agent-mode): always render the completed trail inline Remove the "Worked for X" collapse that folded everything before the final text block once a turn cleanly ended. It hid content the user was mid-read on and broke reading flow; a finished turn now renders exactly like the live streaming view — nothing is hidden. Also drops the now-unused splitTrailingText helper and adds a regression test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor(agent-mode): drop dead turnDurationMs plumbing With the "Worked for X" collapse removed, the turn's wall-clock duration is never read. Remove turnDurationMs end-to-end (session, store, and the persisted message type) and the turnStartedAt bookkeeping that existed solely to compute it. turnStopReason is fully preserved. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Zero Liu
GitHub
parent
703529ac67
commit
4cfea5761d
9 changed files with 42 additions and 198 deletions
|
|
@ -267,7 +267,7 @@ describe("AgentMessageStore", () => {
|
|||
const v2 = store.getDisplayMessages()[0];
|
||||
expect(v2).not.toBe(v1);
|
||||
|
||||
store.markTurnComplete(id, "end_turn", 10);
|
||||
store.markTurnComplete(id, "end_turn");
|
||||
const v3 = store.getDisplayMessages()[0];
|
||||
expect(v3).not.toBe(v2);
|
||||
expect(v3.turnStopReason).toBe("end_turn");
|
||||
|
|
|
|||
|
|
@ -31,7 +31,6 @@ interface StoredAgentMessage {
|
|||
context?: MessageContext;
|
||||
content?: unknown[];
|
||||
turnStopReason?: StopReason;
|
||||
turnDurationMs?: number;
|
||||
// Live per-agent fan-out state. In-memory only; the persisted body carries the
|
||||
// composite that reconstructs it on load.
|
||||
fanout?: FanoutTurn;
|
||||
|
|
@ -234,7 +233,6 @@ export class AgentMessageStore {
|
|||
content: message.content,
|
||||
parts: message.parts,
|
||||
turnStopReason: message.turnStopReason,
|
||||
turnDurationMs: message.turnDurationMs,
|
||||
version: 0,
|
||||
});
|
||||
this.lastDisplay = null;
|
||||
|
|
@ -242,16 +240,15 @@ export class AgentMessageStore {
|
|||
}
|
||||
|
||||
/**
|
||||
* Stamp a finished turn's `stopReason` and frozen `durationMs` onto its
|
||||
* placeholder assistant message. Returns false if the message is missing or
|
||||
* already marked complete — the latter lets callers skip notifying.
|
||||
* Stamp a finished turn's `stopReason` onto its placeholder assistant message.
|
||||
* Returns false if the message is missing or already marked complete — the
|
||||
* latter lets callers skip notifying.
|
||||
*/
|
||||
markTurnComplete(id: string, stopReason: StopReason, durationMs: number): boolean {
|
||||
markTurnComplete(id: string, stopReason: StopReason): boolean {
|
||||
const msg = this.messages.find((m) => m.id === id);
|
||||
if (!msg) return false;
|
||||
if (msg.turnStopReason !== undefined) return false;
|
||||
msg.turnStopReason = stopReason;
|
||||
msg.turnDurationMs = durationMs;
|
||||
this.touch(msg);
|
||||
return true;
|
||||
}
|
||||
|
|
@ -461,7 +458,6 @@ export class AgentMessageStore {
|
|||
content: msg.content,
|
||||
parts: msg.parts,
|
||||
turnStopReason: msg.turnStopReason,
|
||||
turnDurationMs: msg.turnDurationMs,
|
||||
fanout,
|
||||
version: 0,
|
||||
});
|
||||
|
|
@ -488,7 +484,6 @@ export class AgentMessageStore {
|
|||
content: m.content,
|
||||
parts: m.parts,
|
||||
turnStopReason: m.turnStopReason,
|
||||
turnDurationMs: m.turnDurationMs,
|
||||
// Snapshot so each adapted view carries a fresh reference; the orchestrator
|
||||
// mutates one turn in place, so the same reference would freeze the dropdown.
|
||||
...(m.fanout ? { fanout: snapshotFanoutTurn(m.fanout) } : {}),
|
||||
|
|
|
|||
|
|
@ -959,7 +959,6 @@ export class AgentSession {
|
|||
): Promise<StopReason> {
|
||||
const placeholderId = this.placeholderId;
|
||||
const sessionId = this.backendSessionId!;
|
||||
const turnStartedAt = Date.now();
|
||||
try {
|
||||
// Extract live Web Viewer content (reader-mode markdown, YouTube
|
||||
// transcripts) just before the prompt is built so it reflects the page
|
||||
|
|
@ -988,7 +987,7 @@ export class AgentSession {
|
|||
// gate can be bypassed (pasting a pill), so re-check entitlement here at
|
||||
// the session boundary. Paying users short-circuit; everyone else is hard-blocked.
|
||||
if (!(await this.ensureMultiAgentEntitlement())) {
|
||||
return this.blockFanoutForEntitlement(placeholderId, turnStartedAt);
|
||||
return this.blockFanoutForEntitlement(placeholderId);
|
||||
}
|
||||
|
||||
// Give every fan-out agent the PRIOR visible transcript as a read-only
|
||||
|
|
@ -1012,7 +1011,7 @@ export class AgentSession {
|
|||
// the next normal turn skip the block, permanently stripping the project
|
||||
// manifest from the main chat. Leaving it unset lets that turn deliver it
|
||||
// (and each ephemeral fan-out, being memoryless, re-receives it meanwhile).
|
||||
return await this.runFanoutPath(placeholderId, displayText, promptBlocks, turnStartedAt);
|
||||
return await this.runFanoutPath(placeholderId, displayText, promptBlocks);
|
||||
}
|
||||
|
||||
// Single-agent path: prepend any buffered fan-out turns as one labeled
|
||||
|
|
@ -1057,10 +1056,7 @@ export class AgentSession {
|
|||
);
|
||||
this.store.markMessageError(placeholderId, message);
|
||||
}
|
||||
if (
|
||||
placeholderId &&
|
||||
this.store.markTurnComplete(placeholderId, resp.stopReason, Date.now() - turnStartedAt)
|
||||
) {
|
||||
if (placeholderId && this.store.markTurnComplete(placeholderId, resp.stopReason)) {
|
||||
this.notifyMessages();
|
||||
}
|
||||
// Some backends flush the prompt result before the turn's last content
|
||||
|
|
@ -1110,13 +1106,13 @@ export class AgentSession {
|
|||
* Clean up a paywall-blocked fan-out turn: surface the upgrade prompt and
|
||||
* finalize the placeholder as an error so no dangling bubble remains.
|
||||
*/
|
||||
private blockFanoutForEntitlement(placeholderId: string, turnStartedAt: number): StopReason {
|
||||
private blockFanoutForEntitlement(placeholderId: string): StopReason {
|
||||
showMultiAgentUpgradePrompt();
|
||||
this.store.markMessageError(
|
||||
placeholderId,
|
||||
"Multi-agent QA is a Copilot Plus feature. Upgrade to mention more than one agent in a turn."
|
||||
);
|
||||
this.store.markTurnComplete(placeholderId, "refusal", Date.now() - turnStartedAt);
|
||||
this.store.markTurnComplete(placeholderId, "refusal");
|
||||
this.currentMessageIds = new Set();
|
||||
if (this.placeholderId === placeholderId) this.placeholderId = null;
|
||||
this.notifyMessages();
|
||||
|
|
@ -1133,8 +1129,7 @@ export class AgentSession {
|
|||
private async runFanoutPath(
|
||||
placeholderId: string,
|
||||
originalPromptText: string,
|
||||
promptBlocks: PromptContent[],
|
||||
turnStartedAt: number
|
||||
promptBlocks: PromptContent[]
|
||||
): Promise<StopReason> {
|
||||
const signal = this.abortController?.signal ?? new AbortController().signal;
|
||||
const input: FanoutRunInput = {
|
||||
|
|
@ -1181,7 +1176,7 @@ export class AgentSession {
|
|||
this.pendingFanoutContext.push({ question: originalPromptText, summary: replay });
|
||||
}
|
||||
}
|
||||
if (this.store.markTurnComplete(placeholderId, stopReason, Date.now() - turnStartedAt)) {
|
||||
if (this.store.markTurnComplete(placeholderId, stopReason)) {
|
||||
this.notifyMessages();
|
||||
}
|
||||
if (this.placeholderId === placeholderId) this.placeholderId = null;
|
||||
|
|
|
|||
|
|
@ -888,12 +888,6 @@ export interface AgentChatMessage {
|
|||
* leave the trail uncollapsed so the user sees what happened.
|
||||
*/
|
||||
turnStopReason?: StopReason;
|
||||
/**
|
||||
* Wall-clock ms the turn took, frozen at `prompt()` resolution. Stored
|
||||
* so re-renders don't shift the "Worked for X" label. Absent until the
|
||||
* turn ends.
|
||||
*/
|
||||
turnDurationMs?: number;
|
||||
/**
|
||||
* Per-agent fan-out state when this assistant message is a multi-agent QA turn.
|
||||
* LIVE in-memory only — persistence rides in the body as a composite
|
||||
|
|
|
|||
|
|
@ -181,7 +181,6 @@ const AgentChatMessages = memo(
|
|||
showThinkingTail={message.id === streamingMessageId && showBottomLoader}
|
||||
app={app}
|
||||
turnStopReason={message.turnStopReason}
|
||||
turnDurationMs={message.turnDurationMs}
|
||||
/>
|
||||
</div>
|
||||
) : (
|
||||
|
|
|
|||
|
|
@ -107,3 +107,28 @@ describe("AgentTrail copy / insert actions", () => {
|
|||
expect(screen.queryByTitle("Insert / Replace at cursor")).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe("AgentTrail inline trail (no collapse)", () => {
|
||||
beforeEach(() => {
|
||||
(window as unknown as { activeDocument: Document }).activeDocument = window.document;
|
||||
});
|
||||
|
||||
it("renders research inline with no 'Worked for' toggle on a completed research+answer turn", () => {
|
||||
renderTrail({
|
||||
parts: [
|
||||
// Multi-word title with no vendorToolName renders verbatim as the
|
||||
// ActionCard's collapsed line (GENERIC_SUMMARY → genericToolLabel).
|
||||
{ kind: "tool_call", id: "t1", title: "Search vault", status: "completed" },
|
||||
text("The final answer."),
|
||||
],
|
||||
turnStopReason: "end_turn",
|
||||
});
|
||||
|
||||
// The "Worked for X" collapse is gone: the whole trail renders inline.
|
||||
expect(screen.queryByText(/Worked for/i)).toBeNull();
|
||||
// The trailing prose renders as the final answer.
|
||||
expect(screen.getByText("The final answer.")).toBeTruthy();
|
||||
// The research tool card renders inline (not folded behind a toggle).
|
||||
expect(screen.getByText("Search vault")).toBeTruthy();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,10 +1,5 @@
|
|||
import React, { useState } from "react";
|
||||
import {
|
||||
agentResponseText,
|
||||
buildAgentTrail,
|
||||
splitTrailingText,
|
||||
type RenderNode,
|
||||
} from "@/agentMode/ui/agentTrail";
|
||||
import React from "react";
|
||||
import { agentResponseText, buildAgentTrail, type RenderNode } from "@/agentMode/ui/agentTrail";
|
||||
import type { AgentMessagePart, StopReason } from "@/agentMode/session/types";
|
||||
import { ActionCard } from "@/agentMode/ui/ActionCard";
|
||||
import { AgentMessageActions } from "@/agentMode/ui/AgentMessageActions";
|
||||
|
|
@ -14,10 +9,6 @@ import { ReasoningBlock } from "@/agentMode/ui/ReasoningBlock";
|
|||
import { AgentMarkdownText } from "@/agentMode/ui/AgentMarkdownText";
|
||||
import { planEntryClass, planEntryIcon } from "@/agentMode/ui/planEntryStyles";
|
||||
import { BottomLoadingIndicator } from "@/components/chat-components/BottomLoadingIndicator";
|
||||
import { Collapsible, CollapsibleContent, CollapsibleTrigger } from "@/components/ui/collapsible";
|
||||
import { formatDuration } from "@/lib/duration";
|
||||
import { cn } from "@/lib/utils";
|
||||
import { Sparkles, ChevronRight } from "lucide-react";
|
||||
import { App } from "obsidian";
|
||||
|
||||
interface AgentTrailProps {
|
||||
|
|
@ -32,13 +23,9 @@ interface AgentTrailProps {
|
|||
showThinkingTail?: boolean;
|
||||
/** Obsidian `App` for the markdown renderer used by `text` parts. */
|
||||
app: App;
|
||||
/** Backend stopReason once the turn has ended. Only `end_turn` triggers
|
||||
* collapsing — cancelled / refusal / max_tokens leave the trail expanded
|
||||
* so the user can see exactly where things stopped. */
|
||||
/** Backend stopReason once the turn has ended. Only `cancelled` suppresses
|
||||
* the Copy / Insert affordances (treated as having no user-visible answer). */
|
||||
turnStopReason?: StopReason;
|
||||
/** Frozen wall-clock duration of the turn, in ms. Drives the
|
||||
* "Worked for X" label on a collapsed turn. */
|
||||
turnDurationMs?: number;
|
||||
}
|
||||
|
||||
export const AgentTrail: React.FC<AgentTrailProps> = ({
|
||||
|
|
@ -47,17 +34,7 @@ export const AgentTrail: React.FC<AgentTrailProps> = ({
|
|||
showThinkingTail,
|
||||
app,
|
||||
turnStopReason,
|
||||
turnDurationMs,
|
||||
}) => {
|
||||
// Decide whether the turn qualifies for the "Worked for X" collapse. The
|
||||
// collapse is post-stream only — while events are still arriving the user
|
||||
// sees every tool call land in real time.
|
||||
const canCollapse =
|
||||
!isStreaming &&
|
||||
turnStopReason === "end_turn" &&
|
||||
typeof turnDurationMs === "number" &&
|
||||
parts.length > 0;
|
||||
|
||||
// Copy / Insert act on the agent's full textual response. Gate them off while
|
||||
// the message is still streaming and on cancelled turns (treated as having no
|
||||
// user-visible answer), plus whenever there is no prose to act on.
|
||||
|
|
@ -67,27 +44,6 @@ export const AgentTrail: React.FC<AgentTrailProps> = ({
|
|||
<AgentMessageActions text={answer} app={app} />
|
||||
) : null;
|
||||
|
||||
if (canCollapse) {
|
||||
const { research, final } = splitTrailingText(parts);
|
||||
// Both halves must be non-empty for a collapse to be meaningful: research
|
||||
// gives the user something to hide, and final gives them something to read
|
||||
// inline. If either is missing, fall through to the linear render.
|
||||
const researchHasContent = research.some((p) => p.kind !== "text" || p.text.trim().length > 0);
|
||||
const finalHasContent = final.some((p) => p.text.trim().length > 0);
|
||||
if (researchHasContent && finalHasContent) {
|
||||
return (
|
||||
<div className="tw-group tw-flex tw-flex-col tw-gap-1">
|
||||
<WorkedForBlock research={research} durationMs={turnDurationMs} app={app} />
|
||||
{final.map((p, i) => (
|
||||
// eslint-disable-next-line @eslint-react/no-array-index-key -- text parts are append-only and may contain duplicate text
|
||||
<AgentMarkdownText key={`final-${i}`} text={p.text} app={app} />
|
||||
))}
|
||||
{actions}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
return (
|
||||
<div className="tw-group tw-flex tw-flex-col tw-gap-1">
|
||||
<LinearTrail
|
||||
|
|
@ -126,49 +82,6 @@ const LinearTrail: React.FC<{
|
|||
);
|
||||
};
|
||||
|
||||
interface WorkedForBlockProps {
|
||||
research: AgentMessagePart[];
|
||||
durationMs: number;
|
||||
app: App;
|
||||
}
|
||||
|
||||
/**
|
||||
* Collapsed-by-default "Worked for X" header that wraps the research portion
|
||||
* of a completed turn. Clicking expands the original trail inline — same
|
||||
* components as the linear path, no streaming spinners (the turn has ended).
|
||||
*/
|
||||
const WorkedForBlock: React.FC<WorkedForBlockProps> = ({ research, durationMs, app }) => {
|
||||
const [isExpanded, setIsExpanded] = useState(false);
|
||||
return (
|
||||
<Collapsible
|
||||
open={isExpanded}
|
||||
onOpenChange={setIsExpanded}
|
||||
className="tw-mb-2 tw-mt-1 tw-w-full max-md:tw-mb-1.5 max-md:tw-mt-0.5"
|
||||
>
|
||||
<CollapsibleTrigger asChild>
|
||||
<div className="copilot-divider-b tw-flex tw-w-full tw-cursor-pointer tw-items-center tw-gap-1.5 tw-pb-2 tw-text-left tw-text-sm tw-text-muted hover:tw-text-normal">
|
||||
<span className="tw-flex tw-size-icon-xs tw-shrink-0 tw-items-center tw-justify-center">
|
||||
<Sparkles className="tw-size-3 tw-text-muted" />
|
||||
</span>
|
||||
<span className="tw-font-medium">Worked for</span>
|
||||
<span className="tw-text-muted">{formatDuration(durationMs)}</span>
|
||||
<ChevronRight
|
||||
className={cn(
|
||||
"tw-ml-auto tw-size-3 tw-text-muted tw-transition-transform",
|
||||
isExpanded && "tw-rotate-90"
|
||||
)}
|
||||
/>
|
||||
</div>
|
||||
</CollapsibleTrigger>
|
||||
<CollapsibleContent>
|
||||
<div className="tw-mt-2">
|
||||
<LinearTrail parts={research} isStreaming={false} app={app} />
|
||||
</div>
|
||||
</CollapsibleContent>
|
||||
</Collapsible>
|
||||
);
|
||||
};
|
||||
|
||||
function renderNode(
|
||||
node: RenderNode,
|
||||
key: string | number,
|
||||
|
|
|
|||
|
|
@ -1,9 +1,4 @@
|
|||
import {
|
||||
agentResponseText,
|
||||
buildAgentTrail,
|
||||
splitTrailingText,
|
||||
type RenderNode,
|
||||
} from "@/agentMode/ui/agentTrail";
|
||||
import { agentResponseText, buildAgentTrail, type RenderNode } from "@/agentMode/ui/agentTrail";
|
||||
import type { AgentMessagePart } from "@/agentMode/session/types";
|
||||
|
||||
function tool(
|
||||
|
|
@ -303,49 +298,6 @@ describe("buildAgentTrail", () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe("splitTrailingText", () => {
|
||||
it("returns empty final when there are no trailing text parts", () => {
|
||||
const parts: AgentMessagePart[] = [tool("a"), thought("...")];
|
||||
const { research, final } = splitTrailingText(parts);
|
||||
expect(research).toEqual(parts);
|
||||
expect(final).toEqual([]);
|
||||
});
|
||||
|
||||
it("groups all trailing text parts as the final answer", () => {
|
||||
const parts: AgentMessagePart[] = [
|
||||
tool("a"),
|
||||
text("intermediate"),
|
||||
tool("b"),
|
||||
text("final part 1"),
|
||||
text("final part 2"),
|
||||
];
|
||||
const { research, final } = splitTrailingText(parts);
|
||||
expect(research.map((p) => p.kind)).toEqual(["tool_call", "text", "tool_call"]);
|
||||
expect(final.map((p) => p.text)).toEqual(["final part 1", "final part 2"]);
|
||||
});
|
||||
|
||||
it("treats a tool_call after text as research, not final", () => {
|
||||
const parts: AgentMessagePart[] = [tool("a"), text("midway answer"), tool("b")];
|
||||
const { research, final } = splitTrailingText(parts);
|
||||
expect(research).toHaveLength(3);
|
||||
expect(final).toEqual([]);
|
||||
});
|
||||
|
||||
it("returns empty research when the whole turn is text", () => {
|
||||
const parts: AgentMessagePart[] = [text("just"), text(" text")];
|
||||
const { research, final } = splitTrailingText(parts);
|
||||
expect(research).toEqual([]);
|
||||
expect(final.map((p) => p.text)).toEqual(["just", " text"]);
|
||||
});
|
||||
|
||||
it("trailing thought breaks the run (thought is research)", () => {
|
||||
const parts: AgentMessagePart[] = [text("answer"), thought("post-hoc reasoning")];
|
||||
const { research, final } = splitTrailingText(parts);
|
||||
expect(research).toHaveLength(2);
|
||||
expect(final).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("agentResponseText", () => {
|
||||
it("collects all text parts in stream order, even across interleaved research", () => {
|
||||
const parts: AgentMessagePart[] = [
|
||||
|
|
|
|||
|
|
@ -25,35 +25,6 @@ export interface BuildAgentTrailOptions {
|
|||
maxDepth?: number;
|
||||
}
|
||||
|
||||
/**
|
||||
* Split a turn's parts into the trailing user-visible answer and everything
|
||||
* that came before (the "research"). The boundary is the last contiguous run
|
||||
* of `text` parts: any `tool_call`, `thought`, or `plan` part after a text
|
||||
* run reclassifies that run as research.
|
||||
*
|
||||
* Used by the trail UI to fold the research portion into a single
|
||||
* "Worked for X" block once a turn has cleanly ended (`stopReason: end_turn`).
|
||||
* Streaming turns and cancelled / refused / errored turns keep the trail
|
||||
* uncollapsed — that's the caller's responsibility, not this helper's.
|
||||
*/
|
||||
export function splitTrailingText(parts: AgentMessagePart[]): {
|
||||
research: AgentMessagePart[];
|
||||
final: TextPart[];
|
||||
} {
|
||||
const final: TextPart[] = [];
|
||||
let boundary = parts.length;
|
||||
for (let i = parts.length - 1; i >= 0; i--) {
|
||||
const p = parts[i];
|
||||
if (p.kind === "text") {
|
||||
final.unshift(p);
|
||||
boundary = i;
|
||||
continue;
|
||||
}
|
||||
break;
|
||||
}
|
||||
return { research: parts.slice(0, boundary), final };
|
||||
}
|
||||
|
||||
/**
|
||||
* The agent's full textual response across the turn, ready for the clipboard
|
||||
* or the editor: every `text` part in stream order (not just the trailing run),
|
||||
|
|
|
|||
Loading…
Reference in a new issue