docs(contact-center): add ai-docs SDD pattern documentation#4723
docs(contact-center): add ai-docs SDD pattern documentation#4723ciscoRankush merged 4 commits intowebex:task-refactorfrom
Conversation
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fb5d146901
ℹ️ 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".
packages/@webex/contact-center/ai-docs/patterns/sdk-plugin-patterns.md
Outdated
Show resolved
Hide resolved
Add pattern documentation for AI agents to reference during code generation: - typescript-patterns.md — type conventions, error handling, LoggerProxy usage - testing-patterns.md — Jest patterns, mocking, test structure - event-driven-patterns.md — event emission, listener patterns - websocket-patterns.md — WebSocket connection, reconnection patterns - sdk-plugin-patterns.md — WebexPlugin extension, service registration Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
fb5d146 to
8b25a96
Compare
| const event2: CC_EVENTS = 'InvalidEvent'; // ❌ Compile error | ||
| ``` | ||
|
|
||
| ### Response Type Pattern |
| getOrgId: jest.fn(() => 'mockOrgId'), | ||
| }, | ||
| config: config, | ||
| }) as unknown as WebexSDK; |
There was a problem hiding this comment.
We should not use unkown, thie way LLM would put unknown everywhere
|
|
||
| ```bash | ||
| # Run tests with coverage | ||
| yarn workspace @webex/contact-center test --coverage |
There was a problem hiding this comment.
| yarn workspace @webex/contact-center test --coverage | |
| yarn workspace @webex/contact-center test:unit --coverage |
| ### Message Reception | ||
|
|
||
| ``` | ||
| WebSocket → WebSocketManager → cc.handleWebsocketMessage → Event Emission |
There was a problem hiding this comment.
Mermaid diagram. This is a very speicifc flow. We can add a generic flow to explain the generic event reception
There was a problem hiding this comment.
this is more machine readable statement , so lets keep it
| ### From Plugin Class | ||
|
|
||
| ```typescript | ||
| // Using WebexPlugin's emit method | ||
| this.emit(AGENT_EVENTS.AGENT_STATE_CHANGE, eventData); | ||
|
|
||
| // Using trigger for some events (alternative method) | ||
| this.trigger(TASK_EVENTS.TASK_INCOMING, task); | ||
| ``` | ||
|
|
||
| ### From TaskManager | ||
|
|
||
| ```typescript | ||
| // TaskManager extends EventEmitter | ||
| export default class TaskManager extends EventEmitter { | ||
| private emitTaskEvent(eventType: string, task: ITask) { | ||
| this.emit(eventType, task); | ||
| } | ||
|
|
||
| public handleIncomingTask(taskData: TaskData) { | ||
| const task = this.createTask(taskData); | ||
| this.emit(TASK_EVENTS.TASK_INCOMING, task); | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| --- |
There was a problem hiding this comment.
- We dont need subheading
What we need?
- How we emit events. Two ways
- We need to differntiate when to use trigger and when to use emit
P.S: emit is used when the class is extendin EventEmitter and trigger is used in the other case-trigger is very speicifc for cc object as it extends WebexPlugin - Code example of each way
| ### In Application Code | ||
|
|
||
| ```typescript | ||
| // Subscribe to events | ||
| const cc = webex.cc; | ||
|
|
||
| cc.on('agent:stateChange', (event) => { | ||
| console.log('Agent state changed:', event.state); | ||
| }); | ||
|
|
||
| cc.on('task:incoming', (task) => { | ||
| console.log('New task:', task.interactionId); | ||
| }); | ||
|
|
||
| // Unsubscribe | ||
| const handler = (event) => { /* handle */ }; | ||
| cc.on('agent:stateChange', handler); | ||
| cc.off('agent:stateChange', handler); | ||
| ``` | ||
|
|
||
| ### Internal Subscription | ||
|
|
||
| ```typescript | ||
| // In constructor | ||
| constructor() { | ||
| this.$webex.once(READY, () => { | ||
| this.services.webSocketManager.on('message', this.handleWebsocketMessage); | ||
| this.services.connectionService.on('connectionLost', this.handleConnectionLost); | ||
|
|
||
| this.taskManager.on(TASK_EVENTS.TASK_INCOMING, this.handleIncomingTask); | ||
| this.taskManager.on(TASK_EVENTS.TASK_HYDRATE, this.handleTaskHydrate); | ||
| }); | ||
| } | ||
|
|
||
| // Cleanup in deregister | ||
| public async deregister() { | ||
| this.taskManager.off(TASK_EVENTS.TASK_INCOMING, this.handleIncomingTask); | ||
| this.services.webSocketManager.off('message', this.handleWebsocketMessage); | ||
| } | ||
| ``` | ||
|
|
There was a problem hiding this comment.
What we need?
- How to add/remove an event listener?
- Examples
2.1 Internal event listening
2.2 Events sent to application (from cc,task)
| ## Connection Events | ||
|
|
||
| ### Connection Lost/Reconnected | ||
|
|
||
| ```typescript | ||
| // services/core/websocket/types.ts | ||
| export type ConnectionLostDetails = { | ||
| isConnectionLost: boolean; | ||
| isSocketReconnected: boolean; | ||
| }; | ||
|
|
||
| // Handling in cc.ts | ||
| private async handleConnectionLost(msg: ConnectionLostDetails): Promise<void> { | ||
| if (msg.isConnectionLost) { | ||
| LoggerProxy.info('Connection lost', { | ||
| module: CC_FILE, | ||
| method: 'handleConnectionLost', | ||
| }); | ||
| } else if (msg.isSocketReconnected) { | ||
| LoggerProxy.info('Connection reconnected', { | ||
| module: CC_FILE, | ||
| method: 'handleConnectionLost', | ||
| }); | ||
|
|
||
| if (this.$config?.allowAutomatedRelogin) { | ||
| await this.silentRelogin(); | ||
| } | ||
| } | ||
| } | ||
| ``` |
There was a problem hiding this comment.
This is not part of SDK. Lets remove this
packages/@webex/contact-center/ai-docs/patterns/testing-patterns.md
Outdated
Show resolved
Hide resolved
…tern docs Apply reviewer feedback from Shreyas and Kesari plus source code validation: - Delete sdk-plugin-patterns.md (too cc.ts-specific) and websocket-patterns.md (lifecycle content folded into event-driven-patterns.md) - Fix AGENT_EVENTS/TASK_EVENTS to use enum syntax with correct values - Fix Failure type to use Msg<T> wrapper matching actual GlobalTypes.ts - Fix interface naming convention to acknowledge I prefix for contracts - Fix MetricsManager mock, file tree paths, test command, and event testing patterns - Genericize event flow, document trigger vs emit accurately, add WebSocket Lifecycle - Remove all broken links to deleted files across AGENTS.md, RULES.md, README.md Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add 5 missing CC_AGENT_EVENTS members (AGENT_DN_REGISTERED, AGENT_BUDDY_AGENTS, AGENT_BUDDY_AGENTS_SUCCESS, AGENT_BUDDY_AGENTS_RETRIEVE_FAILED, AGENT_CONTACT_RESERVED) to match config/types.ts - Fix arrow function claim: document both arrow property and .bind(this) patterns used in cc.ts - Fix getActiveTasks → getAllTasks in testing-patterns.md mock Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
|
||
| ## Event Emission | ||
|
|
||
| There are **two methods** for emitting events: `trigger` and `emit`. Both work on the `cc` object (which extends `WebexPlugin`), but they should be used in different contexts. |
There was a problem hiding this comment.
emit doesn't work on cc object because it doesn't extend event emitter class. We are adding @ts-ignore to be able to invoke it. Emphasis here should be that classes which extend EventEmitter can use emit for event emission but for classes like cc.ts can use trigger for emission
packages/@webex/contact-center/ai-docs/patterns/event-driven-patterns.md
Show resolved
Hide resolved
| const cc = webex.cc; | ||
|
|
||
| // Agent events (from cc) | ||
| cc.on(AGENT_EVENTS.AGENT_STATE_CHANGE, (event) => { |
There was a problem hiding this comment.
In these examples, we should have callbacks while listening because .off won;t work if we don;t have separate callback function
… doc emit does not work on cc object (extends WebexPlugin, not EventEmitter). Use trigger for WebexPlugin classes, emit for EventEmitter classes. Also adds named callbacks in examples so .off() works properly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f5cd0f03c9
ℹ️ 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".
| } | ||
|
|
||
| // Register with .bind(this) | ||
| this.services.connectionService.on('connectionLost', this.handleConnectionLost.bind(this)); |
There was a problem hiding this comment.
Keep a stable callback when binding connectionLost listener
The on('connectionLost', this.handleConnectionLost.bind(this)) pattern creates a new function instance at registration time, but the cleanup examples later use off(..., this.handleConnectionLost). Because those are different references, the listener cannot be removed, so repeated register/deregister cycles can accumulate duplicate reconnection handlers and fire relogin logic multiple times.
Useful? React with 👍 / 👎.
| // Register listeners after SDK is ready | ||
| this.$webex.once(READY, () => { | ||
| this.services.webSocketManager.on('message', this.handleWebsocketMessage); | ||
| this.services.connectionService.on('connectionLost', this.handleConnectionLost); |
There was a problem hiding this comment.
Bind handleConnectionLost in the internal listener example
This example subscribes this.handleConnectionLost without binding even though the handler is shown as a regular class method; with EventEmitter-style callbacks, this inside the handler will be the emitter, not the ContactCenter instance. That breaks access to instance members like $config/silentRelogin, so reconnection recovery logic can silently stop working if generated code follows this snippet.
Useful? React with 👍 / 👎.
| this.trigger(TASK_EVENTS.TASK_HYDRATE, task); | ||
| ``` | ||
|
|
||
| > **Note**: `trigger` requires `// @ts-ignore` because WebexPlugin's TypeScript type definitions don't expose it. |
There was a problem hiding this comment.
This should be emit here, it should say that we can still use emit but with @ts-ignore. We can address this PRs which are not merged yet
COMPLETES #SPARK-775699
This pull request addresses
AI agents need reference documentation for the established coding patterns in the @webex/contact-center SDK so they can generate code that is consistent with existing conventions. Without pattern docs, agents
produce code that uses console.log instead of LoggerProxy, misses MetricsManager tracking, uses incorrect error handling, and deviates from established type conventions.
by making the following changes
ai-docs/patterns/with 5 pattern documentation files:Change Type
The following scenarios were tested
../../patterns/relative pathsThe GAI Coding Policy And Copyright Annotation Best Practices
I certified that