Archived
fix: avoid duplicate skill loading cards
This commit is contained in:
@@ -0,0 +1,5 @@
|
|||||||
|
---
|
||||||
|
"@jmfederico/pi-web": patch
|
||||||
|
---
|
||||||
|
|
||||||
|
Prevent live skill-loading cards from duplicating when the finalized transcript groups multiple skill reads.
|
||||||
@@ -162,9 +162,9 @@ function normalizeContent(content: unknown, message: unknown): ChatPart[] {
|
|||||||
if (type === "toolCall") {
|
if (type === "toolCall") {
|
||||||
const toolName = getString(part, "name") ?? "tool";
|
const toolName = getString(part, "name") ?? "tool";
|
||||||
const args = getProperty(part, "arguments");
|
const args = getProperty(part, "arguments");
|
||||||
const skillRead = toolName === "read" ? parseSkillReadPath(getString(args, "path")) : undefined;
|
|
||||||
if (skillRead !== undefined) return [{ type: "skillRead", ...skillRead }];
|
|
||||||
const toolCallId = getString(part, "id");
|
const toolCallId = getString(part, "id");
|
||||||
|
const skillRead = toolName === "read" ? parseSkillReadPath(getString(args, "path")) : undefined;
|
||||||
|
if (skillRead !== undefined) return [{ type: "skillRead", ...skillRead, ...(toolCallId === undefined ? {} : { toolCallId }) }];
|
||||||
return [{ type: "toolCall", ...(toolCallId === undefined ? {} : { toolCallId }), toolName, summary: summarizeArgs(args), ...(args === undefined ? {} : { args }) }];
|
return [{ type: "toolCall", ...(toolCallId === undefined ? {} : { toolCallId }), toolName, summary: summarizeArgs(args), ...(args === undefined ? {} : { args }) }];
|
||||||
}
|
}
|
||||||
if (type === "image") {
|
if (type === "image") {
|
||||||
|
|||||||
@@ -101,8 +101,9 @@ describe("applyTranscriptEvent", () => {
|
|||||||
]);
|
]);
|
||||||
});
|
});
|
||||||
|
|
||||||
it("replaces streamed skill reads when the finalized assistant message includes thinking", () => {
|
it("replaces streamed thinking and skill reads when the finalized assistant message includes thinking", () => {
|
||||||
const streamed: ChatLine[] = [
|
const streamed: ChatLine[] = [
|
||||||
|
{ role: "assistant", parts: [{ type: "thinking", text: "load skill" }] },
|
||||||
{ role: "skill", parts: [{ type: "skillRead", name: "playwright", path: "/skills/playwright/SKILL.md" }] },
|
{ role: "skill", parts: [{ type: "skillRead", name: "playwright", path: "/skills/playwright/SKILL.md" }] },
|
||||||
{ role: "tool", parts: [{ type: "toolResult", toolName: "read", text: "skill content", isError: false }] },
|
{ role: "tool", parts: [{ type: "toolResult", toolName: "read", text: "skill content", isError: false }] },
|
||||||
];
|
];
|
||||||
@@ -174,8 +175,8 @@ describe("applyTranscriptEvent", () => {
|
|||||||
messages = applyTranscriptEvent(messages, { type: "tool.start", toolName: "read", toolCallId: "2", summary: "", args: { path: "/skills/sentry-cli/SKILL.md" } }) ?? messages;
|
messages = applyTranscriptEvent(messages, { type: "tool.start", toolName: "read", toolCallId: "2", summary: "", args: { path: "/skills/sentry-cli/SKILL.md" } }) ?? messages;
|
||||||
|
|
||||||
expect(messages).toEqual([
|
expect(messages).toEqual([
|
||||||
{ role: "skill", parts: [{ type: "skillRead", name: "playwright", path: "/skills/playwright/SKILL.md" }] },
|
{ role: "skill", parts: [{ type: "skillRead", name: "playwright", path: "/skills/playwright/SKILL.md", toolCallId: "1" }] },
|
||||||
{ role: "skill", parts: [{ type: "skillRead", name: "sentry-cli", path: "/skills/sentry-cli/SKILL.md" }] },
|
{ role: "skill", parts: [{ type: "skillRead", name: "sentry-cli", path: "/skills/sentry-cli/SKILL.md", toolCallId: "2" }] },
|
||||||
]);
|
]);
|
||||||
});
|
});
|
||||||
|
|
||||||
@@ -185,6 +186,74 @@ describe("applyTranscriptEvent", () => {
|
|||||||
messages = applyTranscriptEvent(messages, { type: "tool.start", toolName: "read", toolCallId: "1", summary: "", args: { path: "/skills/playwright/SKILL.md" } }) ?? messages;
|
messages = applyTranscriptEvent(messages, { type: "tool.start", toolName: "read", toolCallId: "1", summary: "", args: { path: "/skills/playwright/SKILL.md" } }) ?? messages;
|
||||||
|
|
||||||
expect(messages).toEqual([
|
expect(messages).toEqual([
|
||||||
|
{ role: "skill", parts: [{ type: "skillRead", name: "playwright", path: "/skills/playwright/SKILL.md", toolCallId: "1" }] },
|
||||||
|
]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("replaces multiple streamed skill reads with the finalized grouped skill message", () => {
|
||||||
|
const firstTool: ChatLine = { role: "tool", parts: [{ type: "toolExecution", toolCallId: "read-1", toolName: "read", summary: "/skills/code-quality-architecture/SKILL.md", status: "success", resultText: "content" }] };
|
||||||
|
const secondTool: ChatLine = { role: "tool", parts: [{ type: "toolExecution", toolCallId: "read-2", toolName: "read", summary: "/skills/relay/SKILL.md", status: "success", resultText: "content" }] };
|
||||||
|
const thirdTool: ChatLine = { role: "tool", parts: [{ type: "toolExecution", toolCallId: "read-3", toolName: "read", summary: "/skills/skill-creator/SKILL.md", status: "success", resultText: "content" }] };
|
||||||
|
const streamed: ChatLine[] = [
|
||||||
|
{ role: "skill", parts: [{ type: "skillRead", name: "code-quality-architecture", path: "/skills/code-quality-architecture/SKILL.md", toolCallId: "read-1" }] },
|
||||||
|
firstTool,
|
||||||
|
{ role: "skill", parts: [{ type: "skillRead", name: "relay", path: "/skills/relay/SKILL.md", toolCallId: "read-2" }] },
|
||||||
|
secondTool,
|
||||||
|
{ role: "skill", parts: [{ type: "skillRead", name: "skill-creator", path: "/skills/skill-creator/SKILL.md", toolCallId: "read-3" }] },
|
||||||
|
thirdTool,
|
||||||
|
];
|
||||||
|
|
||||||
|
expect(applyTranscriptEvent(streamed, {
|
||||||
|
type: "message.end",
|
||||||
|
message: {
|
||||||
|
role: "assistant",
|
||||||
|
content: [
|
||||||
|
{ type: "toolCall", id: "read-1", name: "read", arguments: { path: "/skills/code-quality-architecture/SKILL.md" } },
|
||||||
|
{ type: "toolCall", id: "read-2", name: "read", arguments: { path: "/skills/relay/SKILL.md" } },
|
||||||
|
{ type: "toolCall", id: "read-3", name: "read", arguments: { path: "/skills/skill-creator/SKILL.md" } },
|
||||||
|
],
|
||||||
|
timestamp: "2026-05-09T12:00:00.000Z",
|
||||||
|
},
|
||||||
|
})).toEqual([
|
||||||
|
{
|
||||||
|
role: "skill",
|
||||||
|
parts: [
|
||||||
|
{ type: "skillRead", name: "code-quality-architecture", path: "/skills/code-quality-architecture/SKILL.md", toolCallId: "read-1" },
|
||||||
|
{ type: "skillRead", name: "relay", path: "/skills/relay/SKILL.md", toolCallId: "read-2" },
|
||||||
|
{ type: "skillRead", name: "skill-creator", path: "/skills/skill-creator/SKILL.md", toolCallId: "read-3" },
|
||||||
|
],
|
||||||
|
meta: { timestamp: "2026-05-09T12:00:00.000Z" },
|
||||||
|
},
|
||||||
|
firstTool,
|
||||||
|
secondTool,
|
||||||
|
thirdTool,
|
||||||
|
]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("ignores streamed skill read starts that are already in a finalized grouped skill message", () => {
|
||||||
|
const messages: ChatLine[] = [
|
||||||
|
{
|
||||||
|
role: "skill",
|
||||||
|
parts: [
|
||||||
|
{ type: "skillRead", name: "code-quality-architecture", path: "/skills/code-quality-architecture/SKILL.md", toolCallId: "read-1" },
|
||||||
|
{ type: "skillRead", name: "relay", path: "/skills/relay/SKILL.md", toolCallId: "read-2" },
|
||||||
|
],
|
||||||
|
meta: { timestamp: "2026-05-09T12:00:00.000Z" },
|
||||||
|
},
|
||||||
|
{ role: "tool", parts: [{ type: "toolExecution", toolCallId: "read-1", toolName: "read", summary: "/skills/code-quality-architecture/SKILL.md", status: "success", resultText: "content" }] },
|
||||||
|
];
|
||||||
|
|
||||||
|
expect(applyTranscriptEvent(messages, { type: "tool.start", toolName: "read", toolCallId: "read-2", summary: "", args: { path: "/skills/relay/SKILL.md" } })).toEqual(messages);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("allows the same skill read after a user boundary", () => {
|
||||||
|
const messages: ChatLine[] = [
|
||||||
|
{ role: "skill", parts: [{ type: "skillRead", name: "playwright", path: "/skills/playwright/SKILL.md" }] },
|
||||||
|
textMessage("user", "load it again"),
|
||||||
|
];
|
||||||
|
|
||||||
|
expect(applyTranscriptEvent(messages, { type: "tool.start", toolName: "read", toolCallId: "", summary: "", args: { path: "/skills/playwright/SKILL.md" } })).toEqual([
|
||||||
|
...messages,
|
||||||
{ role: "skill", parts: [{ type: "skillRead", name: "playwright", path: "/skills/playwright/SKILL.md" }] },
|
{ role: "skill", parts: [{ type: "skillRead", name: "playwright", path: "/skills/playwright/SKILL.md" }] },
|
||||||
]);
|
]);
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -35,8 +35,8 @@ function applyFinalMessage(messages: ChatLine[], rawMessage: unknown): ChatLine[
|
|||||||
}
|
}
|
||||||
|
|
||||||
function applyFinalLine(messages: ChatLine[], displayEnded: ChatLine): ChatLine[] {
|
function applyFinalLine(messages: ChatLine[], displayEnded: ChatLine): ChatLine[] {
|
||||||
const skillReadIndex = findMatchingSkillRead(messages, displayEnded);
|
const skillReadIndexes = findMatchingSkillReadIndexes(messages, displayEnded);
|
||||||
if (skillReadIndex >= 0) return [...messages.slice(0, skillReadIndex), displayEnded, ...messages.slice(skillReadIndex + 1)];
|
if (skillReadIndexes.length > 0) return replaceSkillReadLines(messages, skillReadIndexes, displayEnded);
|
||||||
const last = messages.at(-1);
|
const last = messages.at(-1);
|
||||||
if (last?.role !== displayEnded.role) return [...messages, displayEnded];
|
if (last?.role !== displayEnded.role) return [...messages, displayEnded];
|
||||||
if (displayEnded.role === "assistant" || sameMessageText(last, displayEnded)) return [...messages.slice(0, -1), displayEnded];
|
if (displayEnded.role === "assistant" || sameMessageText(last, displayEnded)) return [...messages.slice(0, -1), displayEnded];
|
||||||
@@ -58,7 +58,9 @@ function parseSkillReadPath(path: string | undefined): { name: string; path: str
|
|||||||
|
|
||||||
function appendToolExecutionStart(messages: ChatLine[], event: Extract<SessionUiEvent, { type: "tool.start" }>): ChatLine[] {
|
function appendToolExecutionStart(messages: ChatLine[], event: Extract<SessionUiEvent, { type: "tool.start" }>): ChatLine[] {
|
||||||
const skillRead = event.toolName === "read" ? parseSkillReadPath(getString(event.args, "path")) : undefined;
|
const skillRead = event.toolName === "read" ? parseSkillReadPath(getString(event.args, "path")) : undefined;
|
||||||
if (skillRead !== undefined) return appendLine(messages, { role: "skill", parts: [{ type: "skillRead", ...skillRead }] });
|
if (skillRead !== undefined) {
|
||||||
|
return appendLine(messages, { role: "skill", parts: [{ type: "skillRead", ...skillRead, ...(event.toolCallId === "" ? {} : { toolCallId: event.toolCallId }) }] });
|
||||||
|
}
|
||||||
|
|
||||||
const part: ToolExecutionPart = {
|
const part: ToolExecutionPart = {
|
||||||
type: "toolExecution",
|
type: "toolExecution",
|
||||||
@@ -151,16 +153,51 @@ function stringifyToolContent(content: unknown): string {
|
|||||||
return "";
|
return "";
|
||||||
}
|
}
|
||||||
|
|
||||||
function findMatchingSkillRead(messages: ChatLine[], ended: ChatLine): number {
|
function findMatchingSkillReadIndexes(messages: ChatLine[], ended: ChatLine): number[] {
|
||||||
const endedReads = skillReads(ended);
|
const endedReads = skillReads(ended);
|
||||||
if (endedReads.length === 0) return -1;
|
if (endedReads.length === 0) return [];
|
||||||
for (let index = messages.length - 1; index >= 0; index--) {
|
|
||||||
const message = messages[index];
|
const matchedIndexes: number[] = [];
|
||||||
if (message?.role !== "skill") continue;
|
let readEnd = endedReads.length;
|
||||||
const reads = skillReads(message);
|
const lowerBound = lastUserBoundaryIndex(messages) + 1;
|
||||||
if (sameSkillReads(reads, endedReads)) return index;
|
|
||||||
|
for (let index = messages.length - 1; index >= lowerBound; index--) {
|
||||||
|
const reads = skillReads(messages[index]);
|
||||||
|
if (reads.length === 0) continue;
|
||||||
|
const readStart = readEnd - reads.length;
|
||||||
|
if (readStart < 0) continue;
|
||||||
|
if (!sameSkillReads(reads, endedReads.slice(readStart, readEnd))) continue;
|
||||||
|
matchedIndexes.unshift(index);
|
||||||
|
readEnd = readStart;
|
||||||
|
if (readEnd === 0) return matchedIndexes;
|
||||||
}
|
}
|
||||||
return -1;
|
|
||||||
|
return [];
|
||||||
|
}
|
||||||
|
|
||||||
|
function replaceSkillReadLines(messages: ChatLine[], indexes: number[], replacement: ChatLine): ChatLine[] {
|
||||||
|
const replacementIndexes = indexesWithAdjacentAssistantFragment(messages, indexes, replacement);
|
||||||
|
const insertIndex = replacementIndexes[0];
|
||||||
|
if (insertIndex === undefined) return messages;
|
||||||
|
const replaced = new Set(replacementIndexes);
|
||||||
|
const next: ChatLine[] = [];
|
||||||
|
for (let index = 0; index < messages.length; index++) {
|
||||||
|
if (index === insertIndex) next.push(replacement);
|
||||||
|
const message = messages[index];
|
||||||
|
if (message !== undefined && !replaced.has(index)) next.push(message);
|
||||||
|
}
|
||||||
|
return next;
|
||||||
|
}
|
||||||
|
|
||||||
|
function indexesWithAdjacentAssistantFragment(messages: ChatLine[], indexes: number[], replacement: ChatLine): number[] {
|
||||||
|
const firstIndex = indexes[0];
|
||||||
|
if (replacement.role !== "assistant" || firstIndex === undefined) return indexes;
|
||||||
|
const previousIndex = firstIndex - 1;
|
||||||
|
return isStreamedAssistantFragment(messages[previousIndex]) ? [previousIndex, ...indexes] : indexes;
|
||||||
|
}
|
||||||
|
|
||||||
|
function isStreamedAssistantFragment(message: ChatLine | undefined): boolean {
|
||||||
|
return message?.role === "assistant" && message.parts.length > 0 && message.parts.every((part) => part.type === "text" || part.type === "thinking");
|
||||||
}
|
}
|
||||||
|
|
||||||
function skillReads(message: ChatLine | undefined): SkillRead[] {
|
function skillReads(message: ChatLine | undefined): SkillRead[] {
|
||||||
@@ -176,6 +213,7 @@ function sameSkillReads(left: SkillRead[], right: SkillRead[]): boolean {
|
|||||||
|
|
||||||
function sameSkillRead(left: SkillRead, right: SkillRead | undefined): boolean {
|
function sameSkillRead(left: SkillRead, right: SkillRead | undefined): boolean {
|
||||||
if (right === undefined) return false;
|
if (right === undefined) return false;
|
||||||
|
if (left.toolCallId !== undefined && right.toolCallId !== undefined) return left.toolCallId === right.toolCallId;
|
||||||
return normalizeSkillPath(left.path) === normalizeSkillPath(right.path) || left.name === right.name;
|
return normalizeSkillPath(left.path) === normalizeSkillPath(right.path) || left.name === right.name;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -201,11 +239,32 @@ function appendNewMessage(messages: ChatLine[], rawMessage: unknown): ChatLine[]
|
|||||||
|
|
||||||
function appendLine(messages: ChatLine[], line: ChatLine): ChatLine[] {
|
function appendLine(messages: ChatLine[], line: ChatLine): ChatLine[] {
|
||||||
const last = messages.at(-1);
|
const last = messages.at(-1);
|
||||||
if (line.role === "skill" && sameSkillReads(skillReads(last), skillReads(line))) return messages;
|
if (isDuplicateSkillLine(messages, line)) return messages;
|
||||||
if (last?.role === line.role && line.role !== "skill") return [...messages.slice(0, -1), { ...last, parts: [...last.parts, ...line.parts] }];
|
if (last?.role === line.role && line.role !== "skill") return [...messages.slice(0, -1), { ...last, parts: [...last.parts, ...line.parts] }];
|
||||||
return [...messages, line];
|
return [...messages, line];
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function isDuplicateSkillLine(messages: ChatLine[], line: ChatLine): boolean {
|
||||||
|
const reads = skillReads(line);
|
||||||
|
if (line.role !== "skill" || reads.length === 0) return false;
|
||||||
|
const lowerBound = lastUserBoundaryIndex(messages) + 1;
|
||||||
|
return reads.every((read) => hasMatchingSkillRead(messages, read, lowerBound));
|
||||||
|
}
|
||||||
|
|
||||||
|
function hasMatchingSkillRead(messages: ChatLine[], read: SkillRead, lowerBound: number): boolean {
|
||||||
|
for (let index = messages.length - 1; index >= lowerBound; index--) {
|
||||||
|
if (skillReads(messages[index]).some((candidate) => sameSkillRead(candidate, read))) return true;
|
||||||
|
}
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
function lastUserBoundaryIndex(messages: ChatLine[]): number {
|
||||||
|
for (let index = messages.length - 1; index >= 0; index--) {
|
||||||
|
if (messages[index]?.role === "user") return index;
|
||||||
|
}
|
||||||
|
return -1;
|
||||||
|
}
|
||||||
|
|
||||||
function isRecord(value: unknown): value is Record<string, unknown> {
|
function isRecord(value: unknown): value is Record<string, unknown> {
|
||||||
return typeof value === "object" && value !== null;
|
return typeof value === "object" && value !== null;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -24,7 +24,7 @@ export type ChatPart =
|
|||||||
| { type: "image"; mimeType: string; data: string }
|
| { type: "image"; mimeType: string; data: string }
|
||||||
| { type: "thinking"; text: string }
|
| { type: "thinking"; text: string }
|
||||||
| { type: "skillInvocation"; name: string; location: string; content: string }
|
| { type: "skillInvocation"; name: string; location: string; content: string }
|
||||||
| { type: "skillRead"; name: string; path: string }
|
| { type: "skillRead"; name: string; path: string; toolCallId?: string }
|
||||||
| { type: "toolCall"; toolCallId?: string; toolName: string; summary: string; args?: unknown }
|
| { type: "toolCall"; toolCallId?: string; toolName: string; summary: string; args?: unknown }
|
||||||
| ToolExecutionPart
|
| ToolExecutionPart
|
||||||
| { type: "toolResult"; toolCallId?: string; toolName: string; text: string; isError: boolean; content?: unknown; details?: unknown }
|
| { type: "toolResult"; toolCallId?: string; toolName: string; text: string; isError: boolean; content?: unknown; details?: unknown }
|
||||||
|
|||||||
Reference in New Issue
Block a user