Skip to content

Commit d03dfad

Browse files
daewoongohdaewoongoh
committed
fix(webview): stop flushing throttled state for partial say messages
`addToClineMessages` called `flushPostStateToWebviewThrottled()` for every message with `partial === true`. With lodash debounce configured `{ leading: true, trailing: true }`, a flush issued right after a leading-edge invocation has no pending trailing invocation to run, so it only cancels the trailing timer. The next call then hits the leading edge again and posts immediately, so the debounce never coalesced anything on the streaming path. Measured against lodash.debounce 4.0.8 with the provider's own settings (500ms wait, 1000ms maxWait), 20 messages arriving 100ms apart produced: with the interleaved flush: 20 full-state posts without it: 3 full-state posts Because `requiresImmediateState` matched every partial first chunk, throttling was effectively inert for tool-heavy tasks and the gray-screen OOM that #1078 set out to fix remained reproducible at high message counts. Drop `partial` from the condition. Partial `say` messages are safe on the throttled path: the trailing/maxWait post carries the message's current text, so a `messageUpdated` dropped for a `ts` the webview does not know yet is superseded rather than lost. Partial asks are unaffected — `Task#ask` adds them without `isAnswered`, so they keep flushing through the ask clause and retain the ordering guarantee unanswered asks depend on. The existing suite could not catch this: every Task-level test mocks the provider, so it asserted that flush was *called* rather than that state posts were actually coalesced. Retarget the partial-message test at the new contract and add a characterization test in ClineProvider.spec.ts that pins the debounce semantics against the real timer.
1 parent 0b2b228 commit d03dfad

3 files changed

Lines changed: 48 additions & 22 deletions

File tree

