Skip to content

feat: remove assistantMessageParser experiment flag and enable for all users #7300

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Aug 21, 2025
Merged

feat: remove assistantMessageParser experiment flag and enable for all users #7300

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 1 addition & 7 deletions packages/types/src/experiment.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,7 @@ import type { Keys, Equals, AssertEqual } from "./type-fu.js"
* ExperimentId
*/

export const experimentIds = [
"powerSteering",
"multiFileApplyDiff",
"preventFocusDisruption",
"assistantMessageParser",
] as const
export const experimentIds = ["powerSteering", "multiFileApplyDiff", "preventFocusDisruption"] as const
Copy link
Contributor Author

@roomote roomote bot Aug 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this removes an experimental flag and makes the feature default for all users, should we consider adding a note about any potential impacts on existing users who may have had the experiment disabled? Perhaps in the PR description or as a comment here?


export const experimentIdsSchema = z.enum(experimentIds)

Expand All @@ -25,7 +20,6 @@ export const experimentsSchema = z.object({
powerSteering: z.boolean().optional(),
multiFileApplyDiff: z.boolean().optional(),
preventFocusDisruption: z.boolean().optional(),
assistantMessageParser: z.boolean().optional(),
})

export type Experiments = z.infer<typeof experimentsSchema>
Expand Down
30 changes: 10 additions & 20 deletions src/core/task/Task.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ import { ToolRepetitionDetector } from "../tools/ToolRepetitionDetector"
import { FileContextTracker } from "../context-tracking/FileContextTracker"
import { RooIgnoreController } from "../ignore/RooIgnoreController"
import { RooProtectedController } from "../protect/RooProtectedController"
import { type AssistantMessageContent, presentAssistantMessage, parseAssistantMessage } from "../assistant-message"
import { type AssistantMessageContent, presentAssistantMessage } from "../assistant-message"
Copy link
Contributor Author

@roomote roomote bot Aug 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The import statement still includes parseAssistantMessage which is no longer used after this refactor. Could we clean this up?

import { AssistantMessageParser } from "../assistant-message/AssistantMessageParser"
import { truncateConversationIfNeeded } from "../sliding-window"
import { ClineProvider } from "../webview/ClineProvider"
Expand Down Expand Up @@ -270,8 +270,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
didRejectTool = false
didAlreadyUseTool = false
didCompleteReadingStream = false
assistantMessageParser?: AssistantMessageParser
isAssistantMessageParserEnabled = false
assistantMessageParser: AssistantMessageParser
private lastUsedInstructions?: string
private skipPrevResponseIdOnce: boolean = false

Expand Down Expand Up @@ -355,6 +354,9 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
TelemetryService.instance.captureTaskCreated(this.taskId)
}

// Initialize the assistant message parser
this.assistantMessageParser = new AssistantMessageParser()
Copy link
Contributor Author

@roomote roomote bot Aug 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that AssistantMessageParser is enabled for all users, could we verify that it has adequate test coverage? The existing tests have been updated to remove the experiment references, but it would be good to ensure the parser functionality itself is well-tested.


