Skip to content

Commit f333240

Browse files
authored
fix(sessions): stop chat-title regeneration on every view remount (#3315)
1 parent 30bacd1 commit f333240

5 files changed

Lines changed: 269 additions & 25 deletions

File tree

packages/core/src/sessions/chatTitle.test.ts

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,83 @@ describe("decideTitleGeneration", () => {
103103
});
104104
expect(decision.shouldGenerateFromTaskDescription).toBe(true);
105105
});
106+
107+
it.each([
108+
{
109+
name: "skips a catch-up fire when the title is locked and a summary exists",
110+
promptCount: 1 + REGENERATE_INTERVAL,
111+
lastGeneratedAtCount: 0,
112+
initialDescriptionHandled: false,
113+
titleLocked: true,
114+
hasSummary: true,
115+
expected: false,
116+
},
117+
{
118+
name: "runs a catch-up fire when no summary exists yet",
119+
promptCount: 1 + REGENERATE_INTERVAL,
120+
lastGeneratedAtCount: 0,
121+
initialDescriptionHandled: false,
122+
titleLocked: true,
123+
hasSummary: false,
124+
expected: true,
125+
},
126+
{
127+
name: "runs a catch-up fire when the title is not locked",
128+
promptCount: 1 + REGENERATE_INTERVAL,
129+
lastGeneratedAtCount: 0,
130+
initialDescriptionHandled: false,
131+
titleLocked: false,
132+
hasSummary: true,
133+
expected: true,
134+
},
135+
{
136+
name: "still refreshes the summary at the interval while locked",
137+
promptCount: 1 + REGENERATE_INTERVAL,
138+
lastGeneratedAtCount: 1,
139+
initialDescriptionHandled: false,
140+
titleLocked: true,
141+
hasSummary: true,
142+
expected: true,
143+
},
144+
{
145+
name: "still generates on the first prompt while locked",
146+
promptCount: 1,
147+
lastGeneratedAtCount: 0,
148+
initialDescriptionHandled: false,
149+
titleLocked: true,
150+
hasSummary: false,
151+
expected: true,
152+
},
153+
{
154+
name: "treats a description-handled interval fire as organic while locked",
155+
promptCount: REGENERATE_INTERVAL,
156+
lastGeneratedAtCount: 0,
157+
initialDescriptionHandled: true,
158+
titleLocked: true,
159+
hasSummary: true,
160+
expected: true,
161+
},
162+
])(
163+
"$name",
164+
({
165+
promptCount,
166+
lastGeneratedAtCount,
167+
initialDescriptionHandled,
168+
titleLocked,
169+
hasSummary,
170+
expected,
171+
}) => {
172+
const decision = decideTitleGeneration({
173+
promptCount,
174+
lastGeneratedAtCount,
175+
initialDescriptionHandled,
176+
task: { title: "Custom", description: "d" },
177+
isTitleLocked: () => titleLocked,
178+
hasSummary,
179+
});
180+
expect(decision.shouldGenerateFromPrompts).toBe(expected);
181+
},
182+
);
106183
});
107184

