fix(collab-web): dedup active transcript tools
- Dedup active tools against committed assistant tool calls as well as the stream ghost. - Hide the working shimmer while active tools are rendering, preserving it for between-tool model work. - Added focused Transcript regression coverage for duplicate cards and stale shimmer. Fixes #4948
This commit is contained in:
@@ -51,20 +51,20 @@ Decompose first, then {{#if taskBatch}}batch the independent leaves{{else}}issue
|
||||
|
||||
{{#if taskBatch}}
|
||||
task(
|
||||
context: "# Goal\nReview the auth diff...\n# Constraints\nRead-only...\n# Contract\nReturn findings as severity/file/line/fix...",
|
||||
context: "# Goal\nReview the auth diff…\n# Constraints\nRead-only…\n# Contract\nReturn findings as severity/file/line/fix…",
|
||||
tasks: [
|
||||
{ id: "AuthOwner", role: "Auth Storage Reviewer", assignment: "# Target\npackages/ai/src/auth-storage.ts\n# Change\nTrace credential selection...\n# Acceptance\nReturn confirmed findings only..." },
|
||||
{ id: "PromptOwner", role: "Prompt Contract Reviewer", assignment: "# Target\npackages/coding-agent/src/prompts/**\n# Change\nCheck active-tool guidance...\n# Acceptance\nReturn mismatches and exact prompt lines..." },
|
||||
{ id: "AuthOwner", role: "Auth Storage Reviewer", assignment: "# Target\npackages/ai/src/auth-storage.ts\n# Change\nTrace credential selection…\n# Acceptance\nReturn confirmed findings only…" },
|
||||
{ id: "PromptOwner", role: "Prompt Contract Reviewer", assignment: "# Target\npackages/coding-agent/src/prompts/**\n# Change\nCheck active-tool guidance…\n# Acceptance\nReturn mismatches and exact prompt lines…" },
|
||||
]
|
||||
)
|
||||
{{else}}
|
||||
task(
|
||||
role: "Auth Storage Reviewer",
|
||||
assignment: "# Target\npackages/ai/src/auth-storage.ts\n# Change\nReview the auth diff. Shared contract: read-only; return findings as severity/file/line/fix.\n# Acceptance\nReturn confirmed findings only..."
|
||||
assignment: "# Target\npackages/ai/src/auth-storage.ts\n# Change\nReview the auth diff. Shared contract: read-only; return findings as severity/file/line/fix.\n# Acceptance\nReturn confirmed findings only…"
|
||||
)
|
||||
task(
|
||||
role: "Prompt Contract Reviewer",
|
||||
assignment: "# Target\npackages/coding-agent/src/prompts/**\n# Change\nCheck active-tool guidance. Shared contract: read-only; return mismatches and exact prompt lines.\n# Acceptance\nReturn confirmed findings only..."
|
||||
assignment: "# Target\npackages/coding-agent/src/prompts/**\n# Change\nCheck active-tool guidance. Shared contract: read-only; return mismatches and exact prompt lines.\n# Acceptance\nReturn confirmed findings only…"
|
||||
)
|
||||
{{/if}}
|
||||
|
||||
|
||||
@@ -2,6 +2,10 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed the live collab transcript rendering duplicate tool cards and a stale `thinking…` shimmer while a committed tool call is still running. ([#4948](https://github.com/can1357/oh-my-pi/issues/4948))
|
||||
|
||||
## [16.3.7] - 2026-07-05
|
||||
|
||||
### Fixed
|
||||
|
||||
@@ -263,16 +263,22 @@ export function Transcript(props: TranscriptProps): ReactNode {
|
||||
if (el !== null && lockRef.current) el.scrollTop = el.scrollHeight;
|
||||
}, [entries, stream, activeTools, working]);
|
||||
|
||||
// Active tools not already represented as toolCall blocks in the stream ghost.
|
||||
const streamIds = new Set<string>();
|
||||
// Active tools not already represented as toolCall blocks in committed rows or the stream ghost.
|
||||
const renderedToolIds = new Set<string>();
|
||||
for (const entry of entries) {
|
||||
if (entry.type !== "message" || entry.message.role !== "assistant") continue;
|
||||
for (const block of entry.message.content) {
|
||||
if (block.type === "toolCall") renderedToolIds.add(block.id);
|
||||
}
|
||||
}
|
||||
if (stream !== null) {
|
||||
for (const block of stream.content) {
|
||||
if (block.type === "toolCall") streamIds.add(block.id);
|
||||
if (block.type === "toolCall") renderedToolIds.add(block.id);
|
||||
}
|
||||
}
|
||||
const tailTools: ActiveTool[] = [];
|
||||
for (const tool of activeTools.values()) {
|
||||
if (!streamIds.has(tool.toolCallId)) tailTools.push(tool);
|
||||
if (!renderedToolIds.has(tool.toolCallId)) tailTools.push(tool);
|
||||
}
|
||||
|
||||
return (
|
||||
@@ -317,7 +323,7 @@ export function Transcript(props: TranscriptProps): ReactNode {
|
||||
))}
|
||||
</Row>
|
||||
)}
|
||||
{working && stream === null && (
|
||||
{working && stream === null && activeTools.size === 0 && (
|
||||
<Row kind="assistant" gutter="agent">
|
||||
<div className="tr-shimmer">thinking…</div>
|
||||
</Row>
|
||||
|
||||
@@ -0,0 +1,4 @@
|
||||
class TestHTMLElement {}
|
||||
|
||||
const globals = globalThis as typeof globalThis & { HTMLElement?: typeof HTMLElement };
|
||||
globals.HTMLElement ??= TestHTMLElement as unknown as typeof HTMLElement;
|
||||
@@ -0,0 +1,109 @@
|
||||
import { describe, expect, it } from "bun:test";
|
||||
import type { AssistantMessage, SessionEntry } from "@oh-my-pi/pi-wire";
|
||||
import { renderToStaticMarkup } from "react-dom/server";
|
||||
import "./transcript-dom-shim";
|
||||
import { Transcript } from "../src/components/transcript/Transcript";
|
||||
import type { ActiveTool } from "../src/lib/client";
|
||||
|
||||
const TOOL_CALL_ID = "call-running-tool";
|
||||
const TOOL_NAME = "probe_tool";
|
||||
|
||||
function assistantUsage(): AssistantMessage["usage"] {
|
||||
return { input: 1, output: 2, cacheRead: 0, cacheWrite: 0, totalTokens: 3, cost: { total: 0 } };
|
||||
}
|
||||
|
||||
function committedAssistantToolCall(): SessionEntry {
|
||||
return {
|
||||
type: "message",
|
||||
id: "assistant-entry-1",
|
||||
parentId: null,
|
||||
timestamp: "2026-07-09T00:00:00Z",
|
||||
message: {
|
||||
role: "assistant",
|
||||
content: [
|
||||
{ type: "text", text: "I will run the tool." },
|
||||
{
|
||||
type: "toolCall",
|
||||
id: TOOL_CALL_ID,
|
||||
name: TOOL_NAME,
|
||||
arguments: { target: "fixture-input" },
|
||||
intent: "Inspect fixture input",
|
||||
},
|
||||
],
|
||||
model: "test/model",
|
||||
usage: assistantUsage(),
|
||||
stopReason: "stop",
|
||||
timestamp: 1,
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
function activeTool(): ActiveTool {
|
||||
return {
|
||||
toolCallId: TOOL_CALL_ID,
|
||||
toolName: TOOL_NAME,
|
||||
args: { target: "fixture-input" },
|
||||
intent: "Inspect fixture input",
|
||||
startedAt: 1,
|
||||
};
|
||||
}
|
||||
|
||||
function renderTranscript(props: {
|
||||
entries?: readonly SessionEntry[];
|
||||
activeTools?: ReadonlyMap<string, ActiveTool>;
|
||||
working: boolean;
|
||||
}): string {
|
||||
return renderToStaticMarkup(
|
||||
<Transcript
|
||||
entries={props.entries ?? []}
|
||||
stream={null}
|
||||
streamDone={true}
|
||||
activeTools={props.activeTools ?? new Map()}
|
||||
working={props.working}
|
||||
/>,
|
||||
);
|
||||
}
|
||||
|
||||
function countElements(html: string, selector: string): number {
|
||||
let count = 0;
|
||||
new HTMLRewriter()
|
||||
.on(selector, {
|
||||
element() {
|
||||
count++;
|
||||
},
|
||||
})
|
||||
.transform(html);
|
||||
return count;
|
||||
}
|
||||
|
||||
function countOccurrences(text: string, needle: string): number {
|
||||
let count = 0;
|
||||
let start = 0;
|
||||
while (true) {
|
||||
const index = text.indexOf(needle, start);
|
||||
if (index === -1) return count;
|
||||
count++;
|
||||
start = index + needle.length;
|
||||
}
|
||||
}
|
||||
|
||||
describe("Transcript live tool rendering", () => {
|
||||
it("renders one running card for a committed tool call without the working shimmer", () => {
|
||||
const html = renderTranscript({
|
||||
entries: [committedAssistantToolCall()],
|
||||
activeTools: new Map([[TOOL_CALL_ID, activeTool()]]),
|
||||
working: true,
|
||||
});
|
||||
|
||||
expect(countElements(html, ".tv-card")).toBe(1);
|
||||
expect(countElements(html, '[aria-label="running"]')).toBe(1);
|
||||
expect(countOccurrences(html, TOOL_NAME)).toBe(1);
|
||||
expect(html).not.toContain("thinking…");
|
||||
});
|
||||
|
||||
it("keeps the working shimmer when no tool is active", () => {
|
||||
const html = renderTranscript({ working: true, activeTools: new Map() });
|
||||
|
||||
expect(html).toContain("thinking…");
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user