mirror of
https://github.com/logancyang/obsidian-copilot.git
synced 2026-07-22 07:50:24 +00:00
fix(agent-mode): coalesce streamed prose split by an invisible trail part
A streamed agent message renders each `text` part as its own block-level markdown div, so when a top-level-invisible part — a hidden ToolSearch, an empty plan, or a concurrent background sub-agent's event — lands between two prose deltas, one sentence splits into two stacked blocks and shows a spurious mid-sentence line break in the live UI. The saved markdown, built from the flat `displayText` (deltas concatenated with no separator), has no break, so the artifact is render-only and self-heals on reload. Add a shared `isTopLevelInvisible` predicate and apply it across all three consumers of the parts stream: - `foldNodes` skips empty plans and coalesces adjacent text nodes, so the live trail matches the saved `displayText`. - `agentResponseText` derives from the folded trail, so Copy/Insert no longer injects a blank line into a split sentence (a visible tool card / reasoning / non-empty plan between prose still yields a real `\n\n` break). - `splitTrailingText` treats invisible parts as transparent and the collapsed "Worked for X" view renders the final run as one block, so the collapse can't strand half a sentence in the hidden research. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
a78b38c8c2
commit
8d92e2d331
3 changed files with 212 additions and 30 deletions
|
|
@ -78,10 +78,11 @@ export const AgentTrail: React.FC<AgentTrailProps> = ({
|
|||
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} />
|
||||
))}
|
||||
{/* `final` may be several text parts only because a top-level-invisible
|
||||
part (a background sub-agent event, a dropped ToolSearch, an empty
|
||||
plan) split one prose run; join with "" to render the original
|
||||
sentence as one block, not a broken-apart paragraph. */}
|
||||
<AgentMarkdownText text={final.map((p) => p.text).join("")} app={app} />
|
||||
{actions}
|
||||
</div>
|
||||
);
|
||||
|
|
|
|||
|
|
@ -27,6 +27,17 @@ function text(value: string): AgentMessagePart {
|
|||
return { kind: "text", text: value };
|
||||
}
|
||||
|
||||
function plan(contents: string[]): AgentMessagePart {
|
||||
return {
|
||||
kind: "plan",
|
||||
entries: contents.map((content) => ({
|
||||
content,
|
||||
priority: "medium" as const,
|
||||
status: "pending" as const,
|
||||
})),
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Wrap a list of tool_call parts as children of a sub-agent (Task) so the
|
||||
* trail builder treats them as depth-1 peers — the level where compaction
|
||||
|
|
@ -301,6 +312,65 @@ describe("buildAgentTrail", () => {
|
|||
const tree = buildAgentTrail(parts);
|
||||
expect(tree.map((n) => n.type)).toEqual(["reasoning", "plan"]);
|
||||
});
|
||||
|
||||
describe("coalesces a prose run split by a top-level-invisible part", () => {
|
||||
it("merges text around a hidden ToolSearch into one block", () => {
|
||||
const parts: AgentMessagePart[] = [
|
||||
text("let the "),
|
||||
tool("ts", { vendorToolName: "ToolSearch" }),
|
||||
text("kid feel the change"),
|
||||
];
|
||||
const tree = buildAgentTrail(parts);
|
||||
expect(tree).toHaveLength(1);
|
||||
expect(tree[0].type).toBe("text");
|
||||
if (tree[0].type === "text") {
|
||||
expect(tree[0].part.text).toBe("let the kid feel the change");
|
||||
}
|
||||
});
|
||||
|
||||
it("merges text around an empty plan into one block", () => {
|
||||
const parts: AgentMessagePart[] = [text("let the "), plan([]), text("kid feel the change")];
|
||||
const tree = buildAgentTrail(parts);
|
||||
expect(tree).toHaveLength(1);
|
||||
expect(tree[0].type).toBe("text");
|
||||
if (tree[0].type === "text") {
|
||||
expect(tree[0].part.text).toBe("let the kid feel the change");
|
||||
}
|
||||
});
|
||||
|
||||
it("merges text around a sub-agent child, keeping the sub-agent card", () => {
|
||||
// A backgrounded sub-agent's child event lands between two prose deltas;
|
||||
// it renders nested under the parent card (not a top-level peer), so the
|
||||
// prose must stay one block — this is the real-world repro.
|
||||
const parts = withSubagent("task1", [
|
||||
text("let the "),
|
||||
tool("c1", { vendorToolName: "Read" }),
|
||||
text("kid feel the change"),
|
||||
]);
|
||||
const tree = buildAgentTrail(parts);
|
||||
expect(tree.map((n) => n.type)).toEqual(["subagent", "text"]);
|
||||
const node = tree[1];
|
||||
if (node.type === "text") {
|
||||
expect(node.part.text).toBe("let the kid feel the change");
|
||||
}
|
||||
});
|
||||
|
||||
it("keeps two blocks when a VISIBLE tool splits the prose", () => {
|
||||
const parts: AgentMessagePart[] = [
|
||||
text("before "),
|
||||
tool("x", { vendorToolName: "Read" }),
|
||||
text("after"),
|
||||
];
|
||||
const tree = buildAgentTrail(parts);
|
||||
expect(tree.map((n) => n.type)).toEqual(["text", "action", "text"]);
|
||||
});
|
||||
|
||||
it("keeps two blocks when a non-empty plan splits the prose", () => {
|
||||
const parts: AgentMessagePart[] = [text("before "), plan(["step 1"]), text("after")];
|
||||
const tree = buildAgentTrail(parts);
|
||||
expect(tree.map((n) => n.type)).toEqual(["text", "plan", "text"]);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("splitTrailingText", () => {
|
||||
|
|
@ -344,6 +414,33 @@ describe("splitTrailingText", () => {
|
|||
expect(research).toHaveLength(2);
|
||||
expect(final).toEqual([]);
|
||||
});
|
||||
|
||||
it("keeps a sentence whole in final when an empty plan splits it", () => {
|
||||
const parts: AgentMessagePart[] = [
|
||||
tool("a"),
|
||||
text("intro"),
|
||||
tool("b"),
|
||||
text("let the "),
|
||||
plan([]),
|
||||
text("kid feel the change"),
|
||||
];
|
||||
const { research, final } = splitTrailingText(parts);
|
||||
expect(final.map((p) => p.text)).toEqual(["let the ", "kid feel the change"]);
|
||||
// The empty plan is invisible but stays in research; the visible `tool b`
|
||||
// is the real boundary.
|
||||
expect(research.map((p) => p.kind)).toEqual(["tool_call", "text", "tool_call", "plan"]);
|
||||
});
|
||||
|
||||
it("a sub-agent child in the trailing run stays transparent", () => {
|
||||
const parts = withSubagent("task1", [
|
||||
text("let the "),
|
||||
tool("c1"),
|
||||
text("kid feel the change"),
|
||||
]);
|
||||
const { research, final } = splitTrailingText(parts);
|
||||
expect(final.map((p) => p.text)).toEqual(["let the ", "kid feel the change"]);
|
||||
expect(research.map((p) => p.kind)).toEqual(["tool_call", "tool_call"]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("agentResponseText", () => {
|
||||
|
|
@ -372,9 +469,34 @@ describe("agentResponseText", () => {
|
|||
expect(agentResponseText(parts)).toBe("A\n\nB");
|
||||
});
|
||||
|
||||
it("drops a whitespace-only text part without leaving a stray blank line", () => {
|
||||
it("coalesces fragments separated only by a dropped whitespace part", () => {
|
||||
const parts: AgentMessagePart[] = [text("A"), text(" "), text("B")];
|
||||
expect(agentResponseText(parts)).toBe("A\n\nB");
|
||||
// The whitespace part renders nothing, so the fragments are one prose run —
|
||||
// joined with no separator, not a blank-line paragraph break.
|
||||
expect(agentResponseText(parts)).toBe("AB");
|
||||
});
|
||||
|
||||
it("joins prose split by a hidden ToolSearch with no break", () => {
|
||||
const parts: AgentMessagePart[] = [
|
||||
text("let the "),
|
||||
tool("ts", { vendorToolName: "ToolSearch" }),
|
||||
text("kid feel the change"),
|
||||
];
|
||||
expect(agentResponseText(parts)).toBe("let the kid feel the change");
|
||||
});
|
||||
|
||||
it("joins prose split by an empty plan with no break", () => {
|
||||
const parts: AgentMessagePart[] = [text("let the "), plan([]), text("kid feel the change")];
|
||||
expect(agentResponseText(parts)).toBe("let the kid feel the change");
|
||||
});
|
||||
|
||||
it("joins prose split by a concurrent sub-agent child with no break", () => {
|
||||
const parts = withSubagent("task1", [
|
||||
text("let the "),
|
||||
tool("c1"),
|
||||
text("kid feel the change"),
|
||||
]);
|
||||
expect(agentResponseText(parts)).toBe("let the kid feel the change");
|
||||
});
|
||||
|
||||
it("sanitizes the text the same way legacy chat copy does", () => {
|
||||
|
|
|
|||
|
|
@ -27,9 +27,13 @@ export interface BuildAgentTrailOptions {
|
|||
|
||||
/**
|
||||
* 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.
|
||||
* that came before (the "research"). The boundary is the last run of `text`
|
||||
* parts; a *visible* `tool_call`, `thought`, or non-empty `plan` after a text
|
||||
* run reclassifies that run as research. Top-level-invisible parts (hidden
|
||||
* ToolSearch, sub-agent children, an empty plan) are transparent: they stay in
|
||||
* `research` but do not break the trailing run, so a sentence split by one (e.g.
|
||||
* a background sub-agent event arriving mid-prose) stays whole in `final`. The
|
||||
* caller renders `final` as a single coalesced block to match.
|
||||
*
|
||||
* 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`).
|
||||
|
|
@ -40,39 +44,46 @@ export function splitTrailingText(parts: AgentMessagePart[]): {
|
|||
research: AgentMessagePart[];
|
||||
final: TextPart[];
|
||||
} {
|
||||
const final: TextPart[] = [];
|
||||
let boundary = parts.length;
|
||||
const turnToolIds = toolCallIds(parts);
|
||||
const finalIdx = new Set<number>();
|
||||
for (let i = parts.length - 1; i >= 0; i--) {
|
||||
const p = parts[i];
|
||||
if (p.kind === "text") {
|
||||
final.unshift(p);
|
||||
boundary = i;
|
||||
finalIdx.add(i);
|
||||
continue;
|
||||
}
|
||||
if (isTopLevelInvisible(p, turnToolIds)) continue;
|
||||
break;
|
||||
}
|
||||
return { research: parts.slice(0, boundary), final };
|
||||
const research: AgentMessagePart[] = [];
|
||||
const final: TextPart[] = [];
|
||||
parts.forEach((p, i) => {
|
||||
if (finalIdx.has(i)) final.push(p as TextPart);
|
||||
else research.push(p);
|
||||
});
|
||||
return { research, 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),
|
||||
* joined and run through the same sanitization legacy chat applies
|
||||
* (`cleanMessageForCopy`), so tool-call cards, reasoning, plans, and chat-only
|
||||
* artifacts never leak in. Interleaving research (a `thought` or `tool_call`
|
||||
* between two prose chunks) must not drop the earlier prose, so we collect all
|
||||
* text parts rather than only the trailing run. Empty/whitespace-only parts are
|
||||
* skipped so they don't leave stray blank lines.
|
||||
* or the editor. Derived from the folded trail so Copy/Insert matches what the
|
||||
* user sees: each top-level `text` node is one prose block — the trail coalesces
|
||||
* a run split only by a top-level-invisible part (hidden ToolSearch, sub-agent
|
||||
* child, empty plan) back into one — and distinct blocks (separated by a
|
||||
* *visible* tool card, reasoning, or non-empty plan) join with a blank line.
|
||||
* Interleaving research must not drop earlier prose, so every block is kept, not
|
||||
* just the trailing run. Run through the same sanitization legacy chat applies
|
||||
* (`cleanMessageForCopy`) so tool cards, reasoning, and chat-only artifacts
|
||||
* never leak in.
|
||||
* Returns `""` when the turn produced no prose (a tool-only turn, or one
|
||||
* cancelled mid-tool) — the trail UI uses that to gate the Copy / Insert
|
||||
* affordances off so they never sit under an empty bubble.
|
||||
*/
|
||||
export function agentResponseText(parts: AgentMessagePart[]): string {
|
||||
const text = parts
|
||||
.filter((p): p is TextPart => p.kind === "text" && p.text.trim().length > 0)
|
||||
.map((p) => p.text)
|
||||
.join("\n\n");
|
||||
return cleanMessageForCopy(text);
|
||||
const blocks = buildAgentTrail(parts)
|
||||
.filter((n): n is Extract<RenderNode, { type: "text" }> => n.type === "text")
|
||||
.map((n) => n.part.text);
|
||||
return cleanMessageForCopy(blocks.join("\n\n"));
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -99,6 +110,36 @@ function isHiddenTool(part: AgentMessagePart): boolean {
|
|||
return part.kind === "tool_call" && part.vendorToolName === "ToolSearch";
|
||||
}
|
||||
|
||||
/** Every `tool_call` id present in a turn — used to recognize sub-agent children
|
||||
* (a `tool_call` whose `parentToolCallId` names one of these). */
|
||||
function toolCallIds(parts: AgentMessagePart[]): Set<string> {
|
||||
const ids = new Set<string>();
|
||||
for (const p of parts) if (p.kind === "tool_call") ids.add(p.id);
|
||||
return ids;
|
||||
}
|
||||
|
||||
/**
|
||||
* Parts that render to nothing at the *top level* of the trail: the hidden
|
||||
* `ToolSearch` loader, an empty `plan` (`PlanPill` returns null), and sub-agent
|
||||
* children (shown nested under their parent's card, never as a top-level peer).
|
||||
*
|
||||
* A prose run split only by these is one block: the store concatenates the
|
||||
* streamed deltas into `displayText` with no separator, so a part that leaves no
|
||||
* visible peer between two prose chunks must not turn them into two paragraphs.
|
||||
* This is the difference between a real `\n\n` break (a visible tool card /
|
||||
* reasoning / non-empty plan sits between the chunks) and a spurious one (e.g. a
|
||||
* background sub-agent event arriving mid-sentence).
|
||||
*/
|
||||
function isTopLevelInvisible(part: AgentMessagePart, turnToolIds: Set<string>): boolean {
|
||||
if (isHiddenTool(part)) return true;
|
||||
if (part.kind === "plan") return part.entries.length === 0;
|
||||
return (
|
||||
part.kind === "tool_call" &&
|
||||
part.parentToolCallId !== undefined &&
|
||||
turnToolIds.has(part.parentToolCallId)
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Fold a flat `AgentMessagePart[]` into a render tree. Compaction folds
|
||||
* runs of `N >= 2` consecutive same-`toolKey` peers into one aggregate
|
||||
|
|
@ -169,16 +210,34 @@ function foldNodes(
|
|||
}
|
||||
if (p.kind === "text") {
|
||||
// Streamed prose breaks compaction (design doc §"Compaction"): a text
|
||||
// part between two same-tool calls disqualifies grouping. Pushing
|
||||
// straight to `out` here naturally enforces that — the next tool_call
|
||||
// can't see a prior aggregate/action of the same key as `prev`.
|
||||
// node between two same-tool calls disqualifies grouping. Keeping the
|
||||
// text in `out` enforces that — the next tool_call can't see a prior
|
||||
// aggregate/action of the same key as `prev`.
|
||||
// Skip empty/whitespace-only text parts so they don't become a flex
|
||||
// child contributing `gap-1` plus their own padding to the trail.
|
||||
if (p.text.trim().length === 0) continue;
|
||||
out.push({ type: "text", part: p });
|
||||
// Coalesce with the previous text node when one is adjacent. Two text
|
||||
// parts only become adjacent after a top-level-invisible part (hidden
|
||||
// ToolSearch, sub-agent child, empty plan) was dropped between them, so
|
||||
// merging reconstructs the original prose run — byte-identical to the
|
||||
// flat `displayText` the store saved — instead of two stacked blocks that
|
||||
// read as a spurious mid-sentence line break.
|
||||
const prevNode = out[out.length - 1];
|
||||
if (prevNode && prevNode.type === "text") {
|
||||
out[out.length - 1] = {
|
||||
type: "text",
|
||||
part: { kind: "text", text: prevNode.part.text + p.text },
|
||||
};
|
||||
} else {
|
||||
out.push({ type: "text", part: p });
|
||||
}
|
||||
continue;
|
||||
}
|
||||
if (p.kind === "plan") {
|
||||
// An empty plan renders nothing (`PlanPill` returns null); emitting it as
|
||||
// a node would wedge between two prose parts and block the coalescing
|
||||
// above, leaving the spurious break. Drop it so the prose stays one block.
|
||||
if (p.entries.length === 0) continue;
|
||||
out.push({ type: "plan", part: p });
|
||||
continue;
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue