feat(ccwidgets): merge rtt with next#666
Conversation
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f75a3c08dc
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| return transcriptLines.map((line, index) => { | ||
| const payload: RealtimeTranscriptionData = { | ||
| messageId: line.messageId, | ||
| conversationId: line.conversationId, | ||
| role: line.role, |
There was a problem hiding this comment.
Filter realtime transcript lines by current task
useRealtimeTranscript receives currentTaskId but maps every item in realtimeTranscriptLines without task/conversation filtering. Since the store subscribes transcription events on each task, multi-task sessions will show utterances from other interactions in the currently selected task transcript, which can leak cross-call content and corrupt the UI. Filter by the selected task (or store a task identifier on each line) before mapping.
Useful? React with 👍 / 👎.
| aria-selected={activeTab === 'live'} | ||
| className={`real-time-transcript__tab ${activeTab === 'live' ? 'real-time-transcript__tab--active' : ''}`.trim()} | ||
| onClick={() => onTabChange?.('live')} | ||
| data-testid="real-time-transcript:live-tab" | ||
| > |
There was a problem hiding this comment.
Add an IVR tab trigger to the transcript tablist
The component models two tabs ('ivr' | 'live') and has separate IVR rendering, but the tablist renders only a Live tab and its handler only emits 'live'. In the default state, users have no in-component way to switch to IVR content, so ivrTranscript is effectively inaccessible unless a parent forces activeTab='ivr' externally.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5c933ce8eb
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| CallControlCAD, | ||
| store, | ||
| OutdialCall, | ||
| RealtimeTranscript, |
There was a problem hiding this comment.
Import RealTimeTranscript using the exported symbol name
@webex/cc-widgets now exports the widget as RealTimeTranscript, but this sample imports RealtimeTranscript (lowercase t in Time). In environments that type-check or validate ESM named imports, this causes a build/runtime failure (RealtimeTranscript is undefined), so the new sample toggle cannot render the transcript widget.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
I went back to SDK PR#4794 to realize that the transcripts are being started always based on certain events:
export const TRANSCRIPT_EVENT_MAP = {
[CC_EVENTS.AGENT_CONTACT_ASSIGNED]: 'START',
[CC_EVENTS.AGENT_CONSULTING]: 'START',
[CC_EVENTS.AGENT_CONSULT_CONFERENCED]: 'START',
[CC_EVENTS.AGENT_WRAPUP]: 'STOP',
[CC_EVENTS.AGENT_CONSULT_ENDED]: 'STOP',
[CC_EVENTS.PARTICIPANT_LEFT_CONFERENCE]: 'STOP',
};
However, what if the developer doesn't want to enable transcript from SDK or in this case, the RTT widget isn't included by the developer?
There was a problem hiding this comment.
Shouldn't it still be "transcripts" because it is not one transcript you get?
There was a problem hiding this comment.
feature says real time transcript and everywhere we are keeping without s so to make it consistent I changed
There was a problem hiding this comment.
When you say, "feature says", where is this?
I was trying to find the help doc and the help doc says real time transcripts in plural?
There was a problem hiding this comment.
I checked in AgentDesktop Code and contact center announcements for this feature. Let me know if you still want it to be changed.
There was a problem hiding this comment.
Can we remove this file change? Looks like they're only quotes and spacing change that's unnecessary
| <section className={`real-time-transcript ${className || ''}`.trim()} data-testid="real-time-transcript:root"> | ||
| <div className="real-time-transcript__content" data-testid="real-time-transcript:live-content"> | ||
| {sortedEntries.length === 0 ? ( | ||
| <div className="real-time-transcript__empty">No live transcript available.</div> |
There was a problem hiding this comment.
Should this be behind a constant so that we can do i18n later?
| const normalizedRealtimeData = { | ||
| ...data, | ||
| role, | ||
| caller, | ||
| content: (payload?.content || data.content || '').trim(), | ||
| publishTimestamp: Number.isNaN(publishTimestamp) ? Date.now() : publishTimestamp, | ||
| }; |
There was a problem hiding this comment.
Couldn't we have encapsulated this normalization in the SDK itself so that other SDK users also benefitted from it?
We could have the raw data but also the normalized one?
Or better, we handle this whole merging of transcripts at the SDK itself and just send the new list of transcripts to Widgets?
There was a problem hiding this comment.
This is to be able to provide control to the consumers. As of now we do not know how each consumer would like to handle these event data so that's why I have kept it in widgets.
AGENT_CONTACT_ASSIGNED is an event that happens for any incoming call and for consult offer, it is going to be CONSULTING so transcriptions are basically supposed to start for any incoming accepted task. RealTime transcription enablement is actually dependent on if the feature is enabled from control hub setting |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a8fb0ad9e9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const transcriptData = payload.data; | ||
| if (!transcriptData?.messageId) return; |
There was a problem hiding this comment.
Normalize transcription payload before reading messageId
handleRealtimeTranscription assumes the SDK callback shape is always { data: ... } and immediately reads payload.data, so any realtime event emitted as a direct transcript object is treated as missing messageId and ignored. In that environment, the widget never renders live transcript lines even though events are arriving. This mismatch is also reflected by the new test case in this commit that sends a direct payload and expects it to be accepted.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3898f6db3d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const newTranscriptData = { | ||
| ...transcriptData, | ||
| role, | ||
| content, | ||
| publishTimestamp: normalizedPublishTimestamp, |
There was a problem hiding this comment.
Persist actionEvent when storing transcript updates
handleRealtimeTranscription currently stores only payload.data, so notifDetails.actionEvent is discarded before data reaches useRealTimeTranscript. In sessions where the SDK emits action/tombstone transcript updates, those events never appear in the widget even though RealTimeTranscriptEntry and the component support rendering event rows, resulting in missing live transcript context.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cc968f31b0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const removedTaskId = taskToRemove.data?.interactionId; | ||
| if (removedTaskId && this.store.currentTask?.data?.interactionId === removedTaskId) { | ||
| this.store.realtimeTranscriptionData = []; |
There was a problem hiding this comment.
Preserve transcripts for remaining active tasks
Clearing this.store.realtimeTranscriptionData when the removed task is the current task wipes transcript history for all tasks, not just the ended one. In multi-task sessions, transcription listeners are registered per task, so another active interaction can already have lines in this shared array; ending the current task will erase those lines and the remaining task transcript appears empty until new chunks arrive. Remove only the ended task's entries (or key storage by task/conversation) instead of resetting the whole array.
Useful? React with 👍 / 👎.
| if (taskToRemove) { | ||
| const removedTaskId = taskToRemove.data?.interactionId; | ||
| if (removedTaskId && this.store.currentTask?.data?.interactionId === removedTaskId) { | ||
| this.store.realtimeTranscriptionData = []; |
There was a problem hiding this comment.
This should be wrapped in runInAction
There was a problem hiding this comment.
When you say, "feature says", where is this?
I was trying to find the help doc and the help doc says real time transcripts in plural?
There was a problem hiding this comment.
Why are we extending outdial call tests as part of this PR?
There was a problem hiding this comment.
These tests were failing in the pipeline and in my local as well, not sure why that was happening because there is no change in outdial code
There was a problem hiding this comment.
That's strange. What if we take a fresh clone of next branch? Do we see the same error there?
There was a problem hiding this comment.
Yes I did pull fresh next branch before adding this and it was still failing for me yesterday. I found it strange too. I can try it again today and if it is not needed, I will remove it
mkesavan13
left a comment
There was a problem hiding this comment.
While the outdial flakyness was a mystery, the changes still are good improvements. So approving the PR.
Thank you for your contribution! |
COMPLETES #https://jira-eng-sjc12.cisco.com/jira/browse/CAI-7783
This pull request addresses
Merging the feature branch created for rtt with next
by making the following changes
Merging the feature branch created for rtt with next
Change Type
The following scenarios were tested
https://app.vidcast.io/share/06138e83-f481-4598-81ef-4f72c4ea04c3
Have test conference and consult flows in Agent desktop and in Widgets, behavior is consistent:
The GAI Coding Policy And Copyright Annotation Best Practices
Checklist before merging
Make sure to have followed the contributing guidelines before submitting.