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
|
|
||
| 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.
| 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.
|
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 👍 / 👎.
c9e4457 to
a21a8b2
Compare
github-actions
Bot
removed
the
validated
3c579ce to
9ac15fb
Compare
|
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". |
9ac15fb to
b019afc
Compare
b019afc to
c35ee00
Compare
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. Edit callbacks do not require a return value. A missing editable summary payload renders as unavailable and exposes no copy or feedback actions. 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. Each CallControl panel owns its constrained layout styles.The store's Webpack build emits the declarations required by downstream workspace imports. A clean build previously omitted these files, widening SDK-derived summary section keys to
string | number | symboland failing the component build. The React sample pins the same SDK version as the store to avoid incompatible class identities during type checking. The store module spec documents both requirements.Fixes overlapping structured summary text in consult/transfer and post-call views. Summary previews now size the Momentum button host automatically and wrap through its exposed
button-textpart. Paragraphs and bullet lists reserve their full height; disabled content stays readable, and keyboard activation/focus still opens and exits the keyed editor. The shared summary styles and module spec describe this behavior.Empty or whitespace-only structured fields now render no label or row in consult/transfer and post-call summaries. A focused editor stays mounted when cleared, and its blank row disappears after editing ends. Raw SDK values and cleared local response keys are preserved; clipboard formatting already omits blank sections.
Dependencies and review
The store dependency #748 is merged. #750 provides widget/store wiring and includes this branch. Consult/transfer and wrap-up presentation is owned here.
The store summary observability fix is included in #750. This branch owns the summary presentation and store declaration build configuration.
Verification
Under Node 24.17.0 and Yarn 4.5.1:
yarn workspace @webex/cc-components test:unit --runInBand --silentpassed: 59 suites, 1,077 tests, and 234 snapshots; 4 existing tests remain skipped. The suite includes regression coverage for missing initiator and post-call summary content.yarn workspace @webex/cc-components build:srcpassed.yarn workspace @webex/cc-components test:stylespassed.yarn run buildpassed.git diff --checkpassed.Browser inspection used synthetic summary props in a local preview. Authenticated call flows were not replayed.