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: 57dfea3335
ℹ️ 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".
| 'on-recording-toggle', | ||
| 'conference-enabled', | ||
| ]); | ||
| expectObservedAttributes(AI_ASSISTANT_TAG, [ |
There was a problem hiding this comment.
In the cc-widgets Jest/jsdom suite, WebAIAssistant still declares seven wellness props in src/wc.ts (onWellnessBreakOffered through wellnessBreakOverlayTarget), and r2wc includes each declared prop in observedAttributes. This newly added exact-equality assertion omits all of them, so the cc-widgets unit suite has a guaranteed failure; include the existing dashed wellness attributes or avoid asserting the unrelated element here.
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). |
|
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". |
|
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". |
|
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". |
| selectedFeedback: entry.response.feedback, | ||
| feedbackPending: Boolean(entry.feedbackPending), | ||
| recordReceiverSummaryCopied: (expectedRevision) => | ||
| store.recordAISummaryCopied('receiver', expectedRevision, task), |
There was a problem hiding this comment.
Methods which we are invoking from store can just be placed in this file itself if they have no dependency on SDK
| /** The receiving agent's consult/transfer summary, pushed by the backend for the task's interaction. */ | ||
| export const getReceiverSummary = (task?: ITask): AIAssistantReceiverSummary | undefined => { | ||
| const interactionId = task?.data.interactionId; | ||
| const entry = interactionId ? store.aiSummaries[interactionId]?.receiver : undefined; |
There was a problem hiding this comment.
nitpick: Use more specific name which at least helps us identify that this entry object is nothing but actually summary data something like summaryData or summary itself
| entryPoint: CATEGORY_ENTRY_POINT, | ||
| } as const; | ||
|
|
||
| const VOICE_DESTINATION_CATEGORY_LABELS: Record<CategoryType, string> = { |
There was a problem hiding this comment.
What's the difference between this constant and one present right above: DESTINATION_CATEGORY. Also it should be on constants file
| 'Entry Point': 'Entry point', | ||
| }; | ||
|
|
||
| const useReactId = (React as typeof React & {useId: () => string}).useId; |
There was a problem hiding this comment.
Check if this would work for React versions lesser than React 18. Also if we are doing this for generating a unique id, would be good to check how else we are doing it in other places and keep the common approach
| {items.length === 0 && | ||
| (isVoiceDestinationLayout ? ( | ||
| <li> | ||
| <Text tagName="small" type="body-secondary"> |
There was a problem hiding this comment.
Why do we need Text component twice, one time as part of list and otherwise directly? Is there a common way to fix that
| import type {WrapUpSummaryProps} from '../../task.types'; | ||
| import './wrap-up-summary.styles.scss'; | ||
|
|
||
| const useReactId = (React as typeof React & {useId: () => string}).useId; |
| const normalizedQuery = query.trim().toLocaleLowerCase(); | ||
| const filteredReasons = reasons.filter((reason) => reason.name.toLocaleLowerCase().includes(normalizedQuery)); | ||
|
|
||
| useLayoutEffect(() => { |
There was a problem hiding this comment.
I see in multiple files now that we have introduced this new hook. I would like to evaluate if we really need this hook as this is generally introduced when we want to update something in our DOM before browser renders it. Using this might slow down the UI a bit so use only if absolutely necessary. If code/logic inside could also be addressed within useEffect then we can avoid use of this
| }; | ||
|
|
||
| // Freeze summary edits and prevent a second completion while its wrap-up is pending. | ||
| const runWrapupCompletion = async (wrapupReason: string | null, wrapupId: string | null) => { |
There was a problem hiding this comment.
Can this be in helper file instead of here, please check
| if (!onAISummaryStatusChange) { | ||
| return undefined; | ||
| } | ||
| return store.onAISummaryStatusChange(onAISummaryStatusChange); |
There was a problem hiding this comment.
Same comment here, see if really need this method in store layer else it can be present in helper file directly
…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
…dget-integration # 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/src/helper.ts # packages/contact-center/task/tests/helper.ts # packages/contact-center/test-fixtures/ai-docs/test-fixtures-spec.md # packages/contact-center/test-fixtures/src/aiSummaryFixtures.ts # packages/contact-center/test-fixtures/src/fixtures.ts # yarn.lock
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 201d73853e
ℹ️ 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".
| /** The receiving agent's consult/transfer summary, pushed by the backend for the task's interaction. */ | ||
| export const getReceiverSummary = (task?: ITask): AIAssistantReceiverSummary | undefined => { | ||
| const interactionId = task?.data.interactionId; | ||
| const entry = interactionId ? store.aiSummaries[interactionId]?.receiver : undefined; |
There was a problem hiding this comment.
Whenever AI Assistant has a current task, this dereferences store.aiSummaries, but the merged StoreWrapper has no top-level aiSummaries getter; summaries live at store.cc.aiSummaries. getReceiverSummary therefore throws before it can determine whether a receiver entry exists, and the outer error boundary renders an empty fallback, removing Real-time Assist and wellness content too. Port this adapter to the current map and entry/callback signatures.
AGENTS.md reference: AGENTS.md:L55-L57
Useful? React with 👍 / 👎.
| if (!onAISummaryStatusChange) { | ||
| return undefined; | ||
| } | ||
| return store.onAISummaryStatusChange(onAISummaryStatusChange); |
There was a problem hiding this comment.
When any host supplies the new callback (the changed React sample does whenever CallControl is mounted), the merged StoreWrapper has no onAISummaryStatusChange method or status-listener registry, so this effect invokes undefined. Because the effect belongs to the component outside the nested ErrorBoundary, mounting that public callback can unmount the consumer tree; implement the store subscription or do not expose the callback until it exists.
AGENTS.md reference: AGENTS.md:L55-L57
Useful? React with 👍 / 👎.
| return undefined; | ||
| } | ||
|
|
||
| const entries = store.aiSummaries[currentTask.data.interactionId]; |
There was a problem hiding this comment.
When either AI-summary capability is enabled, this function passes its initial guard and dereferences the absent top-level store.aiSummaries map (the merged store exposes store.cc.aiSummaries and also no longer provides the revision/edit/copy helpers used below). Since CallControlInternal evaluates this adapter during render, the enclosing boundary falls back to an empty fragment and removes all call controls for exactly the tasks eligible for summaries; align this block with the current SDK-backed response contract.
AGENTS.md reference: AGENTS.md:L55-L57
Useful? React with 👍 / 👎.
| "@webex/cc-ai-assistant": "workspace:*", | ||
| "@webex/cc-widgets": "workspace:*", | ||
| "@webex/contact-center": "3.12.0-next.133", | ||
| "@webex/contact-center": "3.12.0-next.148", |
There was a problem hiding this comment.
On CI or any immutable install, this version bump makes installation fail because the sample workspace snapshot in yarn.lock still declares 3.12.0-next.133. Yarn 4.5.1 help defines --immutable as “Abort with an error exit code if the lockfile was to be modified”; running node .yarn/releases/yarn-4.5.1.cjs install --immutable exits with YN0028 and shows the exact 133→148 lockfile change, while the Install Dependencies steps in .github/workflows/pull-request.yml run plain yarn (immutable by default on CI). Regenerate and commit the lockfile.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a3c5ddf11
ℹ️ 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 interactionId = currentTask.data.interactionId; | ||
| const entries = store.cc.aiSummaries[interactionId]; |
There was a problem hiding this comment.
When an SDK summary request resolves or a receiver summary arrives while the widgets remain mounted, this read is not invalidated: Store marks cc as observable.ref in packages/contact-center/store/src/store.ts, while all summary writers replace only the nested cc.aiSummaries property without replacing cc. Consequently the observer wrappers do not re-run either this adapter or getReceiverSummary, leaving the new views absent or stuck in the generating state (and local edit/feedback projections stale) until an unrelated observable happens to render the widget; expose an observable store field or revision for these updates.
Useful? React with 👍 / 👎.
This branch has not been deployed
2 participants
Summary
Connects consult/transfer, receiving-agent and post-call summary views to the merged SDK-backed store. Includes the updated #749 branch; the entire
packages/contact-center/cc-componentstree matches #749 exactly.Widget adapters read
store.cc.aiSummaries, pass raw SDK content unchanged, and supply local edited values separately. Edits, copy counts and deferred feedback update the SDK response record insiderunInAction; callbacks reject stale task/content identities. Unchanged edits do not modify the response, and reverting a structured edit removes its override. Receiver feedback uses the displayed SDK payload when calling the store.SDK requests and response submission remain in the store. Consult/transfer sends the initiator response as the operation starts; wrap-up awaits its summary response before calling the original Task's wrap-up operation. The existing sample files and store files are unchanged by this synchronization. Direct MobX dependencies are declared for the adapters, and the sample lockfile entry matches its already-declared SDK version.
Dependencies and merge blockers
#748 is merged; #749 must merge before this PR. Review the widget integration layer.
ccasobservable.refand adds an ordinarycc.aiSummariesproperty. Replacing that nested property does not notify observers, so live summary updates need a separate store fix. The store remains untouched as requested.onAISummaryStatusChangeandAISummaryStatusDetailstill reference APIs removed from the merged store. These two existing errors prevent the task package's full TypeScript check. This synchronization leaves that callback plumbing flagged for cleanup.Both blockers are recorded in the task module spec. This PR is not ready to merge while they remain.
Verification
Under Node 24:
yarn install --immutable --mode=skip-buildpassed with existing peer warnings.git diff --checkpassed.