Skip to content

Commit 68f1ef6

Browse files
tatoalotatoalo
authored
fix(sessions): dedupe cloud prompt echo when attachments add a summary (#3100)
1 parent bb651ea commit 68f1ef6

5 files changed

Lines changed: 126 additions & 27 deletions

File tree

‎packages/core/src/editor/cloud-prompt.test.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import {
44
buildCloudTaskDescription,
55
serializeCloudPrompt,
66
stripAbsoluteFileTags,
7+
stripTrailingAttachmentSummary,
78
} from "@posthog/core/editor/cloud-prompt";
89

910
import { beforeEach, describe, expect, it, vi } from "vitest";
@@ -63,6 +64,23 @@ describe("cloud-prompt", () => {
6364
);
6465
});
6566

67+
it.each([
68+
[
69+
"text + trailing summary",
70+
"do this\n\nAttached files: a.png, b.txt",
71+
"do this",
72+
],
73+
["summary only", "Attached files: a.png", ""],
74+
["no summary", "do this", "do this"],
75+
[
76+
"summary not at end",
77+
"Attached files: a.png\n\nthen do this",
78+
"Attached files: a.png\n\nthen do this",
79+
],
80+
])("stripTrailingAttachmentSummary: %s", (_label, input, expected) => {
81+
expect(stripTrailingAttachmentSummary(input)).toBe(expected);
82+
});
83+
6684
it("uses resource_link path references for text attachments", async () => {
6785
const blocks = await buildCloudPromptBlocks(
6886
'read this <file path="/tmp/test.txt" />',

‎packages/core/src/editor/cloud-prompt.ts‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,15 @@ export function getAbsoluteAttachmentPaths(
147147
);
148148
}
149149

150+
export const ATTACHMENT_SUMMARY_PREFIX = "Attached files: ";
151+
const TRAILING_ATTACHMENT_SUMMARY_REGEX = new RegExp(
152+
`(?:^|\\n)${ATTACHMENT_SUMMARY_PREFIX}[^\\n]*$`,
153+
);
154+
155+
export function stripTrailingAttachmentSummary(text: string): string {
156+
return text.replace(TRAILING_ATTACHMENT_SUMMARY_REGEX, "").trim();
157+
}
158+
150159
export function buildCloudTaskDescription(
151160
prompt: string,
152161
filePaths: string[] = [],
@@ -160,7 +169,7 @@ export function buildCloudTaskDescription(
160169
return strippedPrompt;
161170
}
162171

163-
const attachmentSummary = `Attached files: ${attachmentNames.join(", ")}`;
172+
const attachmentSummary = `${ATTACHMENT_SUMMARY_PREFIX}${attachmentNames.join(", ")}`;
164173
return strippedPrompt
165174
? `${strippedPrompt}\n\n${attachmentSummary}`
166175
: attachmentSummary;

‎packages/core/src/sessions/cloudPrompt.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import type { ContentBlock } from "@agentclientprotocol/sdk";
22
import {
3+
ATTACHMENT_SUMMARY_PREFIX,
34
buildCloudTaskDescription,
45
getAbsoluteAttachmentPaths,
56
stripAbsoluteFileTags,
@@ -87,7 +88,7 @@ function summarizePrompt(text: string, filePaths: string[]): string {
8788
return text.trim();
8889
}
8990

90-
const attachmentSummary = `Attached files: ${filePaths.map(getFileName).join(", ")}`;
91+
const attachmentSummary = `${ATTACHMENT_SUMMARY_PREFIX}${filePaths.map(getFileName).join(", ")}`;
9192
return text.trim()
9293
? `${text.trim()}\n\n${attachmentSummary}`
9394
: attachmentSummary;

‎packages/ui/src/features/sessions/components/mergeConversationItems.test.ts‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,57 @@ describe("mergeConversationItems", () => {
7272
expect(pinned.content).toBe(echoedWithContext);
7373
});
7474

75+
it("cloud: dedupes the echo when the placeholder carries an 'Attached files:' summary the echo lacks", () => {
76+
const echoed = {
77+
...userMessage("echo", "hello\n\nDo this"),
78+
attachments: [
79+
{ id: "file:///tmp/clipboard.png", label: "clipboard.png" },
80+
],
81+
};
82+
const result = mergeConversationItems({
83+
conversationItems: [echoed, userMessage("other", "different")],
84+
optimisticItems: [
85+
userMessage("opt", "hello\n\nDo this\n\nAttached files: clipboard.png"),
86+
],
87+
isCloud: true,
88+
});
89+
expect(result.map((i) => i.id)).toEqual(["opt", "other"]);
90+
const pinned = result.find((i) => i.id === "opt");
91+
if (pinned?.type !== "user_message")
92+
throw new Error("expected user_message");
93+
expect(pinned.content).toBe("hello\n\nDo this");
94+
expect(pinned.attachments).toEqual(echoed.attachments);
95+
});
96+
97+
it("cloud: dedupes an attachment-only placeholder against its text-less echo", () => {
98+
const echoed = {
99+
...userMessage("echo", ""),
100+
attachments: [{ id: "file:///tmp/notes.txt", label: "notes.txt" }],
101+
};
102+
const result = mergeConversationItems({
103+
conversationItems: [echoed],
104+
optimisticItems: [userMessage("opt", "Attached files: notes.txt")],
105+
isCloud: true,
106+
});
107+
expect(result.map((i) => i.id)).toEqual(["opt"]);
108+
const pinned = result.find((i) => i.id === "opt");
109+
if (pinned?.type !== "user_message")
110+
throw new Error("expected user_message");
111+
expect(pinned.attachments).toEqual(echoed.attachments);
112+
});
113+
114+
it("cloud: a pinned placeholder consumes only one echo, later identical messages render", () => {
115+
const result = mergeConversationItems({
116+
conversationItems: [
117+
userMessage("echo", "repeat"),
118+
userMessage("again", "repeat"),
119+
],
120+
optimisticItems: [userMessage("opt", "repeat")],
121+
isCloud: true,
122+
});
123+
expect(result.map((i) => i.id)).toEqual(["opt", "again"]);
124+
});
125+
75126
it("cloud: dedupe is no-op when there are no optimistic items", () => {
76127
const conversationItems = [
77128
userMessage("a", "first"),

‎packages/ui/src/features/sessions/components/mergeConversationItems.ts‎

Lines changed: 45 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { stripTrailingAttachmentSummary } from "@posthog/core/editor/cloud-prompt";
12
import type { ConversationItem } from "./buildConversationItems";
23
import { extractChannelContext } from "./session-update/channelContext";
34
import { extractCustomInstructions } from "./session-update/customInstructions";
@@ -8,15 +9,22 @@ interface MergeConversationItemsArgs {
89
isCloud: boolean;
910
}
1011

12+
type UserMessageItem = Extract<ConversationItem, { type: "user_message" }>;
13+
1114
// The pinned optimistic bubble is seeded from the bare task description, but the
1215
// echoed `session/prompt` that streams back from the sandbox may additionally
1316
// carry the channel's CONTEXT.md and/or the user's personalization, folded into
1417
// the prompt at task creation (see buildChannelContextText /
15-
// buildCustomInstructionsText in @posthog/core). Dedupe and upgrade compare on the
16-
// text with both blocks stripped so the echo still matches its placeholder.
18+
// buildCustomInstructionsText in @posthog/core). The description side instead
19+
// appends an `Attached files: <names>` summary line that the echo carries as
20+
// resource_link blocks, not text (see buildCloudTaskDescription). Dedupe and
21+
// upgrade compare on the text with all three stripped so the echo still matches
22+
// its placeholder.
1723
function strippedUserContent(content: string): string {
1824
const withoutChannel = extractChannelContext(content)?.stripped ?? content;
19-
return extractCustomInstructions(withoutChannel)?.stripped ?? withoutChannel;
25+
const withoutInstructions =
26+
extractCustomInstructions(withoutChannel)?.stripped ?? withoutChannel;
27+
return stripTrailingAttachmentSummary(withoutInstructions);
2028
}
2129

2230
// Cloud's initial optimistic is pinned to the top so the user's prompt stays
@@ -44,43 +52,55 @@ export function mergeConversationItems({
4452
const tailOptimisticItems = optimisticItems.filter(
4553
(item) => item.type === "user_message" && item.pinToTop === false,
4654
);
47-
const pinnedOptimisticUserContents = new Set(
48-
pinnedOptimisticItems
49-
.filter(
50-
(item): item is Extract<typeof item, { type: "user_message" }> =>
51-
item.type === "user_message",
52-
)
53-
.map((item) => strippedUserContent(item.content)),
54-
);
55+
const unconsumedPinnedKeyCounts = new Map<string, number>();
56+
for (const item of pinnedOptimisticItems) {
57+
if (item.type !== "user_message") continue;
58+
const key = strippedUserContent(item.content);
59+
unconsumedPinnedKeyCounts.set(
60+
key,
61+
(unconsumedPinnedKeyCounts.get(key) ?? 0) + 1,
62+
);
63+
}
5564

5665
// When the echoed prompt matches a pinned optimistic placeholder, drop the
57-
// echo but remember its content: it may carry the channel CONTEXT.md block the
58-
// placeholder lacks, so we surface the richer copy on the pinned bubble below.
59-
const echoedContentByKey = new Map<string, string>();
66+
// echo but remember it: it may carry the channel CONTEXT.md block and the
67+
// attachment chips the placeholder lacks, so we surface the richer copy on
68+
// the pinned bubble below.
69+
const echoedItemByKey = new Map<string, UserMessageItem>();
6070
const dedupedConversation =
61-
pinnedOptimisticUserContents.size === 0
71+
unconsumedPinnedKeyCounts.size === 0
6272
? conversationItems
6373
: conversationItems.filter((item) => {
6474
if (item.type !== "user_message") return true;
6575
const key = strippedUserContent(item.content);
66-
if (!pinnedOptimisticUserContents.has(key)) return true;
67-
if (!echoedContentByKey.has(key)) {
68-
echoedContentByKey.set(key, item.content);
76+
const remaining = unconsumedPinnedKeyCounts.get(key) ?? 0;
77+
if (remaining === 0) return true;
78+
unconsumedPinnedKeyCounts.set(key, remaining - 1);
79+
if (!echoedItemByKey.has(key)) {
80+
echoedItemByKey.set(key, item);
6981
}
7082
return false;
7183
});
7284

7385
const resolvedPinnedItems =
74-
echoedContentByKey.size === 0
86+
echoedItemByKey.size === 0
7587
? pinnedOptimisticItems
7688
: pinnedOptimisticItems.map((item) => {
7789
if (item.type !== "user_message") return item;
78-
const echoed = echoedContentByKey.get(
79-
strippedUserContent(item.content),
80-
);
81-
return echoed !== undefined && echoed !== item.content
82-
? { ...item, content: echoed }
83-
: item;
90+
const echoed = echoedItemByKey.get(strippedUserContent(item.content));
91+
if (
92+
!echoed ||
93+
(echoed.content === item.content && !echoed.attachments?.length)
94+
) {
95+
return item;
96+
}
97+
return {
98+
...item,
99+
content: echoed.content,
100+
...(echoed.attachments?.length
101+
? { attachments: echoed.attachments }
102+
: {}),
103+
};
84104
});
85105

86106
return [

0 commit comments

Comments
 (0)