docs(contact-center): add config service AI documentation-Agents#4730
docs(contact-center): add config service AI documentation-Agents#4730Shreyas281299 wants to merge 2 commits intowebex:nextfrom
Conversation
Shreyas281299
added
the
validated
|
@codex review |
![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: 55fdceefd7
ℹ️ 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".
|
|
||
| ## Related | ||
|
|
||
| - [ARCHITECTURE.md](ARCHITECTURE.md) - Technical deep-dive |
There was a problem hiding this comment.
The Related section points to ARCHITECTURE.md, but this repo does not contain that file (the only architecture doc is webex-plugin-architecture.md), so the new guide currently includes a dead link. Please update this entry to an existing document or remove it to avoid sending readers/agents to missing documentation.
Useful? React with 👍 / 👎.
| const profile = await cc.register(); | ||
|
|
||
| // Profile contains all agent configuration | ||
| console.log('Agent ID:', profile.agentId); |
There was a problem hiding this comment.
console.log(); — violates Critical Rule 2. Replace with LoggerProxy.error(...)
| --- | ||
|
|
||
| ## Related | ||
|
|
There was a problem hiding this comment.
AI agents need navigation back to the orchestrator. Add link to ../../../AGENTS.md
| @@ -0,0 +1,186 @@ | |||
| # Config Service - AI Agent Guide | |||
|
|
|||
| > **Purpose**: Fetch and aggregate agent configuration data from multiple API endpoints to build the agent Profile. | |||
There was a problem hiding this comment.
Should state "This is the authoritative doc for Core scope" per root AGENTS.md Rule 7
|
|
||
| - [ARCHITECTURE.md](ARCHITECTURE.md) - Technical deep-dive | ||
| - [types.ts](../types.ts) - Type definitions | ||
| - [Util.ts](../Util.ts) - Profile parsing utilities |
There was a problem hiding this comment.
is it Utils.ts or Util.ts ? I am not sure just check once .
|
|
||
| **Flow**: | ||
| 1. Fetch user data (`getUserUsingCI`) | ||
| 2. Fetch org info, settings, tenant data in parallel |
There was a problem hiding this comment.
Add the method name for each of these like how it was done for the first point
|
|
||
| ## Quick Usage | ||
|
|
||
| ```typescript |
There was a problem hiding this comment.
Code reference here should talk about hpw it is used internally instead of adding cc.register()
|
|
||
| ## Overview | ||
|
|
||
| The Config Service is an **internal service** that builds the comprehensive `Profile` object by: |
There was a problem hiding this comment.
Everywhere we say profile it should say AgentProfile and we should talk about why we are building this agent profile and how it is used
|
|
||
| --- | ||
|
|
||
| ## Data Aggregation Flow |
There was a problem hiding this comment.
This should be present before we talk about each of the methods individually
|
|
||
| ## Error Handling | ||
|
|
||
| Config service methods throw errors on API failures: |
There was a problem hiding this comment.
Is this error handling common for all the API methods that help in creating the agent profile, if so then we should mention that as a point and mention that final agentprofile fetching will error out even if one of the API fails
| | `agentId` | string | Unique agent identifier | | ||
| | `agentName` | string | Display name | | ||
| | `agentMailId` | string | Email address | | ||
| | `teams` | Team[] | Assigned teams | |
There was a problem hiding this comment.
Add reference links maybe for these types because it will not know what Team is or DialpLan
- Add scope authority note per root AGENTS.md Rule 6 - Rename Profile to Agent Profile throughout with usage context - Rewrite Quick Usage to show internal service usage - Replace console.log with LoggerProxy in code examples - Add type reference links for Team, Entity, DialPlan, LoginOption - Add method names to all getAgentConfig flow steps - Move Data Aggregation Flow before API Methods section - Clarify error handling: all-or-nothing via Promise.all - Remove broken ARCHITECTURE.md link, add root AGENTS.md link - Verify Util.ts filename is correct (no change needed) 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: 5dc3052565
ℹ️ 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".
|
|
||
| > **Purpose**: Fetch and aggregate agent configuration data from multiple API endpoints to build the Agent Profile. | ||
| > | ||
| > **Scope Authority**: This is the authoritative documentation for the **Config** service scope. See [Root AGENTS.md](../../../../AGENTS.md) for the orchestrator and cross-scope rules. |
There was a problem hiding this comment.
The new guide points readers to ../../../../AGENTS.md as the orchestrator doc, but a repo-wide check (rg --files -g 'AGENTS.md') shows only this file exists, so the link is dead. In practice, agents following the documented scope chain cannot discover the referenced cross-scope rules, which defeats the purpose of the “Scope Authority” section and leaves navigation instructions broken.
Useful? React with 👍 / 👎.
ciscoRankush
left a comment
ciscoRankush
left a comment
There was a problem hiding this comment.
PR #4730 Review: docs(contact-center): add config service AI documentation-Agents
Blocking (Must Fix)
B1. PR description says "service/core" but file is for "service/config" -- The PR body says "Adds agents.md for service/core folder" but the actual file is at services/config/ai-docs/AGENTS.md. Misleading.
B2. getAgentConfig flow described as sequential, actual code is heavily parallelized (lines 99-108)
The numbered steps imply sequential execution. The actual code (index.ts:57-108) starts user data, org info, org settings, tenant data, URL mapping, and aux codes ALL concurrently as Promises, then awaits user data first (because desktop profile and site info depend on it), then resolves everything via Promise.all. An AI agent following this doc would implement a much less efficient sequential pattern.
B3. getURLMapping completely missing from flow diagram and step list (lines 77-90)
getURLMapping() is called at index.ts:63 and its result urlMappingData is passed to parseAgentConfigs at line 129. This 9th data source is absent from the diagram and the flow description. Also getSiteInfo is missing from the diagram (used to compute multimediaProfileId at line 113).
B4. getOutdialAniEntries params missing filter and attributes fields (lines 112-122)
The OutdialAniParams type (types.ts:1203-1216) also has filter?: string and attributes?: string, used in index.ts:717-718 to build query params. Omitting them would cause AI agents to generate incomplete API calls.
B5. teams type mismatch (line 56)
Doc says Team[] but getAllTeams() returns Promise<TeamList[]> (index.ts:345). TeamList (types.ts:631-658) has fields like id, name, teamType, siteId -- a different shape from Team which has teamId, teamName. The runtime data is TeamList[], not Team[].
Suggestions (Should Fix)
S1. PR template not filled out -- No Change Type checked, no certifications checked, testing scenarios empty. FedRAMP compliance requires these.
S2. Events section imprecise (lines 126-134) -- Says "the config service defines event constants" but it's actually types.ts that defines CC_EVENTS, CC_AGENT_EVENTS, CC_TASK_EVENTS as const objects. The table shows only ~9 events from a total of 75+.
S3. Types table missing 15+ important types (lines 139-149) -- Missing: AgentResponse, TeamList, OrgInfo, OrgSettings, TenantData, URLMapping, DialPlanEntity, SiteInfo, MultimediaProfileResponse, OutdialAniParams, ListAuxCodesResponse, ListTeamsResponse, WrapupData, Entity, and more.
S4. getMultimediaProfileById method undocumented -- Public method at index.ts:248-282 exists in the service class but is not mentioned anywhere in the doc.
S5. Util.ts helper functions not documented -- Important functions like parseAgentConfigs(), getFilterAuxCodes(), getFilteredDialplanEntries(), getDefaultWrapUpCode(), getUrlMapping(), getMsftConfig()/getWebexConfig(), getDefaultAgentDN() are not documented.
S6. Profile table missing 40+ fields (lines 50-66) -- Only 12 fields shown but Profile type has 50+ fields including autoAnswer, forceDefaultDn, regexUS, regexOther, enterpriseId, tenantTimezone, wrapUpData, defaultWrapupCode, maskSensitiveData, lostConnectionRecoveryTimeout, etc. Should explicitly state this is a partial list.
Nits
- N1. Error handling example uses hardcoded
'ConfigService'instead ofCONFIG_FILE_NAMEconstant (lines 155-164) - N2.
constants.tsdescription (line 199) says just "API endpoints" but also contains pagination defaults, agent state constants, method names, and aux code attributes
Reviewer: Rankush Priya
| ``` | ||
|
|
||
| --- | ||
|
|
There was a problem hiding this comment.
[Blocking] The getAgentConfig flow is described as sequential numbered steps, but the actual code (index.ts:57-108) is heavily parallelized:
- Steps 1-3 (user, org info, org settings, tenant data, URL mapping, aux codes) all start concurrently as Promises (lines 59-69)
userConfigDatais awaited first (line 71), thendesktopProfile/siteInfostart (lines 77-78) since they depend on itdialPlandepends ondesktopProfile(lines 80-81)teamsdepends onuserConfigData.teamIds(lines 84-86)- Everything resolves via
Promise.all(lines 88-108)
An AI agent following this sequential doc would implement a much less efficient pattern.
| --- | ||
|
|
||
| ## Data Aggregation Flow | ||
|
|
There was a problem hiding this comment.
[Blocking] This flow diagram is missing two data sources:
getURLMapping()-- called atindex.ts:63, resulturlMappingDatapassed toparseAgentConfigsat line 129getSiteInfo()-- called atindex.ts:78, used to computemultimediaProfileIdat line 113 which is passed toparseAgentConfigs
An AI agent replicating this aggregation pattern would produce an incomplete implementation.
| 1. Fetch user data (`getUserUsingCI`) | ||
| 2. Fetch org info (`getOrgInfo`), settings (`getOrganizationSetting`), tenant data (`getTenantData`) in parallel | ||
| 3. Fetch aux codes with pagination (`getAllAuxCodes`) | ||
| 4. Fetch desktop profile (`getDesktopProfileById`), site info (`getSiteInfo`) |
There was a problem hiding this comment.
[Blocking] getOutdialAniEntries params are incomplete. The OutdialAniParams type (types.ts:1203-1216) also includes:
filter?: string(line 1213)attributes?: string(line 1215)
These are used in index.ts:717-718 to build query parameters. Omitting them means AI agents won't pass filter/attributes when needed.
| --- | ||
|
|
||
| ## Agent Profile Object (Key Fields) | ||
|
|
There was a problem hiding this comment.
[Blocking] teams type is listed as Team[] but getAllTeams() (index.ts:345) returns Promise<TeamList[]>. TeamList (types.ts:631-658) has fields id, name, teamType, siteId etc. -- a different shape from Team which has teamId, teamName. The runtime data in Profile is actually TeamList[].
| The config service defines event constants used throughout the SDK: | ||
|
|
||
| | Event Category | Examples | | ||
| |----------------|----------| |
There was a problem hiding this comment.
[Suggestion] The types table is missing 15+ important types used by the config service: AgentResponse, TeamList, OrgInfo, OrgSettings, TenantData, URLMapping, DialPlanEntity, SiteInfo, MultimediaProfileResponse, OutdialAniParams, ListAuxCodesResponse, ListTeamsResponse, WrapupData, Entity, etc. At minimum, list them with one-line descriptions.
| - **Agent Profile Aggregation**: Combines data from 8+ API endpoints | ||
| - **Aux Codes Fetching**: Gets all idle and wrapup codes with pagination | ||
| - **Team Data**: Retrieves agent's team assignments | ||
| - **Dial Plan**: Fetches number transformation rules |
There was a problem hiding this comment.
[Suggestion] The Profile table only shows 12 of 50+ fields. Notable missing fields: autoAnswer, forceDefaultDn, regexUS, regexOther, enterpriseId, tenantTimezone, wrapUpData, defaultWrapupCode, maskSensitiveData, lostConnectionRecoveryTimeout. Either be comprehensive or explicitly state this is an abbreviated list with a pointer to types.ts for the full definition.
ciscoRankush
dismissed
their stale review
Removing review - posted prematurely
3 participants
COMPLETES # https://jira-eng-gpk2.cisco.com/jira/browse/SPARK-775685
This pull request addresses
Adds agents.md for service/core folder
Change Type
The following scenarios were tested
The GAI Coding Policy And Copyright Annotation Best Practices
I certified that
Make sure to have followed the contributing guidelines before submitting.