fix(sync): deduplicate overlapping delta after coalesced part.updated (#916)
* fix: hide archived section and empty folders when no sessions remain - Only push archived group in useSessionGrouping when there are archived sessions, preventing an empty archived section from rendering - Hide empty folders in archived bucket via shouldKeepFolder check in SessionGroupSection (folders with no sessions and no content in children are filtered out) - Always filter folders through shouldKeepFolder, not just during search * perf: memoize archived folder filtering * fix(sync): deduplicate overlapping delta after coalesced part.updated When message.part.updated coalesces in the event pipeline and a message.part.delta for the same part arrives in the same flush window, the reducer appends the delta verbatim to the already-complete field value, producing duplicated text in tool output and assistant messages. Add targeted overlap reconciliation: only when a part.updated replaces an existing part with overlapping string content, mark the next delta for that field as dedupe-eligible. Normal streaming deltas remain untouched (pure append). Covers four cases: - Full overlap: delta already present -> no-op - Partial overlap: only non-overlapping suffix appended - No overlap: unchanged append behavior - Legitimate repeated output (ha + ha -> haha): preserved --------- Co-authored-by: Bohdan Triapitsyn <artmore@protonmail.com>
This commit is contained in:
committed by
GitHub
co-authored by
Bohdan Triapitsyn
parent
9908f2dc9c
commit
ea5c19e934
@@ -0,0 +1,154 @@
|
|||||||
|
import { describe, expect, it } from 'bun:test'
|
||||||
|
import { applyDirectoryEvent } from '../event-reducer'
|
||||||
|
import { INITIAL_STATE } from '../types'
|
||||||
|
|
||||||
|
describe('applyDirectoryEvent', () => {
|
||||||
|
it('does not duplicate overlapping delta text after a newer part.updated replaces an older one', () => {
|
||||||
|
const state = structuredClone(INITIAL_STATE)
|
||||||
|
const messageID = 'msg-1'
|
||||||
|
const partID = 'part-1'
|
||||||
|
|
||||||
|
applyDirectoryEvent(state, {
|
||||||
|
type: 'message.part.updated',
|
||||||
|
properties: {
|
||||||
|
part: {
|
||||||
|
id: partID,
|
||||||
|
type: 'text',
|
||||||
|
messageID,
|
||||||
|
text: 'Fix typo in ToolOutputDialog — ',
|
||||||
|
},
|
||||||
|
},
|
||||||
|
})
|
||||||
|
|
||||||
|
applyDirectoryEvent(state, {
|
||||||
|
type: 'message.part.updated',
|
||||||
|
properties: {
|
||||||
|
part: {
|
||||||
|
id: partID,
|
||||||
|
type: 'text',
|
||||||
|
messageID,
|
||||||
|
text: 'Fix typo in ToolOutputDialog — toolFailedToReadDiagram vs toolFailedReadDiagram • Let me fix it.',
|
||||||
|
},
|
||||||
|
},
|
||||||
|
})
|
||||||
|
|
||||||
|
applyDirectoryEvent(state, {
|
||||||
|
type: 'message.part.delta',
|
||||||
|
properties: {
|
||||||
|
messageID,
|
||||||
|
partID,
|
||||||
|
field: 'text',
|
||||||
|
delta: 'toolFailedToReadDiagram vs toolFailedReadDiagram • Let me fix it.',
|
||||||
|
},
|
||||||
|
})
|
||||||
|
|
||||||
|
expect(state.part[messageID]).toHaveLength(1)
|
||||||
|
expect(state.part[messageID]?.[0]?.text).toBe(
|
||||||
|
'Fix typo in ToolOutputDialog — toolFailedToReadDiagram vs toolFailedReadDiagram • Let me fix it.',
|
||||||
|
)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('appends only the non-overlapping suffix of a streaming delta', () => {
|
||||||
|
const state = structuredClone(INITIAL_STATE)
|
||||||
|
const messageID = 'msg-2'
|
||||||
|
const partID = 'part-2'
|
||||||
|
|
||||||
|
applyDirectoryEvent(state, {
|
||||||
|
type: 'message.part.updated',
|
||||||
|
properties: {
|
||||||
|
part: {
|
||||||
|
id: partID,
|
||||||
|
type: 'text',
|
||||||
|
messageID,
|
||||||
|
text: 'toolFailedToReadDiagram vs toolFailedRead',
|
||||||
|
},
|
||||||
|
},
|
||||||
|
})
|
||||||
|
|
||||||
|
applyDirectoryEvent(state, {
|
||||||
|
type: 'message.part.updated',
|
||||||
|
properties: {
|
||||||
|
part: {
|
||||||
|
id: partID,
|
||||||
|
type: 'text',
|
||||||
|
messageID,
|
||||||
|
text: 'toolFailedToReadDiagram vs toolFailedReadDiagra',
|
||||||
|
},
|
||||||
|
},
|
||||||
|
})
|
||||||
|
|
||||||
|
applyDirectoryEvent(state, {
|
||||||
|
type: 'message.part.delta',
|
||||||
|
properties: {
|
||||||
|
messageID,
|
||||||
|
partID,
|
||||||
|
field: 'text',
|
||||||
|
delta: 'Diagram • Let me fix it.',
|
||||||
|
},
|
||||||
|
})
|
||||||
|
|
||||||
|
expect(state.part[messageID]?.[0]?.text).toBe(
|
||||||
|
'toolFailedToReadDiagram vs toolFailedReadDiagram • Let me fix it.',
|
||||||
|
)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('appends a non-overlapping delta unchanged', () => {
|
||||||
|
const state = structuredClone(INITIAL_STATE)
|
||||||
|
const messageID = 'msg-3'
|
||||||
|
const partID = 'part-3'
|
||||||
|
|
||||||
|
applyDirectoryEvent(state, {
|
||||||
|
type: 'message.part.updated',
|
||||||
|
properties: {
|
||||||
|
part: {
|
||||||
|
id: partID,
|
||||||
|
type: 'text',
|
||||||
|
messageID,
|
||||||
|
text: 'PR comment done — ',
|
||||||
|
},
|
||||||
|
},
|
||||||
|
})
|
||||||
|
|
||||||
|
applyDirectoryEvent(state, {
|
||||||
|
type: 'message.part.delta',
|
||||||
|
properties: {
|
||||||
|
messageID,
|
||||||
|
partID,
|
||||||
|
field: 'text',
|
||||||
|
delta: 'Let me fix it.',
|
||||||
|
},
|
||||||
|
})
|
||||||
|
|
||||||
|
expect(state.part[messageID]?.[0]?.text).toBe('PR comment done — Let me fix it.')
|
||||||
|
})
|
||||||
|
|
||||||
|
it('preserves legitimate repeated output when no updated-to-delta dedupe window is active', () => {
|
||||||
|
const state = structuredClone(INITIAL_STATE)
|
||||||
|
const messageID = 'msg-4'
|
||||||
|
const partID = 'part-4'
|
||||||
|
|
||||||
|
applyDirectoryEvent(state, {
|
||||||
|
type: 'message.part.updated',
|
||||||
|
properties: {
|
||||||
|
part: {
|
||||||
|
id: partID,
|
||||||
|
type: 'text',
|
||||||
|
messageID,
|
||||||
|
text: 'ha',
|
||||||
|
},
|
||||||
|
},
|
||||||
|
})
|
||||||
|
|
||||||
|
applyDirectoryEvent(state, {
|
||||||
|
type: 'message.part.delta',
|
||||||
|
properties: {
|
||||||
|
messageID,
|
||||||
|
partID,
|
||||||
|
field: 'text',
|
||||||
|
delta: 'ha',
|
||||||
|
},
|
||||||
|
})
|
||||||
|
|
||||||
|
expect(state.part[messageID]?.[0]?.text).toBe('haha')
|
||||||
|
})
|
||||||
|
})
|
||||||
@@ -16,6 +16,39 @@ import { stripSessionDiffSnapshots } from "./sanitize"
|
|||||||
import { syncDebug } from "./debug"
|
import { syncDebug } from "./debug"
|
||||||
|
|
||||||
const SKIP_PARTS = new Set(["patch", "step-start", "step-finish"])
|
const SKIP_PARTS = new Set(["patch", "step-start", "step-finish"])
|
||||||
|
const DELTA_OVERLAP_FIELDS = ["text", "output"] as const
|
||||||
|
|
||||||
|
type DedupeMetadata = {
|
||||||
|
__dedupeNextDeltaFields?: string[]
|
||||||
|
}
|
||||||
|
|
||||||
|
function appendNonOverlappingDelta(existingValue: string | undefined, delta: string) {
|
||||||
|
if (!existingValue || delta.length === 0) return (existingValue ?? "") + delta
|
||||||
|
if (existingValue.endsWith(delta)) return existingValue
|
||||||
|
|
||||||
|
const maxOverlap = Math.min(existingValue.length, delta.length)
|
||||||
|
for (let overlap = maxOverlap; overlap > 0; overlap--) {
|
||||||
|
if (existingValue.endsWith(delta.slice(0, overlap))) {
|
||||||
|
return existingValue + delta.slice(overlap)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return existingValue + delta
|
||||||
|
}
|
||||||
|
|
||||||
|
function getUpdatedDeltaFields(previous: Part, next: Part) {
|
||||||
|
const dedupeFields: string[] = []
|
||||||
|
for (const field of DELTA_OVERLAP_FIELDS) {
|
||||||
|
const previousValue = (previous as Record<string, unknown>)[field]
|
||||||
|
const nextValue = (next as Record<string, unknown>)[field]
|
||||||
|
if (typeof previousValue !== "string" || typeof nextValue !== "string") continue
|
||||||
|
if (previousValue.length === 0 || nextValue.length === 0) continue
|
||||||
|
if (nextValue === previousValue || nextValue.startsWith(previousValue) || previousValue.startsWith(nextValue)) {
|
||||||
|
dedupeFields.push(field)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return dedupeFields
|
||||||
|
}
|
||||||
|
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
// Global events
|
// Global events
|
||||||
@@ -205,7 +238,11 @@ export function applyDirectoryEvent(
|
|||||||
const next = [...parts]
|
const next = [...parts]
|
||||||
const result = Binary.search(next, part.id, (p) => p.id)
|
const result = Binary.search(next, part.id, (p) => p.id)
|
||||||
if (result.found) {
|
if (result.found) {
|
||||||
next[result.index] = part
|
const previous = next[result.index]
|
||||||
|
const dedupeFields = getUpdatedDeltaFields(previous, part)
|
||||||
|
next[result.index] = dedupeFields.length > 0
|
||||||
|
? { ...part, __dedupeNextDeltaFields: dedupeFields } as unknown as Part
|
||||||
|
: part
|
||||||
} else {
|
} else {
|
||||||
// Replace optimistic part (no sessionID) with server part of same type.
|
// Replace optimistic part (no sessionID) with server part of same type.
|
||||||
// Gate: only scan if the first part lacks sessionID (optimistic parts are
|
// Gate: only scan if the first part lacks sessionID (optimistic parts are
|
||||||
@@ -262,9 +299,15 @@ export function applyDirectoryEvent(
|
|||||||
}
|
}
|
||||||
const existing = parts[result.index] as Record<string, unknown>
|
const existing = parts[result.index] as Record<string, unknown>
|
||||||
const existingValue = existing[props.field] as string | undefined
|
const existingValue = existing[props.field] as string | undefined
|
||||||
|
const dedupeFields = (existing as DedupeMetadata).__dedupeNextDeltaFields ?? []
|
||||||
|
const shouldDedupe = dedupeFields.includes(props.field)
|
||||||
// Create new Part object + new array so React detects the change
|
// Create new Part object + new array so React detects the change
|
||||||
const next = [...parts]
|
const next = [...parts]
|
||||||
next[result.index] = { ...existing, [props.field]: (existingValue ?? "") + props.delta } as Part
|
next[result.index] = {
|
||||||
|
...existing,
|
||||||
|
[props.field]: shouldDedupe ? appendNonOverlappingDelta(existingValue, props.delta) : (existingValue ?? "") + props.delta,
|
||||||
|
__dedupeNextDeltaFields: dedupeFields.filter((field) => field !== props.field),
|
||||||
|
} as unknown as Part
|
||||||
draft.part[props.messageID] = next
|
draft.part[props.messageID] = next
|
||||||
return true
|
return true
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user