-
Notifications
You must be signed in to change notification settings - Fork 2.3k
feat: Summarize task titles for legibility and scannability (WIP) #8640
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
base: main
Are you sure you want to change the base?
Conversation
it("should handle missing API configuration", async () => { | ||
const result = await TitleSummarizer.summarizeTitle({ | ||
text: "Some text", | ||
apiConfiguration: undefined as any, | ||
maxLength: 150, | ||
}) | ||
|
||
expect(result.success).toBe(false) | ||
expect(result.error).toBe("No API configuration available") | ||
expect(result.summarizedTitle).toBe("Some text") |
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.
This test has incorrect expectations. When text
is "Some text" (9 characters) and maxLength
is 150, the early return on line 46 of titleSummarizer.ts
will execute, returning { success: true, summarizedTitle: text }
without ever validating the API configuration.
The test expects success: false
and an error about missing API configuration, but this validation never occurs due to the short text length.
To properly test missing API configuration handling, use text longer than maxLength
to bypass the early return:
const longText = "a".repeat(151) // Exceeds maxLength of 150
const result = await TitleSummarizer.summarizeTitle({
text: longText,
apiConfiguration: undefined as any,
maxLength: 150,
})
expect(result.success).toBe(false)
expect(result.error).toBe("No valid API configuration provided")
Note: The error message should also be "No valid API configuration provided" (from singleCompletionHandler.ts:14
) not "No API configuration available".
it("should use custom support prompts when provided", async () => { | ||
const customPrompts = { | ||
SUMMARIZE_TITLE: "Custom summarization prompt: {{userInput}} (max 150 chars)", | ||
} | ||
|
||
// Short text doesn't need summarization | ||
const result = await TitleSummarizer.summarizeTitle({ | ||
text: "Text to summarize", | ||
apiConfiguration: mockApiConfiguration, | ||
customSupportPrompts: customPrompts, | ||
maxLength: 150, | ||
}) | ||
|
||
expect(result.success).toBe(true) | ||
expect(result.summarizedTitle).toBe("Text to summarize") |
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.
This test doesn't actually verify custom support prompts functionality. The text "Text to summarize" (18 characters) is shorter than maxLength
(150), triggering the early return on line 46 that bypasses prompt creation entirely.
To properly test custom support prompts:
const customPrompts = {
SUMMARIZE_TITLE: "Custom summarization prompt: {{userInput}} (max 150 chars)",
}
const longText = "a".repeat(151)
const result = await TitleSummarizer.summarizeTitle({
text: longText,
apiConfiguration: mockApiConfiguration,
customSupportPrompts: customPrompts,
maxLength: 150,
})
// Verify the custom prompt was actually used
expect(singleCompletionHandler).toHaveBeenCalledWith(
mockApiConfiguration,
expect.stringContaining("Custom summarization prompt")
)
@@ -0,0 +1,125 @@ | ||
import { ProviderSettings, ClineMessage, TelemetryEventName } from "@roo-code/types" |
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.
Unused import: ClineMessage
is imported but never used in this file. Please remove it to keep imports clean.
import { ProviderSettings, ClineMessage, TelemetryEventName } from "@roo-code/types" | ||
import { TelemetryService } from "@roo-code/telemetry" | ||
import { supportPrompt } from "../../shared/support-prompt" | ||
import { singleCompletionHandler } from "../../utils/single-completion-handler" | ||
import { ProviderSettingsManager } from "../config/ProviderSettingsManager" | ||
import { ClineProvider } from "./ClineProvider" |
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.
Unused imports: Both TelemetryEventName
and ClineProvider
are imported but never used in this file. The ClineProvider
import (line 6) is particularly unnecessary since there's no reference to it anywhere.
Please remove both unused imports:
import { ProviderSettings } from "@roo-code/types"
import { TelemetryService } from "@roo-code/telemetry"
import { supportPrompt } from "../../shared/support-prompt"
import { singleCompletionHandler } from "../../utils/single-completion-handler"
import { ProviderSettingsManager } from "../config/ProviderSettingsManager"
Related GitHub Issue
Closes: #
Roo Code Task Context (Optional)
Description
Test Procedure
Pre-Submission Checklist
Screenshots / Videos
Documentation Updates
Additional Notes
Get in Touch