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 👍 / 👎.
This branch has not been deployed
1 participant
Summary
Adds props-only summary views for initiating agents, receiving agents and post-call use, with editing, copy, feedback, retry, accessibility and focus recovery. Content uses the SDK
AISummaryshape exposed by the store; revision-bound callbacks delegate changes and tracking to store actions.Dependency and review
Stacked on PR #748, whose store contract is fixed. Review the current heads in the component-only comparison.
PR #750 supplies the widget and CallControl integration. Its consumed prop and callback names are preserved; it needs to pick up this updated component head.
Verification
Under Node 24:
yarn install --immutableyarn build:devyarn workspace @webex/cc-components build:srcyarn workspace @webex/cc-components test:unit --runInBand --silent: 58 suites passed; 1,016 tests passed, 4 skipped; 234 snapshots passed.yarn workspace @webex/cc-components test:styles: passed with no warnings or errors.git diff --check