fix(mcp): remove stale stats from logs list response - #3583
Conversation
The stats field in the MCP logs list response (/api/mcp-logs) was only populated with TotalExecutions, leaving success_rate, average_latency, and total_cost as zero values - stale and misleading. Remove it from the list response struct and from the Go result assembly. UI now reads total_count from pagination (already present in response) for the paginator, and uses the dedicated /api/mcp-logs/stats endpoint for the stat cards, which computes all fields correctly. Preserves: - log rows, pagination, has_logs - /api/mcp-logs/stats endpoint - UI stat cards (they use getMCPLogsStatsQuery) Signed-off-by: Vaibhav mittal <vaibhavmittal929@gmail.com>
📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughThis PR restructures the MCP tool logs search response contract. The ChangesMCP Logs Response Refactoring
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 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)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" 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 |
Confidence Score: 5/5This PR is safe to merge. It removes a field that was already returning incorrect (all-zero) data and replaces the frontend dependency on it with a value that is correctly populated in the same response object. The change is narrow and well-contained. pagination.TotalCount is set at every return path in SearchMCPToolLogs, so the frontend switch from stats.total_executions to pagination.total_count is backed by always-present data. The dedicated stats endpoint and all other log fields are untouched. Types, OpenAPI schema, and UI are updated consistently. No files require special attention. Important Files Changed
Reviews (1): Last reviewed commit: "fix(mcp): remove stale stats from logs l..." | Re-trigger Greptile |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ui/lib/store/apis/mcpLogsApi.ts (1)
46-51: ⚡ Quick winUse the shared
MCPToolLogsResponsetype forgetMCPLogs.This endpoint re-declares a response shape already defined in shared types, which increases drift risk on future contract updates.
♻️ Proposed refactor
import { MCPToolLogEntry, MCPToolLogFilters, MCPToolLogStats, + MCPToolLogsResponse, MCPToolLogFilterData, MCPHistogramResponse, MCPCostHistogramResponse, MCPTopToolsResponse, Pagination, } from "`@/lib/types/logs`"; @@ - getMCPLogs: builder.query< - { - logs: MCPToolLogEntry[]; - pagination: Pagination & { total_count: number }; - has_logs: boolean; - }, + getMCPLogs: builder.query< + MCPToolLogsResponse, { filters: MCPToolLogFilters; pagination: Pagination; } >({As per coding guidelines, "
ui/lib/types/*.ts: Define reusable TypeScript types in shared locations (ui/lib/types/ for frontend)."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/lib/store/apis/mcpLogsApi.ts` around lines 46 - 51, The getMCPLogs query currently re-declares the response shape; update its generic type to use the shared MCPToolLogsResponse type instead of the inline object shape, and add an import for MCPToolLogsResponse from the shared types file; update any references within the getMCPLogs builder.query signature to use MCPToolLogsResponse so the endpoint consumes the single source-of-truth type.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@ui/lib/store/apis/mcpLogsApi.ts`:
- Around line 46-51: The getMCPLogs query currently re-declares the response
shape; update its generic type to use the shared MCPToolLogsResponse type
instead of the inline object shape, and add an import for MCPToolLogsResponse
from the shared types file; update any references within the getMCPLogs
builder.query signature to use MCPToolLogsResponse so the endpoint consumes the
single source-of-truth type.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e7fd8d24-c6b3-4544-8e70-044641d7b52a
📒 Files selected for processing (6)
docs/openapi/schemas/management/logging.yamlframework/logstore/rdb.goframework/logstore/tables.goui/app/workspace/mcp-logs/page.tsxui/lib/store/apis/mcpLogsApi.tsui/lib/types/logs.ts
💤 Files with no reviewable changes (2)
- docs/openapi/schemas/management/logging.yaml
- framework/logstore/tables.go
The stats field in the MCP logs list response (/api/mcp-logs) was only populated with TotalExecutions, leaving success_rate, average_latency, and total_cost as zero values - stale and misleading. Remove it from the list response struct and from the Go result assembly. UI now reads total_count from pagination (already present in response) for the paginator, and uses the dedicated /api/mcp-logs/stats endpoint for the stat cards, which computes all fields correctly. Preserves: - log rows, pagination, has_logs - /api/mcp-logs/stats endpoint - UI stat cards (they use getMCPLogsStatsQuery) Signed-off-by: Vaibhav mittal <vaibhavmittal929@gmail.com>
The stats field in the MCP logs list response (/api/mcp-logs) was only populated with TotalExecutions, leaving success_rate, average_latency, and total_cost as zero values - stale and misleading. Remove it from the list response struct and from the Go result assembly. UI now reads total_count from pagination (already present in response) for the paginator, and uses the dedicated /api/mcp-logs/stats endpoint for the stat cards, which computes all fields correctly. Preserves: - log rows, pagination, has_logs - /api/mcp-logs/stats endpoint - UI stat cards (they use getMCPLogsStatsQuery) Signed-off-by: Vaibhav mittal <vaibhavmittal929@gmail.com>
The stats field in the MCP logs list response (/api/mcp-logs) was only populated with TotalExecutions, leaving success_rate, average_latency, and total_cost as zero values - stale and misleading. Remove it from the list response struct and from the Go result assembly. UI now reads total_count from pagination (already present in response) for the paginator, and uses the dedicated /api/mcp-logs/stats endpoint for the stat cards, which computes all fields correctly. Preserves: - log rows, pagination, has_logs - /api/mcp-logs/stats endpoint - UI stat cards (they use getMCPLogsStatsQuery) Signed-off-by: Vaibhav mittal <vaibhavmittal929@gmail.com>
Summary
Removes the stale top-level
statsobject from the MCP logs list response.While testing MCP logs end-to-end,
/api/mcp-logs?limit=5returned the correct log row withstatus: successandcost: 0.5, but the top-levelstatsobject in the same response showed zero values forsuccess_rate,average_latency, andtotal_cost.The correct stats are already available from the dedicated
/api/mcp-logs/statsendpoint, which is what the MCP Logs UI cards use.Changes
statsfrom the/api/mcp-logslist response.has_logsunchanged./api/mcp-logs/statsunchanged.pagination.total_count.Type of change
Affected areas
How to test
Backend:
go test ./framework/logstore/...go test ./plugins/logging/...go test ./transports/bifrost-http/handlers/...UI:
cd uinpm run buildRuntime verification:
curl -s "http://localhost:9090/api/mcp-logs?limit=5" | jq '{has_stats: has("stats"), log_preserved: (.logs[0] | {tool_name, server_label, status, cost})}'curl -s "http://localhost:9090/api/mcp-logs/stats" | jq .Verified that:
/api/mcp-logsreturnshas_stats: falsetool_name,server_label,status, andcost/api/mcp-logs/statsstill returns correct statsScreenshots/Recordings
Added screenshots for:
has_stats: falseBreaking changes
This removes the top-level
statsfield from the/api/mcp-logslist response.Any consumer using that field should use the dedicated
/api/mcp-logs/statsendpoint instead.Related issues
N/A
Security considerations
No security impact. This does not change auth, secrets handling, log storage, or MCP tool execution behavior.
Checklist
docs/contributing/README.mdand followed the guidelines