diff --git a/CHANGELOG.md b/CHANGELOG.md index 8492bc4485..d745f4fcbf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -341,6 +341,7 @@ Breaking changes in this release: - Bumped to [`botframework-directlinejs@0.15.8`](https://www.npmjs.com/package/botframework-directlinejs/v/0.15.8) to include support for the new `streaming` property, by [@pranavjoshi001](https://github.com/pranavjoshi001), in PR [#5686](https://github.com/microsoft/BotFramework-WebChat/pull/5686) - Removed unused deps `simple-git`, by [@compulim](https://github.com/compulim), in PR [#5786](https://github.com/microsoft/BotFramework-WebChat/pull/5786) - Improved `ActivityKeyerComposer` performance for append scenarios by adding an incremental fast path that only processes newly-appended activities, in PR [#5790](https://github.com/microsoft/BotFramework-WebChat/pull/5790), by [@OEvgeny](https://github.com/OEvgeny) +- Improved livestream performance by pruning intermediate revision activities after a stream session is finalized, in PR [#5798](https://github.com/microsoft/BotFramework-WebChat/pull/5798), by [@OEvgeny](https://github.com/OEvgeny) ### Deprecated diff --git a/__tests__/html2/livestream/activityOrder.html b/__tests__/html2/livestream/activityOrder.html index 983e368a72..04996331d7 100644 --- a/__tests__/html2/livestream/activityOrder.html +++ b/__tests__/html2/livestream/activityOrder.html @@ -246,7 +246,7 @@ expect(currentActivityKeysWithId).toEqual([ [firstActivityKey, ['a-00001']], [thirdActivityKey, ['a-00003']], - [secondActivityKey, ['t-00001', 't-00002', 't-00003', 'a-00002']] + [secondActivityKey, ['a-00002']] ]); }); diff --git a/__tests__/html2/livestream/backtrackToEmpty.html b/__tests__/html2/livestream/backtrackToEmpty.html index b88f0991b8..dd41304ed1 100644 --- a/__tests__/html2/livestream/backtrackToEmpty.html +++ b/__tests__/html2/livestream/backtrackToEmpty.html @@ -264,7 +264,7 @@ // THEN: Should have 2 activity keys only. expect(currentActivityKeysWithId).toEqual([ [expect.any(String), ['a-00001']], - [firstTypingActivityKey, ['t-00001', 't-00002', 't-00003', 't-00004']] + [firstTypingActivityKey, ['t-00004']] ]); // THEN: Should have no active typing. @@ -287,7 +287,7 @@ // THEN: Should have 2 activity keys only. expect(currentActivityKeysWithId).toEqual([ [expect.any(String), ['a-00001']], - [firstTypingActivityKey, ['t-00001', 't-00002', 't-00003', 't-00004']], + [firstTypingActivityKey, ['t-00004']], [expect.any(String), ['a-00002']] ]); diff --git a/__tests__/html2/livestream/chunk.html b/__tests__/html2/livestream/chunk.html index 38e20c3dc4..69280886a7 100644 --- a/__tests__/html2/livestream/chunk.html +++ b/__tests__/html2/livestream/chunk.html @@ -204,7 +204,7 @@ // THEN: Should have 2 activity keys. expect(currentActivityKeysWithId).toEqual([ [firstActivityKey, ['a-00001']], - [secondActivityKey, ['t-00001', 't-00002', 't-00003', 'a-00002']] + [secondActivityKey, ['a-00002']] ]); }); diff --git a/__tests__/html2/livestream/concludedLivestream.html b/__tests__/html2/livestream/concludedLivestream.html index 79304faaaf..6dac028bcc 100644 --- a/__tests__/html2/livestream/concludedLivestream.html +++ b/__tests__/html2/livestream/concludedLivestream.html @@ -137,7 +137,7 @@ await host.snapshot('local'); // THEN: Should have 1 activity key only. - expect(currentActivityKeysWithId).toEqual([[firstTypingActivityKey, ['t-00001', 't-00002']]]); + expect(currentActivityKeysWithId).toEqual([[firstTypingActivityKey, ['t-00002']]]); // THEN: Should have no active typing. expect(currentActiveTyping).toHaveProperty('u-00001', undefined); @@ -167,8 +167,8 @@ ); await host.snapshot('local'); - // THEN: Should have 1 activity key associated with 2 activity IDs only. - expect(currentActivityKeysWithId).toEqual([[firstTypingActivityKey, ['t-00001', 't-00002']]]); + // THEN: Should have 1 activity key associated with a single activity ID only. + expect(currentActivityKeysWithId).toEqual([[firstTypingActivityKey, ['t-00002']]]); // THEN: Should have no active typing. expect(currentActiveTyping).toHaveProperty('u-00001', undefined); @@ -198,8 +198,8 @@ ); await host.snapshot('local'); - // THEN: Should have 1 activity key associated with 2 activity IDs only. - expect(currentActivityKeysWithId).toEqual([[firstTypingActivityKey, ['t-00001', 't-00002']]]); + // THEN: Should have 1 activity key associated with 1 activity ID only (intermediate revisions pruned). + expect(currentActivityKeysWithId).toEqual([[firstTypingActivityKey, ['t-00002']]]); // THEN: Should have no active typing. expect(currentActiveTyping).toHaveProperty('u-00001', undefined); diff --git a/__tests__/html2/livestream/outOfOrder.html b/__tests__/html2/livestream/outOfOrder.html index 3063ce5222..87ed726c2a 100644 --- a/__tests__/html2/livestream/outOfOrder.html +++ b/__tests__/html2/livestream/outOfOrder.html @@ -247,7 +247,7 @@ // THEN: Should have 2 activity keys. expect(currentActivityKeysWithId).toEqual([ [firstActivityKey, ['a-00001']], - [secondActivityKey, ['t-00001', 't-00002', 't-00003', 'a-00002']] + [secondActivityKey, ['a-00002']] ]); // THEN: Should have no typing. diff --git a/__tests__/html2/livestream/outOfOrder.sequenceNumber.html b/__tests__/html2/livestream/outOfOrder.sequenceNumber.html index d31ed8d828..811a26816b 100644 --- a/__tests__/html2/livestream/outOfOrder.sequenceNumber.html +++ b/__tests__/html2/livestream/outOfOrder.sequenceNumber.html @@ -249,7 +249,7 @@ // THEN: Should have 2 activity keys. expect(currentActivityKeysWithId).toEqual([ [firstActivityKey, ['a-00001']], - [secondActivityKey, ['t-00001', 't-00002', 't-00003', 'a-00002']] + [secondActivityKey, ['a-00002']] ]); // THEN: Should have no typing. diff --git a/__tests__/html2/livestream/simultaneous.html b/__tests__/html2/livestream/simultaneous.html index 52c53e4488..59a6d55169 100644 --- a/__tests__/html2/livestream/simultaneous.html +++ b/__tests__/html2/livestream/simultaneous.html @@ -251,7 +251,7 @@ expect(currentActivityKeysWithId).toEqual([ [firstActivityKey, ['a-00001']], [thirdActivityKey, ['t-10001', 't-10002']], - [secondActivityKey, ['t-00001', 't-00002', 'a-00002']] + [secondActivityKey, ['a-00002']] ]); // --- @@ -289,8 +289,8 @@ // THEN: Should have 3 activity keys. expect(currentActivityKeysWithId).toEqual([ [firstActivityKey, ['a-00001']], - [secondActivityKey, ['t-00001', 't-00002', 'a-00002']], - [thirdActivityKey, ['t-10001', 't-10002', 'a-00003']] + [secondActivityKey, ['a-00002']], + [thirdActivityKey, ['a-00003']] ]); }); diff --git a/packages/api-graph/src/private/GraphProvider.tsx b/packages/api-graph/src/private/GraphProvider.tsx index fc8bb12357..7f23dc060e 100644 --- a/packages/api-graph/src/private/GraphProvider.tsx +++ b/packages/api-graph/src/private/GraphProvider.tsx @@ -76,17 +76,21 @@ function GraphProvider(props: GraphProviderProps) { }; }, [graph, setOrderedActivityNodes]); - const orderedActivitiesState = useMemo( - () => - Object.freeze([ - Object.freeze( - orderedActivityNodes.map( - node => node['urn:microsoft:webchat:direct-line-activity:raw-json'][0]['@value'] as WebChatActivity - ) - ) - ] as const), - [orderedActivityNodes] - ); + const orderedActivitiesState = useMemo(() => { + // Filter out stale graph nodes for activities no longer in Redux (e.g. pruned livestream revisions). + // The graph does not support deletion, so stale nodes linger after computeSortedActivities prunes them. + const { activities: storeActivities } = store.getState(); + const validActivitySet = new Set(storeActivities); + const activities: WebChatActivity[] = []; + + for (const node of orderedActivityNodes) { + const activity = node['urn:microsoft:webchat:direct-line-activity:raw-json'][0]['@value'] as WebChatActivity; + + validActivitySet.has(activity) && activities.push(activity); + } + + return Object.freeze([Object.freeze(activities)] as const); + }, [orderedActivityNodes, store]); const context = useMemo( () => diff --git a/packages/core/src/reducers/activities/sort/deleteActivityByLocalId.howToWithLivestream.spec.ts b/packages/core/src/reducers/activities/sort/deleteActivityByLocalId.howToWithLivestream.spec.ts index 153b5c6793..0619ed98ed 100644 --- a/packages/core/src/reducers/activities/sort/deleteActivityByLocalId.howToWithLivestream.spec.ts +++ b/packages/core/src/reducers/activities/sort/deleteActivityByLocalId.howToWithLivestream.spec.ts @@ -132,15 +132,15 @@ scenario('delete livestream activities in part grouping', bdd => { activity4 ) ) - .then('should have 4 activities', (_, state) => { + .then('should have 4 activities in map, 2 visible', (_, state) => { expect(state.activityMap).toHaveProperty('size', 4); expect(state.howToGroupingMap).toHaveProperty('size', 1); expect(state.livestreamSessionMap).toHaveProperty('size', 1); - expect(state.sortedActivities).toHaveLength(4); + expect(state.sortedActivities).toHaveLength(2); expect(state.sortedChatHistoryList).toHaveLength(1); }) .when('the last livestream activity is delete', (_, state) => - deleteActivityByLocalId(state, getLocalIdFromActivity(state.sortedActivities[2])) + deleteActivityByLocalId(state, getLocalIdFromActivity(state.sortedActivities[0])) ) .then('should have 3 activities', (_, state) => { expect(state.activityMap).toHaveProperty('size', 3); diff --git a/packages/core/src/reducers/activities/sort/deleteActivityByLocalId.livestream.spec.ts b/packages/core/src/reducers/activities/sort/deleteActivityByLocalId.livestream.spec.ts index 1bbbe0454a..111facd448 100644 --- a/packages/core/src/reducers/activities/sort/deleteActivityByLocalId.livestream.spec.ts +++ b/packages/core/src/reducers/activities/sort/deleteActivityByLocalId.livestream.spec.ts @@ -96,15 +96,15 @@ scenario('deleting an activity', bdd => { .when('3 activities are upserted', state => upsert({ Date }, upsert({ Date }, upsert({ Date }, state, activity1), activity2), activity3) ) - .then('should have 3 activities', (_, state) => { + .then('should have 3 activities in map, 1 visible', (_, state) => { expect(state.activityMap).toHaveProperty('size', 3); expect(state.howToGroupingMap).toHaveProperty('size', 0); expect(state.livestreamSessionMap).toHaveProperty('size', 1); - expect(state.sortedActivities).toHaveLength(3); + expect(state.sortedActivities).toHaveLength(1); expect(state.sortedChatHistoryList).toHaveLength(1); }) .when('the last activity is delete', (_, state) => - deleteActivityByLocalId(state, getLocalIdFromActivity(state.sortedActivities[2])) + deleteActivityByLocalId(state, getLocalIdFromActivity(state.sortedActivities[0])) ) .then('should have 2 activities', (_, state) => { expect(state.activityMap).toHaveProperty('size', 2); diff --git a/packages/core/src/reducers/activities/sort/private/computeSortedActivities.ts b/packages/core/src/reducers/activities/sort/private/computeSortedActivities.ts index 07c3d1f9e7..1413f930a1 100644 --- a/packages/core/src/reducers/activities/sort/private/computeSortedActivities.ts +++ b/packages/core/src/reducers/activities/sort/private/computeSortedActivities.ts @@ -1,4 +1,21 @@ -import type { Activity, State } from '../types'; +import type { Activity, LivestreamSessionMap, State } from '../types'; + +function* yieldSessionActivities( + session: NonNullable>, + activityMap: State['activityMap'] +): Generator { + if (session.finalized) { + // After finalization, only yield the final revision — intermediate revisions are pruned. + // eslint-disable-next-line no-magic-numbers + const lastEntry = session.activities.at(-1); + + lastEntry && (yield activityMap.get(lastEntry.activityLocalId)!.activity); + } else { + for (const activityEntry of session.activities) { + yield activityMap.get(activityEntry.activityLocalId)!.activity; + } + } +} export default function computeSortedActivities( temporalState: Pick @@ -20,17 +37,13 @@ export default function computeSortedActivities( } else { howToPartEntry.type satisfies 'livestream session'; - for (const activityEntry of livestreamSessionMap.get(howToPartEntry.livestreamSessionId)!.activities) { - yield activityMap.get(activityEntry.activityLocalId)!.activity; - } + yield* yieldSessionActivities(livestreamSessionMap.get(howToPartEntry.livestreamSessionId)!, activityMap); } } } else { sortedEntry.type satisfies 'livestream session'; - for (const activityEntry of livestreamSessionMap.get(sortedEntry.livestreamSessionId)!.activities) { - yield activityMap.get(activityEntry.activityLocalId)!.activity; - } + yield* yieldSessionActivities(livestreamSessionMap.get(sortedEntry.livestreamSessionId)!, activityMap); } } })() diff --git a/packages/core/src/reducers/activities/sort/upsert.howToWithLivestream.spec.ts b/packages/core/src/reducers/activities/sort/upsert.howToWithLivestream.spec.ts index e77c6724e9..e8a20641d1 100644 --- a/packages/core/src/reducers/activities/sort/upsert.howToWithLivestream.spec.ts +++ b/packages/core/src/reducers/activities/sort/upsert.howToWithLivestream.spec.ts @@ -379,10 +379,6 @@ scenario('upserting plain activity in the same grouping', bdd => { ] satisfies SortedChatHistory); }) .and('`sortedActivities` should match', (_, state) => { - expect(state.sortedActivities).toEqual([ - activityToExpectation(activity1, 1_000), - activityToExpectation(activity2, 2_000), - activityToExpectation(activity3, 3_000) - ]); + expect(state.sortedActivities).toEqual([activityToExpectation(activity3, 1_000)]); }); }); diff --git a/packages/core/src/reducers/activities/sort/upsert.livestream.spec.ts b/packages/core/src/reducers/activities/sort/upsert.livestream.spec.ts index a0a864713e..9d9cc9524b 100644 --- a/packages/core/src/reducers/activities/sort/upsert.livestream.spec.ts +++ b/packages/core/src/reducers/activities/sort/upsert.livestream.spec.ts @@ -413,11 +413,6 @@ scenario('upserting a livestream session', bdd => { ]); }) .and('`sortedActivities` should match snapshot', (_, state) => { - expect(state.sortedActivities).toEqual([ - activityToExpectation(activity1, 1_000), - activityToExpectation(activity3, 1_001), - activityToExpectation(activity2, 2_000), - activityToExpectation(activity4, 3_000) - ]); + expect(state.sortedActivities).toEqual([activityToExpectation(activity4, 1_000)]); }); });