108185
describe("selectPromptsForTitle", () => {

packages/core/src/sessions/chatTitle.ts

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,16 +37,36 @@ export function decideTitleGeneration(input: {
3737
lastGeneratedAtCount: number;
3838
initialDescriptionHandled: boolean;
3939
task: Pick<Task, "title" | "description">;
40+
isTitleLocked?: () => boolean;
41+
hasSummary?: boolean;
4042
}): TitleGenerationDecision {
41-
const { promptCount, lastGeneratedAtCount, initialDescriptionHandled, task } =
42-
input;
43+
const {
44+
promptCount,
45+
lastGeneratedAtCount,
46+
initialDescriptionHandled,
47+
task,
48+
isTitleLocked,
49+
hasSummary = false,
50+
} = input;
51+
52+
// A first fire on an already-long conversation whose title the user renamed
53+
// and whose summary is already stored would produce nothing usable. Organic
54+
// triggers (first prompt, every REGENERATE_INTERVAL prompts) still run while
55+
// renamed so the summary stays current.
56+
const skipStaleFire =
57+
promptCount > 1 &&
58+
lastGeneratedAtCount === 0 &&
59+
!initialDescriptionHandled &&
60+
hasSummary &&
61+
(isTitleLocked?.() ?? false);
4362

4463
const shouldGenerateFromPrompts =
45-
(promptCount === 1 &&
64+
!skipStaleFire &&
65+
((promptCount === 1 &&
4666
lastGeneratedAtCount === 0 &&
4767
!initialDescriptionHandled) ||
48-
(promptCount > 1 &&
49-
promptCount - lastGeneratedAtCount >= REGENERATE_INTERVAL);
68+
(promptCount > 1 &&
69+
promptCount - lastGeneratedAtCount >= REGENERATE_INTERVAL));
5070

5171
const shouldGenerateFromTaskDescription =
5272
promptCount === 0 &&

packages/ui/src/features/sessions/hooks/useChatTitleGenerator.test.ts

Lines changed: 104 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@ const mockSetQueriesData = vi.hoisted(() => vi.fn());
1313
const mockSetQueryData = vi.hoisted(() => vi.fn());
1414
const mockUpdateSessionTaskTitle = vi.hoisted(() => vi.fn());
1515
const mockPrompts = vi.hoisted(() => ({ value: [] as string[] }));
16+
const mockSessionSummary = vi.hoisted(() => ({
17+
value: undefined as string | undefined,
18+
}));
1619
const mockSessionStoreSetters = vi.hoisted(() => ({ updateSession: vi.fn() }));
1720
const mockTitleAttachmentPaths = vi.hoisted(() => ({ value: [] as string[] }));
1821
const mockTitleAttachmentClear = vi.hoisted(() => vi.fn());
@@ -77,7 +80,14 @@ vi.mock("@posthog/ui/shell/titleAttachmentStore", () => ({
7780
vi.mock("@posthog/ui/features/sessions/sessionStore", () => {
7881
const state = {
7982
taskIdIndex: { "task-1": "run-1" },
80-
sessions: { "run-1": { events: mockPrompts.value } },
83+
get sessions() {
84+
return {
85+
"run-1": {
86+
events: mockPrompts.value,
87+
conversationSummary: mockSessionSummary.value,
88+
},
89+
};
90+
},
8191
};
8292
const fn = Object.assign(
8393
(selector: (s: typeof state) => unknown) => selector(state),
@@ -89,6 +99,7 @@ vi.mock("@posthog/ui/features/sessions/sessionStore", () => {
8999
};
90100
});
91101

102+
import { useTitleGenerationStore } from "@posthog/ui/features/sessions/titleGenerationStore";
92103
import { useChatTitleGenerator } from "./useChatTitleGenerator";
93104

94105
const TASK_ID = "task-1";
@@ -113,11 +124,18 @@ function cacheTask(task: Task): void {
113124
mockGetQueriesData.mockReturnValue([[["tasks", "list"], [task]]]);
114125
}
115126

127+
interface TitleAndSummaryResult {
128+
title: string;
129+
summary: string;
130+
}
131+
116132
describe("useChatTitleGenerator", () => {
117133
beforeEach(() => {
118134
vi.clearAllMocks();
135+
useTitleGenerationStore.setState({ byTaskId: {} });
119136
mockIsAuthenticated.value = true;
120137
mockPrompts.value = [];
138+
mockSessionSummary.value = undefined;
121139
mockTitleAttachmentPaths.value = [];
122140
mockEnrichDescription.mockImplementation((desc: string) =>
123141
Promise.resolve(desc),
@@ -382,4 +400,89 @@ describe("useChatTitleGenerator", () => {
382400

383401
expect(mockGenerateTitle).not.toHaveBeenCalled();
384402
});
403+
404+
it("does not regenerate when the chat view remounts after a generation", async () => {
405+
mockGenerateTitle.mockResolvedValue({
406+
title: "Fix login bug",
407+
summary: "User is fixing a login issue",
408+
});
409+
mockPrompts.value = ["Fix the login bug"];
410+
411+
const first = renderHook(() =>
412+
useChatTitleGenerator(createTask({ title: "Raw prompt title" })),
413+
);
414+
await waitFor(() => {
415+
expect(mockGenerateTitle).toHaveBeenCalledTimes(1);
416+
});
417+
first.unmount();
418+
419+
renderHook(() =>
420+
useChatTitleGenerator(createTask({ title: "Fix login bug" })),
421+
);
422+
423+
await new Promise((resolve) => setTimeout(resolve, 0));
424+
expect(mockGenerateTitle).toHaveBeenCalledTimes(1);
425+
});
426+
427+
it("shares one in-flight guard across simultaneously mounted views", async () => {
428+
let resolveGeneration!: (value: TitleAndSummaryResult) => void;
429+
mockGenerateTitle.mockReturnValue(
430+
new Promise((resolve) => {
431+
resolveGeneration = resolve;
432+
}),
433+
);
434+
mockPrompts.value = ["Fix the login bug"];
435+
436+
renderHook(() =>
437+
useChatTitleGenerator(createTask({ title: "Raw prompt title" })),
438+
);
439+
renderHook(() =>
440+
useChatTitleGenerator(createTask({ title: "Raw prompt title" })),
441+
);
442+
443+
await waitFor(() => {
444+
expect(mockGenerateTitle).toHaveBeenCalledTimes(1);
445+
});
446+
447+
resolveGeneration({ title: "Fix login bug", summary: "" });
448+
await new Promise((resolve) => setTimeout(resolve, 0));
449+
expect(mockGenerateTitle).toHaveBeenCalledTimes(1);
450+
});
451+
452+
it("skips catch-up generation when the title is locked and a summary exists", async () => {
453+
const lockedTask = createTask({
454+
title: "Custom auth title",
455+
description: "fix auth",
456+
title_manually_set: true,
457+
});
458+
cacheTask(lockedTask);
459+
mockSessionSummary.value = "User wants to fix auth";
460+
mockPrompts.value = Array.from({ length: 8 }, (_, i) => `prompt ${i}`);
461+
462+
renderHook(() => useChatTitleGenerator(lockedTask));
463+
464+
await new Promise((resolve) => setTimeout(resolve, 0));
465+
expect(mockGenerateTitle).not.toHaveBeenCalled();
466+
});
467+
468+
it("runs catch-up generation for a locked title when no summary exists yet", async () => {
469+
const lockedTask = createTask({
470+
title: "Custom auth title",
471+
description: "fix auth",
472+
title_manually_set: true,
473+
});
474+
cacheTask(lockedTask);
475+
mockGenerateTitle.mockResolvedValue({
476+
title: "Auto title",
477+
summary: "User wants to fix auth",
478+
});
479+
mockPrompts.value = Array.from({ length: 8 }, (_, i) => `prompt ${i}`);
480+
481+
renderHook(() => useChatTitleGenerator(lockedTask));
482+
483+
await waitFor(() => {
484+
expect(mockGenerateTitle).toHaveBeenCalledTimes(1);
485+
});
486+
expect(mockUpdateTask).not.toHaveBeenCalled();
487+
});
385488
});

packages/ui/src/features/sessions/hooks/useChatTitleGenerator.ts

Lines changed: 25 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,15 @@ import {
1818
sessionStoreSetters,
1919
useSessionStore,
2020
} from "@posthog/ui/features/sessions/sessionStore";
21+
import {
22+
type TitleGenerationEntry,
23+
titleGenerationStoreApi,
24+
} from "@posthog/ui/features/sessions/titleGenerationStore";
2125
import { taskKeys } from "@posthog/ui/features/tasks/taskKeys";
2226
import { logger } from "@posthog/ui/shell/logger";
2327
import { titleAttachmentStoreApi } from "@posthog/ui/shell/titleAttachmentStore";
2428
import { type QueryClient, useQueryClient } from "@tanstack/react-query";
25-
import { useEffect, useRef } from "react";
29+
import { useEffect } from "react";
2630

2731
const log = logger.scope("chat-title-generator");
2832

@@ -44,9 +48,6 @@ export function useChatTitleGenerator(task: Task): void {
4448
);
4549
const queryClient = useQueryClient();
4650
const client = useOptionalAuthenticatedClient();
47-
const lastGeneratedAtCount = useRef(0);
48-
const initialDescriptionHandled = useRef(false);
49-
const isGenerating = useRef(false);
5051
const isAuthenticated = useAuthStateValue(
5152
(state) => state.status === "authenticated" && !!state.cloudRegion,
5253
);
@@ -61,30 +62,37 @@ export function useChatTitleGenerator(task: Task): void {
6162

6263
useEffect(() => {
6364
if (!isAuthenticated) return;
64-
if (isGenerating.current) return;
65+
66+
const bookkeeping = titleGenerationStoreApi.get(taskId);
67+
if (bookkeeping.inFlight) return;
68+
69+
const state = useSessionStore.getState();
70+
const taskRunId = state.taskIdIndex[taskId];
71+
const session = taskRunId ? state.sessions[taskRunId] : undefined;
72+
const isTitleLocked = () =>
73+
isAutoTitleLocked(getCachedTask(queryClient, taskId) ?? task);
6574

6675
const { shouldGenerateFromPrompts, shouldGenerateFromTaskDescription } =
6776
decideTitleGeneration({
6877
promptCount,
69-
lastGeneratedAtCount: lastGeneratedAtCount.current,
70-
initialDescriptionHandled: initialDescriptionHandled.current,
78+
lastGeneratedAtCount: bookkeeping.lastGeneratedAtCount,
79+
initialDescriptionHandled: bookkeeping.initialDescriptionHandled,
7180
task,
81+
isTitleLocked,
82+
hasSummary: !!session?.conversationSummary,
7283
});
7384

7485
if (!shouldGenerateFromPrompts && !shouldGenerateFromTaskDescription) {
7586
return;
7687
}
7788

78-
isGenerating.current = true;
89+
titleGenerationStoreApi.update(taskId, { inFlight: true });
7990

80-
const state = useSessionStore.getState();
81-
const taskRunId = state.taskIdIndex[taskId];
82-
const session = taskRunId ? state.sessions[taskRunId] : undefined;
8391
let rawContent = task.description;
8492

8593
if (shouldGenerateFromPrompts) {
8694
if (!session?.events) {
87-
isGenerating.current = false;
95+
titleGenerationStoreApi.update(taskId, { inFlight: false });
8896
return;
8997
}
9098

@@ -109,11 +117,8 @@ export function useChatTitleGenerator(task: Task): void {
109117
// up and try again with the file contents.
110118
titleAttachmentStoreApi.clear(taskId);
111119
const { title, summary } = result;
112-
const titleLocked = isAutoTitleLocked(
113-
getCachedTask(queryClient, taskId) ?? task,
114-
);
115120

116-
if (title && titleLocked) {
121+
if (title && isTitleLocked()) {
117122
log.debug("Skipping auto-title, user renamed task", { taskId });
118123
} else if (title) {
119124
if (client) {
@@ -157,13 +162,14 @@ export function useChatTitleGenerator(task: Task): void {
157162
} catch (error) {
158163
log.error("Failed to update task title", { taskId, error });
159164
} finally {
165+
const patch: Partial<TitleGenerationEntry> = { inFlight: false };
160166
if (shouldGenerateFromPrompts) {
161-
lastGeneratedAtCount.current = promptCount;
167+
patch.lastGeneratedAtCount = promptCount;
162168
}
163169
if (shouldGenerateFromTaskDescription) {
164-
initialDescriptionHandled.current = true;
170+
patch.initialDescriptionHandled = true;
165171
}
166-
isGenerating.current = false;
172+
titleGenerationStoreApi.update(taskId, patch);
167173
}
168174
};
169175

0 commit comments

Comments
 (0)