Repository navigation
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bf6d1fd691
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const successor = successorRef.current; | ||
| if (successor?.isConnected) { | ||
| successor.focus(); | ||
| clearFocusSnapshot(); |
There was a problem hiding this comment.
When controlsDisabled or requestPending becomes true while a structured editor has focus, SummarySectionEditor replaces the textarea and all saved successor controls become disabled but remain connected. This branch calls focus() on that disabled node, then clears the snapshot without checking whether focus moved, so focus is left on the document body instead of the containing panel. Revalidate the successor's disabled/aria-disabled/tab-index state (or the resulting active element) before accepting it and otherwise use the panel fallback.
Useful? React with 👍 / 👎.
| `resolution`; the display labels never become SDK section keys. Receiver summaries | ||
| use copy and feedback actions around a display-only Adaptive Card. | ||
|
|
||
| CallControl's optional `aiSummary` prop contains the views and request callbacks. |
There was a problem hiding this comment.
This new canonical section states that CallControl has an optional aiSummary prop, but a repo-wide production-source search finds no such prop, no WrapUpSummary, and no AISummary use outside the new internal component and AIAssistant; this commit also changes no CallControl source or tests. The spec therefore advertises consult/transfer and wrap-up behavior that does not exist in the reviewed tree, so either include that integration or record it as a future gap instead of current behavior.
AGENTS.md reference: AGENTS.md:L68-L68
Useful? React with 👍 / 👎.
Use SDK summary types directly and send only changed structured sections. Guard superseded request results, pair task listener cleanup, and require the original task for post-call submission. Repair the lockfile for immutable installs.
|
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: 762dcba033
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const receiverAdaptiveCard = | ||
| receiverSummary?.surface === 'content' ? receiverSummary.content.adaptiveCard : undefined; | ||
| const receiverDisplayCard = useMemo( | ||
| () => (receiverAdaptiveCard === undefined ? undefined : projectReceiverDisplayCard(receiverAdaptiveCard)), |
There was a problem hiding this comment.
When receiverSummary.surface is content but the SDK payload contains only summaryText/sections (a shape allowed by AIAssistantReceiverSummary.content and explicitly retained by the new typedOnlyUnsupported store fixture/test), this expression leaves receiverDisplayCard undefined. AdaptiveCardRendererBody's render effect exits for a falsy card, so the mutation observer never reaches ready; View summary opens an empty branch without fallback text or copy/feedback controls. Render the text/sections or map this case to unavailable instead.
Useful? React with 👍 / 👎.
| setCopied(false); | ||
| }} | ||
| onClick={() => void handleCopy()} | ||
| aria-label={AI_SUMMARY_MESSAGES.copySummary} |
There was a problem hiding this comment.
After a successful clipboard write, copied changes the icon and screen-reader-only child to Copied, but this explicit label remains Copy Summary. Because aria-label continues to define the focused button's accessible name and there is no live status, screen-reader users receive no success confirmation during the 1.5-second copied state. Expose copyLabel in the accessible name or announce the state separately.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1d6eb17ce5
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| <SummaryEditor | ||
| className="ai-summary__textarea" | ||
| dir="auto" | ||
| value={props.content.summaryText ?? ''} |
There was a problem hiding this comment.
When a mid-call-initiator or post-call entry contains an SDK adaptiveCard but no sections or summaryText—the shape exercised by the cardOnlyUnsupported fixtures and retained as available by the store tests—this fallback feeds '' to the only editor and never renders the card. Consumers therefore show a blank summary with feedback actions, while Copy is a no-op; render the card or map this shape to unavailable instead of treating it as empty plain text.
Useful? React with 👍 / 👎.
| const receiverActionsVisible = Boolean( | ||
| showReceiverSummary && receiverSummary?.surface === 'content' && receiverCardReady | ||
| ); |
There was a problem hiding this comment.
When an open receiver branch gets replacement content under the same branchKey, receiverCardReady remains true from the previous card while AdaptiveCardRendererBody does not clear and replace its imperative DOM until its passive effect runs. During that interval the old rendered text stays actionable but the action callbacks and revision already belong to the replacement, so Copy can copy old text while counting the new revision and feedback can rate a summary that has not rendered; track readiness by card/revision or disable actions until the matching renderer reports success.
Useful? React with 👍 / 👎.
| const renderStatus = () => { | ||
| if (props.state === 'generating') { | ||
| return ( | ||
| <div className="ai-summary__status" data-testid="ai-summary:generating"> |
There was a problem hiding this comment.
When a summary request changes state while focus remains on the containing panel or an external control, the generating, unavailable, and error branches are plain div elements with no status/alert role or live region, so screen readers do not announce that the request is waiting or failed. Mark the appropriate status containers live so these asynchronous transitions, especially the failure that introduces Retry, are conveyed without requiring users to rediscover the changed content.
Useful? React with 👍 / 👎.
| border: 0.1875rem solid var(--mds-color-theme-outline-secondary-normal, #d7d7d8); | ||
| border-block-start-color: var(--mds-color-theme-text-primary-normal, #1b1c1d); | ||
| border-radius: 50%; | ||
| animation: ai-summary-spin 1s linear infinite; |
There was a problem hiding this comment.
When the OS has prefers-reduced-motion: reduce and a summary is generating, this unconditional infinite rotation still runs because the new stylesheet provides no reduced-motion override. The adjacent text already conveys progress, so suppress or replace the animation under the reduced-motion media query to respect users who disable motion.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 28135ae7eb
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| {!showWellnessContent ? ( | ||
| showReceiverSummary ? ( | ||
| renderReceiverSummary() |
There was a problem hiding this comment.
When a receiver summary coexists with wellness history, an offer, or another wellness status (which can happen while an active task blocks a break), showWellnessContent stays true and this conditional never renders the selected receiver branch. Closed/minimized chrome still offers “View summary,” but clicking it only opens the wellness body, while open chrome has no summary trigger; persistent wellness history therefore makes the handoff summary inaccessible until the agent clears that history. Let an explicit receiver selection take precedence over non-modal wellness content.
Useful? React with 👍 / 👎.
| ...this.store.aiSummaries[interactionId], | ||
| [role]: { | ||
| ...entry, | ||
| response: {...entry.response, numberOfTimesCopied: entry.response.numberOfTimesCopied + 1}, |
There was a problem hiding this comment.
When a receiving agent copies a summary but does not also select feedback, this increment remains only in the in-memory entry. A repo-wide search shows the only receiver response send is in setReceiverSummaryFeedback; sendMidCallSummaryResponse reads the initiator entry, and task removal deletes this receiver record. Consequently copy-only interactions never send numberOfTimesCopied to the SDK and silently undercount receiver engagement, so flush the receiver response on copy or at a guaranteed receiver lifecycle boundary.
Useful? React with 👍 / 👎.
| }> = ({section, disabled, onChange, onEditorBlur}) => { | ||
| const [editing, setEditing] = useState(false); | ||
| const label = AI_SUMMARY_MESSAGES.sectionLabels[section.key]; | ||
| if (editing && !disabled) { |
There was a problem hiding this comment.
When controlsDisabled or requestPending becomes true while a structured textarea is active, this condition replaces the textarea but leaves the component's editing state true; removing a focused element does not run the textarea's React onBlur handler. If the request then completes and controls are re-enabled, the textarea remounts with autoFocus and steals focus from the fallback or external control the agent moved to, so clear edit mode on the disabled transition instead of retaining hidden editing state.
Useful? React with 👍 / 👎.
Kesari3008
left a comment
Kesari3008
left a comment
There was a problem hiding this comment.
Test files are remaining and few code changes needed to be discussed in person. Would be good to schedule a sit down review to understand the complete structure/flow on high level
| .ai-summary__feedback-group { | ||
| display: inline-flex; | ||
| align-items: center; | ||
| gap: 4px; |
There was a problem hiding this comment.
Updated the feedback control dimensions to use rem, including the remaining 4px gap.
|
|
||
| export type AIAssistantReceiverSummary = | ||
| | { | ||
| surface: 'content'; |
There was a problem hiding this comment.
why do we need this property ? Won't the response that we get from SdK should be enough to figure out where we received error or empty content or actual summary data
There was a problem hiding this comment.
I kept the small surface discriminator because the SDK payload alone doesn’t express the presentation lifecycle, especially the generic-error state. It lets the component distinguish content, unavailable, and generic error without adding behavior to the SDK.
| export type AIAssistantReceiverSummary = | ||
| | { | ||
| surface: 'content'; | ||
| branchKey: string; |
There was a problem hiding this comment.
What does this property stand for ?
There was a problem hiding this comment.
branchKey identifies which interaction the summary belongs to. The assistant uses it to avoid showing a summary from the previous interaction after the active interaction changes; I added a comment to clarify that.
| showReceiverSummary ? ( | ||
| renderReceiverSummary() | ||
| ) : showLanding ? ( | ||
| <AIAssistantLanding agentName={agentName} showRealTimeAssist={isFeatureEnabled} /> |
There was a problem hiding this comment.
Why was this change needed to move the rendering here?
There was a problem hiding this comment.
I moved the existing landing and RealTimeAssist rendering back to its original position. The receiver-summary branch now uses that same content area, so its addition doesn’t reorder the existing assistant content.
|
|
||
| const renderReceiverSummaryTrigger = (classNameSuffix = '') => | ||
| canViewReceiverSummary ? ( | ||
| <button |
There was a problem hiding this comment.
This button should be momentum component and also what is the difference between this button and button rendered as part of renderLauncherButton
There was a problem hiding this comment.
Changed the View summary control to Momentum’s Button. It opens the receiver summary view; the launcher opens the assistant panel. They have different jobs, even though both are buttons.
| lastFocusSuccessorRef.current = index >= 0 ? (controls[index + 1] ?? null) : null; | ||
| }; | ||
|
|
||
| const openRealTimeAssist = () => { |
There was a problem hiding this comment.
What are these methods for and why this change needed for Real time assist ?
There was a problem hiding this comment.
I checked the Agent Desktop behavior. These handlers preserve focus while switching between RealTimeAssist and the receiver-summary branch, which conditionally replaces the panel body. I kept that focus handling scoped to summary navigation.
There was a problem hiding this comment.
The whole AI Summary folder should be present within the AI Assistant folder as we would be placing all AI assistant feature inside it. Also it would seem more appropriate if we call it AI_ASSISTANT_Summary instead of just AI_SUMMARY
There was a problem hiding this comment.
Moved the summary folder under AIAssistant and renamed its constants with the AI_ASSISTANT_SUMMARY_ prefix.
| {feedback: 'thumbs_down', icon: 'dislike', label: AI_SUMMARY_MESSAGES.dislike}, | ||
| ] as const; | ||
|
|
||
| type AISummaryTooltipControl = 'copy' | Exclude<AISummaryFeedback, 'none'>; |
There was a problem hiding this comment.
These constants and types can be moved to their respective files
There was a problem hiding this comment.
Moved the presentation types and constants into their own files, so the component file no longer defines them inline.
| ); | ||
| } | ||
| return ( | ||
| <button |
There was a problem hiding this comment.
Replaced the structured edit-preview native button with Momentum’s Button.
| ); | ||
| }; | ||
|
|
||
| const AISummary = (props: AISummaryProps): React.ReactElement => { |
There was a problem hiding this comment.
This file seems to have presentational content as well as helper logic(handler methods) together. Structure that we are following in widgets is the we separate them out in 2 different files. Please segragate the 2
There was a problem hiding this comment.
Moved the interaction handlers and summary helper logic into ai-summary.hooks.ts; ai-summary.tsx now owns the rendered UI.
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
…mponents # Conflicts: # ai-docs/CONTRACTS.md # packages/contact-center/store/ai-docs/store-spec.md # packages/contact-center/store/src/store.types.ts # packages/contact-center/store/src/storeEventsWrapper.ts # packages/contact-center/store/tests/storeEventsWrapper.ts # packages/contact-center/task/ai-docs/task-spec.md
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6ff55747fa
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| fireEvent.click(screen.getByTestId('ai-assistant:header-close')); | ||
| expect(props.close).toHaveBeenCalledTimes(1); | ||
| expect(screen.getByTestId('ai-assistant:empty')).toBeInTheDocument(); |
There was a problem hiding this comment.
When the newly added receiver branch is open, clicking the header action only invokes close and never clears viewingReceiverKey; because this case keeps the controlled chrome prop open, the receiver summary remains mounted and this new assertion fails deterministically. The stale selection also resurfaces if a host later reopens the chrome without going through the launcher, so clear the receiver selection in the close path before invoking the host callback.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 33e603257a
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| flex-direction: column; | ||
| gap: 0.75rem; | ||
| min-inline-size: 0; | ||
| overflow: visible; |
There was a problem hiding this comment.
When a receiver Adaptive Card is taller than the remaining height of the fixed 32rem assistant panel, overflow: visible here cannot create a scroll container and the ancestor .ai-assistant__body uses overflow: hidden. The lower card content—and the copy/feedback row rendered after it—is therefore clipped with no way to reach it; make this branch consume the available flex height and scroll vertically, as .ai-assistant__chat does.
Useful? React with 👍 / 👎.
| if (text.trim().length === 0) { | ||
| return; | ||
| } | ||
| await navigator.clipboard.writeText(text); |
There was a problem hiding this comment.
When the widget is embedded in a non-secure context or a browser without the async Clipboard API, navigator.clipboard is undefined, so this throws into the catch and every new summary Copy button silently does nothing (and never records the copy). The existing copySuggestion path in adaptive-card-renderer.utils.ts explicitly falls back to a hidden textarea for this same environment; use an equivalent fallback here and only report success after it succeeds.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c9e44577fd
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (succeeded !== false && wrapupCompletionRef.current === completion) { | ||
| setSelectedWrapupReason(null); | ||
| setSelectedWrapupId(null); | ||
| } |
There was a problem hiding this comment.
When the new summary wrap-up branch is wired through the repository's production useCallControl adapter and currentTask.wrapup() rejects, that adapter catches the rejection in packages/contact-center/task/src/helper.ts:1269-1271 and its Promise<void> resolves to undefined; this succeeded !== false check therefore treats the failed completion as success and clears selectedWrapupReason/selectedWrapupId. The task remains in wrap-up but Complete Wrap-Up becomes disabled until the agent selects a reason again, so propagate/return the failure from the adapter or clear the selection only on an explicit success.
Useful? React with 👍 / 👎.
| if (props.state !== 'content' || props.mode === 'mid-call-receiver' || !displayContent) { | ||
| return null; | ||
| } | ||
| if (!displayContent.sections) { |
There was a problem hiding this comment.
When an SDK payload contains a nonempty summaryText together with sections: {} (or an undefined-only sections object), this truthiness check selects structured rendering, while getDisplaySections() returns no rows and flattenContentForCopy() also ignores summaryText. The content surface consequently shows only the action chrome and Copy is a no-op despite usable text being present; choose the structured branch only when it has displayable sections, otherwise render and copy summaryText.
Useful? React with 👍 / 👎.
| const focused = focusedControlRef.current; | ||
| if (!focused || focused.isConnected) { | ||
| return; |
There was a problem hiding this comment.
When requestPending becomes true after a receiver feedback click, or controlsDisabled changes while the plain editor is focused, the native control remains connected but becomes disabled and browsers can move focus to the document body. This guard returns solely because the saved control is still connected, so neither the successor nor the containing-panel fallback is focused; treat a connected-but-disabled saved control as requiring the same repair as a removed one.
Useful? React with 👍 / 👎.
This branch has not been deployed
2 participants
Summary
Adds the presentation for all three AI summary flows: the initiating agent's consult/transfer summary, the receiving agent's summary, and the post-call wrap-up summary. CallControl owns the destination/reason controls and composes the shared internal AISummary view. Without summary props, the existing CallControl presentation remains available.
The components receive raw SDK content and local
editedSummaryvalues separately. Copy, edit, feedback, request and retry callbacks carry no numeric revision; widget helpers bind them to the displayed SDK payload. The components make no SDK calls. Focus recovery and pending completion guards are limited to summary controls, including capability activation/revocation in an open popover.Dependencies and review
The store dependency #748 is merged. #750 provides widget/store wiring and consumes this exact component tree. Consult/transfer and wrap-up presentation previously present only in #750 is now included here so that component ownership is complete.
The merged store's summary observability gap remains a separate integration blocker; no store files are changed here.
Verification
Under Node 24:
yarn workspace @webex/cc-components build:srcpassed.yarn workspace @webex/cc-components test:stylesandgit diff --checkpassed.