-
Notifications
You must be signed in to change notification settings - Fork 3.9k
feat: Generate message titles in CLI #7435
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
6 issues found across 16 files
React with 👍 or 👎 to teach cubic. You can also tag @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| import { processSlashCommandResult } from "./useChat.helpers.js"; | ||
|
|
||
| // Mock the session module | ||
| vi.mock("../../session.js", () => ({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clear mocks in beforeEach to prevent call history leaking between tests and reduce flakiness.
Prompt for AI agents
Address the following comment on extensions/cli/src/ui/hooks/useChat.clear.test.ts at line 10:
<comment>Clear mocks in beforeEach to prevent call history leaking between tests and reduce flakiness.</comment>
<file context>
@@ -2,9 +2,17 @@ import { ChatCompletionMessageParam } from "openai/resources.mjs";
import { processSlashCommandResult } from "./useChat.helpers.js";
+// Mock the session module
+vi.mock("../../session.js", () => ({
+ loadSession: vi.fn(),
+ updateSessionHistory: vi.fn(),
</file context>
core/util/chatDescriber.ts
Outdated
| try { | ||
| // Try to import CLI dependencies dynamically | ||
| const { convertFromUnifiedHistory } = await import( | ||
| "../../extensions/cli/src/messageConversion.js" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The core module is dynamically importing from extensions/cli, violating the architectural principle that core should be independent of any extensions. This creates tight coupling and reduces reusability. Dependencies should be inverted, e.g., by passing required functions as arguments.
Prompt for AI agents
Address the following comment on core/util/chatDescriber.ts at line 80:
<comment>The `core` module is dynamically importing from `extensions/cli`, violating the architectural principle that `core` should be independent of any `extensions`. This creates tight coupling and reduces reusability. Dependencies should be inverted, e.g., by passing required functions as arguments.</comment>
<file context>
@@ -46,6 +46,91 @@ export class ChatDescriber {
+ try {
+ // Try to import CLI dependencies dynamically
+ const { convertFromUnifiedHistory } = await import(
+ "../../extensions/cli/src/messageConversion.js"
+ );
+ const { getDefaultCompletionOptions } = await import(
</file context>
| currentSessionTitle?: string, | ||
| ): Promise<string | undefined> { | ||
| // Only generate title for untitled sessions | ||
| if (currentSessionTitle && currentSessionTitle !== "Untitled Session") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid hard-coded "Untitled Session" string; reference a shared constant to keep session title checks consistent across modules.
Prompt for AI agents
Address the following comment on extensions/cli/src/ui/hooks/useChat.helpers.ts at line 429:
<comment>Avoid hard-coded "Untitled Session" string; reference a shared constant to keep session title checks consistent across modules.</comment>
<file context>
@@ -407,3 +415,41 @@ export async function handleAutoCompaction({
+ currentSessionTitle?: string,
+): Promise<string | undefined> {
+ // Only generate title for untitled sessions
+ if (currentSessionTitle && currentSessionTitle !== "Untitled Session") {
+ return undefined;
+ }
</file context>
| format: options.format, | ||
| silent: options.silent, | ||
| compactionIndex, | ||
| firstAssistantResponse: isFirstMessage && !options.resume, // Only generate title for new conversations |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
firstAssistantResponse is computed after isFirstMessage is set to false, so the title generation path never triggers; compute it without depending on the mutated flag or capture it before toggling
Prompt for AI agents
Address the following comment on extensions/cli/src/commands/chat.ts at line 475:
<comment>firstAssistantResponse is computed after isFirstMessage is set to false, so the title generation path never triggers; compute it without depending on the mutated flag or capture it before toggling</comment>
<file context>
@@ -433,6 +472,7 @@ async function runHeadlessMode(
format: options.format,
silent: options.silent,
compactionIndex,
+ firstAssistantResponse: isFirstMessage && !options.resume, // Only generate title for new conversations
});
</file context>
| firstAssistantResponse: isFirstMessage && !options.resume, // Only generate title for new conversations | |
| firstAssistantResponse: !options.resume && chatHistory.every(item => item.message.role === "system"), // Only generate title for new conversations |
| currentSession.title, | ||
| ); | ||
|
|
||
| if (generatedTitle) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Possible race: title can be overwritten if it changes while awaiting generateSessionTitle; recheck the session is still untitled before updating.
(Based on your team's feedback about session history-related bugs, I reviewed title updates for race conditions.)
Prompt for AI agents
Address the following comment on extensions/cli/src/ui/hooks/useChat.stream.helpers.ts at line 48:
<comment>Possible race: title can be overwritten if it changes while awaiting generateSessionTitle; recheck the session is still untitled before updating.
(Based on your team's feedback about session history-related bugs, I reviewed title updates for race conditions.)</comment>
<file context>
@@ -28,6 +34,21 @@ export function createStreamCallbacks(
+ currentSession.title,
+ );
+
+ if (generatedTitle) {
+ updateSessionTitle(generatedTitle);
+ }
</file context>
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
|
🎉 This PR is included in version 1.12.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.12.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Description
Generate message titles for the CLI. Reuses existing core logic. Also addresses a few session history-related bugs
Summary by cubic
Generates session titles in the CLI using core logic, updating the title after the first assistant reply. Also fixes session history ordering, improves org switching, and makes /clear start a fresh session.
New Features
Bug Fixes