// Only set up diff strategy if diff is enabled.
if (this.diffEnabled) {
// Default to old strategy, will be updated if experiment is enabled.
Expand Down Expand Up @@ -1751,9 +1753,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
this.didAlreadyUseTool = false
this.presentAssistantMessageLocked = false
this.presentAssistantMessageHasPendingUpdates = false
if (this.assistantMessageParser) {
this.assistantMessageParser.reset()
}
this.assistantMessageParser.reset()

await this.diffViewProvider.reset()

Expand Down Expand Up @@ -1794,12 +1794,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {

// Parse raw assistant message chunk into content blocks.
const prevLength = this.assistantMessageContent.length
if (this.isAssistantMessageParserEnabled && this.assistantMessageParser) {
this.assistantMessageContent = this.assistantMessageParser.processChunk(chunk.text)
} else {
// Use the old parsing method when experiment is disabled
this.assistantMessageContent = parseAssistantMessage(assistantMessage)
}
this.assistantMessageContent = this.assistantMessageParser.processChunk(chunk.text)

if (this.assistantMessageContent.length > prevLength) {
// New content we need to present, reset to
Expand Down Expand Up @@ -2058,11 +2053,8 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
// this.assistantMessageContent.forEach((e) => (e.partial = false))

// Now that the stream is complete, finalize any remaining partial content blocks
if (this.isAssistantMessageParserEnabled && this.assistantMessageParser) {
this.assistantMessageParser.finalizeContentBlocks()
this.assistantMessageContent = this.assistantMessageParser.getContentBlocks()
}
// When using old parser, no finalization needed - parsing already happened during streaming
this.assistantMessageParser.finalizeContentBlocks()
this.assistantMessageContent = this.assistantMessageParser.getContentBlocks()

if (partialBlocks.length > 0) {
// If there is content to update then it will complete and
Expand All @@ -2081,9 +2073,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
await this.providerRef.deref()?.postStateToWebview()

// Reset parser after each complete conversation round
if (this.assistantMessageParser) {
this.assistantMessageParser.reset()
}
this.assistantMessageParser.reset()

// Now add to apiConversationHistory.
// Need to save assistant responses to file before proceeding to
Expand Down
3 changes: 0 additions & 3 deletions src/shared/__tests__/experiments.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ describe("experiments", () => {
powerSteering: false,
multiFileApplyDiff: false,
preventFocusDisruption: false,
assistantMessageParser: false,
}
expect(Experiments.isEnabled(experiments, EXPERIMENT_IDS.POWER_STEERING)).toBe(false)
})
Expand All @@ -39,7 +38,6 @@ describe("experiments", () => {
powerSteering: true,
multiFileApplyDiff: false,
preventFocusDisruption: false,
assistantMessageParser: false,
}
expect(Experiments.isEnabled(experiments, EXPERIMENT_IDS.POWER_STEERING)).toBe(true)
})
Expand All @@ -49,7 +47,6 @@ describe("experiments", () => {
powerSteering: false,
multiFileApplyDiff: false,
preventFocusDisruption: false,
assistantMessageParser: false,
}
expect(Experiments.isEnabled(experiments, EXPERIMENT_IDS.POWER_STEERING)).toBe(false)
})
Expand Down
2 changes: 0 additions & 2 deletions src/shared/experiments.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ export const EXPERIMENT_IDS = {
MULTI_FILE_APPLY_DIFF: "multiFileApplyDiff",
POWER_STEERING: "powerSteering",
PREVENT_FOCUS_DISRUPTION: "preventFocusDisruption",
ASSISTANT_MESSAGE_PARSER: "assistantMessageParser",
} as const satisfies Record<string, ExperimentId>

type _AssertExperimentIds = AssertEqual<Equals<ExperimentId, Values<typeof EXPERIMENT_IDS>>>
Expand All @@ -19,7 +18,6 @@ export const experimentConfigsMap: Record<ExperimentKey, ExperimentConfig> = {
MULTI_FILE_APPLY_DIFF: { enabled: false },
POWER_STEERING: { enabled: false },
PREVENT_FOCUS_DISRUPTION: { enabled: false },
ASSISTANT_MESSAGE_PARSER: { enabled: false },
}

export const experimentDefault = Object.fromEntries(
Expand Down
2 changes: 0 additions & 2 deletions webview-ui/src/context/__tests__/ExtensionStateContext.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,6 @@ describe("mergeExtensionState", () => {
concurrentFileReads: true,
multiFileApplyDiff: true,
preventFocusDisruption: false,
assistantMessageParser: false,
newTaskRequireTodos: false,
} as Record<ExperimentId, boolean>,
}
Expand All @@ -248,7 +247,6 @@ describe("mergeExtensionState", () => {
concurrentFileReads: true,
multiFileApplyDiff: true,
preventFocusDisruption: false,
assistantMessageParser: false,
newTaskRequireTodos: false,
})
})
Expand Down
Loading