‎src/core/task/Task.ts‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1149,8 +1149,19 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
11491149
this.clineMessages.push(message)
11501150
const provider = this.providerRef.deref()
11511151
// Unanswered asks must reach the webview before Message listeners can respond against its state.
1152-
const requiresImmediateState =
1153-
message.partial === true || (message.type === "ask" && message.isAnswered !== true)
1152+
//
1153+
// Partial `say` messages deliberately do NOT flush. `flushPostStateToWebviewThrottled()`
1154+
// invoked right after a leading-edge debounce call has no pending trailing invocation to
1155+
// run, so it only cancels the trailing timer — which makes the *next* call hit the leading
1156+
// edge and post immediately. Flushing on every partial therefore defeated the debounce
1157+
// entirely (one full-state post per message, the very behaviour #1078 set out to remove).
1158+
// They are safe on the throttled path: the trailing/maxWait post carries the message's
1159+
// current text, so a `messageUpdated` dropped for a not-yet-known `ts` is superseded
1160+
// rather than lost.
1161+
//
1162+
// Partial *asks* still flush, via the clause below — `Task#ask` adds them without
1163+
// `isAnswered`, so they keep the ordering guarantee that unanswered asks depend on.
1164+
const requiresImmediateState = message.type === "ask" && message.isAnswered !== true
11541165
try {
11551166
await provider?.postStateToWebviewThrottled()
11561167
} catch (error) {

‎src/core/task/__tests__/Task.spec.ts‎

Lines changed: 8 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2050,7 +2050,7 @@ describe("Cline", () => {
20502050
expect(mockProvider.flushPostStateToWebviewThrottled).not.toHaveBeenCalled()
20512051
})
20522052

2053-
it("waits for a new partial message flush before a following message update", async () => {
2053+
it("keeps a new partial message on the throttled path without flushing", async () => {
20542054
const task = new Task({
20552055
provider: mockProvider,
20562056
apiConfiguration: mockApiConfig,
@@ -2059,11 +2059,6 @@ describe("Cline", () => {
20592059
})
20602060
const taskAccess = getTaskTestAccess(task)
20612061
vi.spyOn(taskAccess, "saveClineMessages").mockResolvedValue(true)
2062-
let releaseFlush!: () => void
2063-
const pendingFlush = new Promise<void>((resolve) => {
2064-
releaseFlush = resolve
2065-
})
2066-
const flushSpy = vi.mocked(mockProvider.flushPostStateToWebviewThrottled).mockReturnValueOnce(pendingFlush)
20672062
const updatePostSpy = vi.mocked(mockProvider.postMessageToWebview)
20682063
const partialMessage = {
20692064
ts: 1,
@@ -2072,22 +2067,15 @@ describe("Cline", () => {
20722067
text: "partial message",
20732068
partial: true,
20742069
}
2075-
let partialAddSettled = false
2076-
const addThenUpdate = taskAccess.addToClineMessages(partialMessage).then(async () => {
2077-
partialAddSettled = true
2078-
await taskAccess.updateClineMessage({ ...partialMessage, text: "updated partial" })
2079-
})
20802070

2081-
await Promise.resolve()
2082-
expect(mockProvider.postStateToWebviewThrottled).toHaveBeenCalledWith()
2083-
expect(flushSpy).toHaveBeenCalledWith()
2084-
expect(partialAddSettled).toBe(false)
2085-
expect(updatePostSpy).not.toHaveBeenCalled()
2086-
2087-
releaseFlush()
2088-
await addThenUpdate
2071+
await taskAccess.addToClineMessages(partialMessage)
2072+
await taskAccess.updateClineMessage({ ...partialMessage, text: "updated partial" })
20892073

2090-
expect(flushSpy.mock.invocationCallOrder[0]).toBeLessThan(updatePostSpy.mock.invocationCallOrder[0])
2074+
// Flushing here would cancel the debounce's trailing timer and send the *next*
2075+
// message straight through the leading edge, collapsing the throttle back to one
2076+
// full-state post per message.
2077+
expect(mockProvider.postStateToWebviewThrottled).toHaveBeenCalledWith()
2078+
expect(mockProvider.flushPostStateToWebviewThrottled).not.toHaveBeenCalled()
20912079
expect(updatePostSpy).toHaveBeenCalledWith({
20922080
type: "messageUpdated",
20932081
clineMessage: {

‎src/core/webview/__tests__/ClineProvider.spec.ts‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1016,6 +1016,33 @@ describe("ClineProvider", () => {
10161016
expect(postStateSpy).toHaveBeenCalledTimes(2)
10171017
})
10181018

1019+
// Characterization test for the semantics that made #1078's throttle a no-op in practice:
1020+
// flushing right after a leading-edge post has no pending trailing invocation to run, so it
1021+
// only cancels the trailing timer — and the next call then hits the leading edge again.
1022+
// Callers on a hot path (see Task#addToClineMessages) must therefore not flush per message.
1023+
test("flushing after every post defeats coalescing entirely", async () => {
1024+
const postStateSpy = vi.spyOn(provider, "postStateToWebviewWithoutTaskHistory").mockResolvedValue(undefined)
1025+
1026+
for (let i = 0; i < 5; i++) {
1027+
await provider.postStateToWebviewThrottled()
1028+
await provider.flushPostStateToWebviewThrottled()
1029+
await vi.advanceTimersByTimeAsync(100)
1030+
}
1031+
1032+
// One full-state post per call: no coalescing at all.
1033+
expect(postStateSpy).toHaveBeenCalledTimes(5)
1034+
1035+
// The same burst without the interleaved flush coalesces into far fewer posts.
1036+
postStateSpy.mockClear()
1037+
for (let i = 0; i < 5; i++) {
1038+
await provider.postStateToWebviewThrottled()
1039+
await vi.advanceTimersByTimeAsync(100)
1040+
}
1041+
await vi.advanceTimersByTimeAsync(500)
1042+
1043+
expect(postStateSpy.mock.calls.length).toBeLessThan(5)
1044+
})
1045+
10191046
test("flushes a pending trailing post exactly once and waits for it", async () => {
10201047
let releasePost!: () => void
10211048
const pendingPost = new Promise<void>((resolve) => {

0 commit comments

Comments
 (0)