Enrich conversations/search response with chatbot stats#588
Enrich conversations/search response with chatbot stats#588SasmithaDilshan wants to merge 2 commits into
Conversation
Append messagesExchanged, troubleshootingAttempts, and kbArticlesReviewed
from the AI chat agent summary endpoint to each conversation in the
POST /projects/{id}/conversations/search response. Stats default to 0
when unavailable.
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
📝 WalkthroughWalkthroughAdds three optional analytics fields to the Conversation type and enriches the conversations search endpoint to populate them by calling the AI chat agent summary API for each conversation; also updates the AI WS client path and fixes a trailing-space in a comment. Changes
Sequence DiagramsequenceDiagram
autonumber
actor Client
participant Service as Service (backend)
participant Search as SearchEngine/DB
participant AIAgent as AI Chat Agent
Client->>Service: GET /projects/{id}/conversations/search
Service->>Search: Query conversations (pagination)
Search-->>Service: Conversation list + totalRecords
loop for each conversation (parallel)
Service->>AIAgent: getSummary(projectId, conversationId)
alt success
AIAgent-->>Service: Summary with stats
Service->>Service: Populate analytics fields
else failure
AIAgent-->>Service: Error
Service->>Service: Set analytics fields = 0
end
end
Service->>Client: Respond { conversations, totalRecords, limit, offset }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/customer-portal/backend/modules/types/types.bal (1)
1046-1051: Keep the “always present” stats on a search-specific response model.These fields are guaranteed for
POST /projects/{id}/conversations/search, but declaring them as optional on the sharedConversationrecord makes the public schema treat them as omittable and also leaks search-only fields into other endpoints that reuseConversation. A dedicated enriched search DTO with requiredintfields would keep the contract explicit.Based on learnings, validate public API surface changes and corresponding mapping logic when changing BAL response types.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/backend/modules/types/types.bal` around lines 1046 - 1051, The Conversation record currently declares messagesExchanged, troubleshootingAttempts, and kbArticlesReviewed as optional, which exposes search-only fields across other endpoints; create a dedicated enriched search DTO (e.g., ConversationSearchResult or ConversationEnrichedForSearch) with those three fields as required int (non-nullable) and update the search handler/mapping to populate and return this DTO for POST /projects/{id}/conversations/search while leaving the shared Conversation record unchanged; ensure mapping logic (the function that builds Conversation from data or the search-specific mapper) sets the required ints and adjust any references to use ConversationSearchResult for the search endpoint only.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/customer-portal/backend/service.bal`:
- Around line 1374-1389: The loop makes a blocking ai_chat_agent:getSummary call
per conversation causing serial upstream calls; change this to bounded
parallelism or batching: collect conversation IDs from
baseResponse.conversations, spawn concurrent getSummary tasks with a fixed
worker/concurrency limit (e.g., semaphore or worker pool) rather than calling
ai_chat_agent:getSummary inside the foreach, map each result back to the cloned
types:Conversation (enriched = conv.clone()) and set
messagesExchanged/troubleshootingAttempts/kbArticlesReviewed from
ai_chat_agent:ConversationSummaryResponse on success or default to 0 on error;
alternatively, if the AI agent supports a batched summary endpoint, replace
per-item ai_chat_agent:getSummary with a single batched call and merge results
into enrichedConversations.
---
Nitpick comments:
In `@apps/customer-portal/backend/modules/types/types.bal`:
- Around line 1046-1051: The Conversation record currently declares
messagesExchanged, troubleshootingAttempts, and kbArticlesReviewed as optional,
which exposes search-only fields across other endpoints; create a dedicated
enriched search DTO (e.g., ConversationSearchResult or
ConversationEnrichedForSearch) with those three fields as required int
(non-nullable) and update the search handler/mapping to populate and return this
DTO for POST /projects/{id}/conversations/search while leaving the shared
Conversation record unchanged; ensure mapping logic (the function that builds
Conversation from data or the search-specific mapper) sets the required ints and
adjust any references to use ConversationSearchResult for the search endpoint
only.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3c421cbf-f48c-44d4-8522-c2e8da4b2c97
📒 Files selected for processing (3)
apps/customer-portal/backend/modules/ai_chat_agent/ai_chat_agent.balapps/customer-portal/backend/modules/types/types.balapps/customer-portal/backend/service.bal
| // Enrich each conversation with chatbot stats from the AI chat agent. | ||
| types:Conversation[] enrichedConversations = []; | ||
| foreach types:Conversation conv in baseResponse.conversations { | ||
| types:Conversation enriched = conv.clone(); | ||
| ai_chat_agent:ConversationSummaryResponse|error summary = ai_chat_agent:getSummary(id, conv.id); | ||
| if summary is ai_chat_agent:ConversationSummaryResponse { | ||
| enriched.messagesExchanged = summary.messagesExchanged; | ||
| enriched.troubleshootingAttempts = summary.troubleshootingAttempts; | ||
| enriched.kbArticlesReviewed = summary.kbArticlesReviewed; | ||
| } else { | ||
| enriched.messagesExchanged = 0; | ||
| enriched.troubleshootingAttempts = 0; | ||
| enriched.kbArticlesReviewed = 0; | ||
| } | ||
| enrichedConversations.push(enriched); | ||
| } |
There was a problem hiding this comment.
Avoid one blocking AI summary call per conversation.
This turns a paginated search into N extra upstream calls, executed serially. Latency will now grow linearly with the page size, and when the AI agent is slow or unavailable the endpoint still sits through every failing request before it can fall back to 0. Please switch this to bounded parallelism, batching, or precomputed/persisted stats instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/customer-portal/backend/service.bal` around lines 1374 - 1389, The loop
makes a blocking ai_chat_agent:getSummary call per conversation causing serial
upstream calls; change this to bounded parallelism or batching: collect
conversation IDs from baseResponse.conversations, spawn concurrent getSummary
tasks with a fixed worker/concurrency limit (e.g., semaphore or worker pool)
rather than calling ai_chat_agent:getSummary inside the foreach, map each result
back to the cloned types:Conversation (enriched = conv.clone()) and set
messagesExchanged/troubleshootingAttempts/kbArticlesReviewed from
ai_chat_agent:ConversationSummaryResponse on success or default to 0 on error;
alternatively, if the AI agent supports a batched summary endpoint, replace
per-item ai_chat_agent:getSummary with a single batched call and merge results
into enrichedConversations.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/customer-portal/backend/service.bal (1)
1374-1396:⚠️ Potential issue | 🟠 MajorLog AI-summary failures; current code silently masks them as zeros.
The fan-out now executes in parallel (good — addresses the prior serial-call concern), but in the error branch at lines 1390–1394, failures from
ai_chat_agent:getSummaryare silently coerced to0for all three fields without any log. If the AI agent is degraded, every conversation in the response will show0stats and there will be no signal in the logs to explain why. That’s a meaningful observability gap for a user-facing endpoint.Also worth considering:
- Per-call timeout bound: the endpoint’s latency is now bounded by the slowest
getSummary; confirm the underlying HTTP client has a timeout so a hung upstream can’t stall the search response.- Bounded concurrency: large pages fan out N concurrent requests to the AI service per search; consider batching (if an AI-side batch endpoint exists) or a worker-pool limit for resilience.
🛠️ Suggested minimal fix — add error log
if summary is ai_chat_agent:ConversationSummaryResponse { enriched.messagesExchanged = summary.messagesExchanged; enriched.troubleshootingAttempts = summary.troubleshootingAttempts; enriched.kbArticlesReviewed = summary.kbArticlesReviewed; } else { + log:printWarn(string `Failed to fetch AI summary for conversation: ${ + baseResponse.conversations[i].id}`, summary); enriched.messagesExchanged = 0; enriched.troubleshootingAttempts = 0; enriched.kbArticlesReviewed = 0; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/customer-portal/backend/service.bal` around lines 1374 - 1396, When a wait on summaryFutures[i] returns an error, don't silently assign zeros—log the failure with the conversation id and the error before falling back to defaults; specifically, in the fan-in loop that iterates baseResponse.conversations and handles ai_chat_agent:getSummary results, add a logger.error (including conv.id and the error value) in the else branch where summary is an error, then continue to set messagesExchanged/troubleshootingAttempts/kbArticlesReviewed to 0. Also verify ai_chat_agent:getSummary/its HTTP client has a per-call timeout and consider adding bounded concurrency (batching or a worker-pool) for the summaryFutures fan-out to avoid N unbounded concurrent requests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/customer-portal/backend/modules/ai_chat_agent/client.bal`:
- Line 43: The client builds the WebSocket URL as a path segment which won't
match the server resource signature; update the WebSocket URL construction in
the return new(...) expression to send sessionId as a query parameter instead
(use aiChatAgentWsBaseUrl and sessionId to produce /ws?sessionId=<id>) so the
server resource function get ws(http:Request req, string sessionId) receives
sessionId correctly.
---
Duplicate comments:
In `@apps/customer-portal/backend/service.bal`:
- Around line 1374-1396: When a wait on summaryFutures[i] returns an error,
don't silently assign zeros—log the failure with the conversation id and the
error before falling back to defaults; specifically, in the fan-in loop that
iterates baseResponse.conversations and handles ai_chat_agent:getSummary
results, add a logger.error (including conv.id and the error value) in the else
branch where summary is an error, then continue to set
messagesExchanged/troubleshootingAttempts/kbArticlesReviewed to 0. Also verify
ai_chat_agent:getSummary/its HTTP client has a per-call timeout and consider
adding bounded concurrency (batching or a worker-pool) for the summaryFutures
fan-out to avoid N unbounded concurrent requests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9d82bad7-bb97-428b-9b51-25a4e0cb8f57
📒 Files selected for processing (2)
apps/customer-portal/backend/modules/ai_chat_agent/client.balapps/customer-portal/backend/service.bal
|
|
||
| isolated function createAiChatAgentWsClient(string sessionId) returns websocket:Client|error { | ||
| return new (string `${aiChatAgentWsBaseUrl}/ws?sessionId=${sessionId}`, { | ||
| return new (string `${aiChatAgentWsBaseUrl}/ws/${sessionId}`, { |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Ballerina HTTP resource function: how to declare a path parameter vs a query parameter in the function signature
💡 Result:
In Ballerina HTTP resource functions, path parameters and query parameters are declared differently in the function signature. Path parameters are declared in the resource path using square brackets with the type, e.g., resource function get users/[string id]. The parameter 'id' of type string is automatically bound from the path segment. Query parameters are declared as regular typed parameters in the function signature without any special annotation (unless resolving ambiguity with payload), e.g., resource function get users(string name?, int limit = 10). They bind from the URL query string ?name=foo&limit=20. They can be optional (? suffix) or have defaults. Example: service / on new http:Listener(9090) { // Path param: matches /users/123 resource function get users/int id returns json { return {"id": id}; } // Query params: matches /users?name=foo&limit=10 resource function get users(string? name, int limit = 10) returns json[] { // use name and limit } }
Citations:
- 1: https://ballerina.io/spec/http
- 2: https://ballerina.io/spec/http/
- 3: https://pasidaleedissanayake.medium.com/an-exploration-of-rest-api-handling-with-ballerina-d57e50c85ed5
- 4: https://github.com/cloudstreet-dev/The-Ballerina-Programming-Language/blob/main/chapter-03-services.md
- 5: https://ballerina.io/learn/by-example/http-query-parameter/
- 6: https://ballerina.io/learn/by-example/http-client-query-parameter/
- 7: https://ballerina.io/learn/by-example/http-client-query-parameter
🏁 Script executed:
#!/bin/bash
# Confirm how `sessionId` is declared on the WS upgrade resource.
rg -nP -C2 'resource function get ws' --type=bal
# Also look for any alternate resource paths that capture sessionId as a path segment.
rg -nP --type=bal 'ws/\[string\s+sessionId\]|get\s+ws\b'Repository: wso2-open-operations/cs-tools
Length of output: 129
🏁 Script executed:
#!/bin/bash
# Locate service.bal file
fd -name "service.bal" apps/customer-portal/backend/
# Search for the WS resource without using --type flag
# Focus on the modules/ai_chat_agent or service.bal at the backend level
rg -nA5 'resource function get ws' apps/customer-portal/backend/ 2>/dev/null || true
# Also check the client.bal file to see the full context of line 43
echo "=== Client code context ==="
fd -name "client.bal" apps/customer-portal/backend/modules/ai_chat_agent/ -x cat -n {}Repository: wso2-open-operations/cs-tools
Length of output: 1247
🏁 Script executed:
#!/bin/bash
# Read the client.bal file directly to see line 43 context
cat apps/customer-portal/backend/modules/ai_chat_agent/client.balRepository: wso2-open-operations/cs-tools
Length of output: 1727
Path/query parameter mismatch: WebSocket upgrade will fail.
The client now builds /ws/${sessionId} as a path segment, but the server resource at service.bal:4597 is declared as:
isolated resource function get ws(http:Request req, string sessionId) returns websocket:Service|websocket:UpgradeError {In Ballerina, a parameter is a path parameter only when it appears in the resource path brackets (e.g., get ws/[string sessionId]). Since sessionId is a plain function parameter here, Ballerina treats it as a query parameter. Requests to /ws/<id> will not match the /ws resource and the WebSocket upgrade will return 404, breaking the chat flow.
Fix either the client (use query-param form: /ws?sessionId=${sessionId}) or the server resource path (add path segment):
🔧 Suggested server-side fix (service.bal ~line 4597)
- isolated resource function get ws(http:Request req, string sessionId) returns websocket:Service|websocket:UpgradeError {
+ isolated resource function get ws/[string sessionId](http:Request req) returns websocket:Service|websocket:UpgradeError {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/customer-portal/backend/modules/ai_chat_agent/client.bal` at line 43,
The client builds the WebSocket URL as a path segment which won't match the
server resource signature; update the WebSocket URL construction in the return
new(...) expression to send sessionId as a query parameter instead (use
aiChatAgentWsBaseUrl and sessionId to produce /ws?sessionId=<id>) so the server
resource function get ws(http:Request req, string sessionId) receives sessionId
correctly.
1 participant
Summary
POST /projects/{id}/conversations/searchresponse with chatbot conversation stats from the AI chat agentmessagesExchanged,troubleshootingAttempts, andkbArticlesReviewed0when the AI chat agent is unavailable or returns an error (never omitted)Changes
apps/customer-portal/backend/modules/types/types.balConversationtype:messagesExchanged— total messages exchanged in the chatbot sessiontroubleshootingAttempts— number of troubleshooting tool invocationskbArticlesReviewed— number of unique KB articles surfacedapps/customer-portal/backend/service.balPOST /projects/{id}/conversations/searchto enrich each conversation with stats fetched viaai_chat_agent:getSummary(projectId, conversationId)0for all stat fields if the chatbot call failsSummary by CodeRabbit
New Features
Chores