docs(contact-center): add config service AI documentation-Architecture#4731
docs(contact-center): add config service AI documentation-Architecture#4731Shreyas281299 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: 3bbf1f4e5c
ℹ️ 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".
| listTeams: (orgId, page, pageSize, filter) => | ||
| `/organization/${orgId}/team?page=${page}&pageSize=${pageSize}&filter=${filter}`, |
There was a problem hiding this comment.
This section states the snippet is Defined in constants.ts, but these paths do not match the implemented endPointMap routes (for example, listTeams in src/services/config/constants.ts uses organization/${orgId}/v2/team..., and several others include different v2 paths/query params). If engineers use this architecture doc to implement or troubleshoot config calls, they will hit incorrect URLs and 404s, so the examples should reflect the real constants.
Useful? React with 👍 / 👎.
…TURE.md Update the API Endpoints section to match the actual endPointMap in constants.ts. Fix incorrect paths (missing v2/, wrong routes), add missing query parameters (agentView=true), and include previously omitted endpoints (multimediaProfile, urlMapping, queueList, entryPointList, addressBookEntries). 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: 576b1ca3af
ℹ️ 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".
| agentId: userData.id, | ||
| agentName: `${userData.firstName} ${userData.lastName}`, |
There was a problem hiding this comment.
The parseAgentConfigs example maps agentId to userData.id, but the real implementation in services/config/Util.ts maps agentId from userData.ciUserId and uses userData.id for analyserUserId; this mismatch can lead engineers to propagate the wrong identifier when they copy this architecture guidance for profile shaping or downstream lookups that key on CI user ID.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Review Summary
This PR adds architecture documentation for the config service, which aligns with the ai-docs framework. The Mermaid diagrams are a nice addition for visualizing data flow. CI is passing since this is docs-only.
However, there are several factual inaccuracies when cross-referenced with the actual source code. These would mislead developers and AI agents who use this documentation as a reference.
Blocking (Must Fix)
B1. agentId mapped to wrong field (line 248)
Doc shows agentId: userData.id but Util.ts:191 uses agentId: userData.ciUserId. userData.id is actually mapped to analyserUserId. This is a critical factual error.
B2. parseAgentConfigs example misrepresents actual logic (lines 240-260)
teams: teamData.map(...)— actual code (Util.ts:185) passesteams: teamDatadirectly, no mapping.idleCodes/wrapupCodesfiltering — real code usesgetFilterAuxCodes()with complex logic (active status, specificCodes, hardcoded "Available" state). The simple.filter()hides critical business logic.
Suggestions (Should Fix)
S1. PR description template not filled out — FedRAMP compliance fields, change type, and test sections are empty. Template states this will result in PR rejection.
S2. Sequence diagram misrepresents getAllTeams concurrency (lines 110-128) — Shows it as sequential, but actual code starts it in parallel with other promises via Promise.all().
S3. Event constants subset is arbitrary (lines 276-309) — Shows only 4 of 18 agent events and 3 of 50+ task events. Should either show full list or clearly point to types.ts as canonical source.
S4. Error handling uses fabricated method names (lines 314-345) — Uses getSomeData, endPointMap.someEndpoint. Better to use an actual method like getUserUsingCI.
Nits
- N1. Uses
page = 0literal instead ofDEFAULT_PAGEconstant (line 221) - N2.
ConfigDatatype (line 237) does not exist in the codebase; actual function uses inline type - N3. Troubleshooting section is generic — could include actual log string patterns
| // Build Profile object | ||
| return { | ||
| agentId: userData.id, | ||
| agentName: `${userData.firstName} ${userData.lastName}`, |
There was a problem hiding this comment.
[Blocking] agentId: userData.id is incorrect. The actual code in Util.ts:191 uses agentId: userData.ciUserId. userData.id is mapped to analyserUserId (Util.ts:239). This would cause AI agents or developers to use the wrong identifier for agent identification.
| agentId: userData.id, | ||
| agentName: `${userData.firstName} ${userData.lastName}`, | ||
| teams: teamData.map(t => ({ teamId: t.id, teamName: t.name })), | ||
| idleCodes: auxCodes.filter(c => c.workTypeCode === 'IDLE_CODE'), |
There was a problem hiding this comment.
[Blocking] teams: teamData.map(t => ({ teamId: t.id, teamName: t.name })) — The actual code at Util.ts:185 just does teams: teamData (passes the raw TeamList[] directly, no mapping).
Also, idleCodes and wrapupCodes filtering is oversimplified. The real code uses getFilterAuxCodes() which checks active status, specificCodes access level, and pushes a hardcoded "Available" state entry (Util.ts:169-180). The simple .filter() shown here hides critical business logic.
| API-->>WR: Teams | ||
| WR-->>Cfg: Team list | ||
|
|
||
| Cfg->>Cfg: parseAgentConfigs(allData) |
There was a problem hiding this comment.
[Suggestion] The sequence diagram shows getAllTeams happening sequentially after the dial plan check. In the actual code (index.ts:84-86), the userTeamPromise is started in parallel alongside other promises and all are awaited via Promise.all(). The diagram misrepresents the concurrency model.
| teamData, | ||
| tenantData, | ||
| orgInfoData, | ||
| auxCodes, |
There was a problem hiding this comment.
[Nit] The ConfigData type referenced here does not exist in the codebase. The actual parseAgentConfigs function in Util.ts uses an inline object type parameter, not a named type.
| } | ||
|
|
||
| return allTeams; | ||
| } |
There was a problem hiding this comment.
[Nit] Uses let page = 0; literal. The actual code uses let page = DEFAULT_PAGE; (imported constant from constants.ts).
| method: HTTP_METHODS.GET, | ||
| }); | ||
|
|
||
| if (response.statusCode !== 200) { |
There was a problem hiding this comment.
[Suggestion] The error handling example uses fabricated method names (getSomeData, METHODS.GET_SOME_DATA, endPointMap.someEndpoint). Since this doc describes the actual architecture, consider using a real method like getUserUsingCI as the example.
2 participants
COMPLETES # https://jira-eng-gpk2.cisco.com/jira/browse/SPARK-775683
This pull request addresses
< DESCRIBE THE CONTEXT OF THE ISSUE >
by making the following changes
< DESCRIBE YOUR CHANGES >
Change Type
The following scenarios were tested
< ENUMERATE TESTS PERFORMED, WHETHER MANUAL OR AUTOMATED >
The GAI Coding Policy And Copyright Annotation Best Practices
I certified that
Make sure to have followed the contributing guidelines before submitting.