fix(web): count canonical MCP sources in analytics - #1651
Conversation
This comment has been minimized.
This comment has been minimized.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review. WalkthroughThe change centralizes MCP source labels and updates analytics classification to include canonical and legacy MCP activity. Tests verify the generated SQL filters and category mapping. ChangesMCP analytics source classification
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The supplied change context does not identify an actionable merge risk. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 4 files. (1 skipped: 1 unsupported.)
✨ 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found and verified against the latest diff
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/web/src/ee/features/analytics/actions.test.ts">
<violation number="1" location="packages/web/src/ee/features/analytics/actions.test.ts:52">
P3: This test asserts on reconstructed SQL string fragments ("THEN 'mcp'", aliases like `c.source_category`) and plain parameter membership instead of seeding audit rows and asserting the computed classification. Any behavior-preserving refactor (renaming the `source_category` alias, restructuring the CASE, or reformatting) fails the test spuriously, and no assertion verifies that a `sourcebot-mcp-server` row is actually classified as `mcp`. Seed rows with the canonical and legacy sources and assert on the returned `rows` values so the regression test stays meaningful if the query changes.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
|
||
| expect(parameters).toContain('sourcebot-mcp-server'); | ||
| expect(parameters).toContain('mcp'); | ||
| expect(query).toContain("THEN 'mcp'"); |
There was a problem hiding this comment.
P3: This test asserts on reconstructed SQL string fragments ("THEN 'mcp'", aliases like c.source_category) and plain parameter membership instead of seeding audit rows and asserting the computed classification. Any behavior-preserving refactor (renaming the source_category alias, restructuring the CASE, or reformatting) fails the test spuriously, and no assertion verifies that a sourcebot-mcp-server row is actually classified as mcp. Seed rows with the canonical and legacy sources and assert on the returned rows values so the regression test stays meaningful if the query changes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/web/src/ee/features/analytics/actions.test.ts, line 52:
<comment>This test asserts on reconstructed SQL string fragments ("THEN 'mcp'", aliases like `c.source_category`) and plain parameter membership instead of seeding audit rows and asserting the computed classification. Any behavior-preserving refactor (renaming the `source_category` alias, restructuring the CASE, or reformatting) fails the test spuriously, and no assertion verifies that a `sourcebot-mcp-server` row is actually classified as `mcp`. Seed rows with the canonical and legacy sources and assert on the returned `rows` values so the regression test stays meaningful if the query changes.</comment>
<file context>
@@ -0,0 +1,57 @@
+
+ expect(parameters).toContain('sourcebot-mcp-server');
+ expect(parameters).toContain('mcp');
+ expect(query).toContain("THEN 'mcp'");
+ expect(query).toContain("WHERE c.source_category IN ('mcp', 'api')");
+ expect(query).toContain("WHERE c.source_category = 'mcp'");
</file context>
1 participant
Summary
sourcebot-mcp-serversource and legacymcpsource as MCP analytics activityask_codebaseon the canonical MCP sourceRegression
MCP analytics was introduced in #950 on March 4, 2026 using the
mcpsource. The shared MCP tool refactor in #1014 on March 22, 2026 changed tool audit metadata tosourcebot-mcp-serverwithout updating the analytics query, which still required an exactmcpmatch.Testing
yarn workspace @sourcebot/web eslint src/ee/features/analytics/actions.ts src/ee/features/analytics/actions.test.ts src/ee/features/mcp/constants.ts src/ee/features/mcp/server.tsyarn workspace @sourcebot/web test src/app/api/(server)/ee/mcp/route.test.ts src/features/tools/adapters.test.ts src/ee/features/analytics/actions.test.ts --runyarn workspace @sourcebot/web tsc --noEmit(blocked by existing generated.next/typeserrors in unrelated settings pages)Note
Low Risk
Analytics classification and MCP audit metadata alignment only; no auth or data-mutation paths. SQL behavior change is limited to metric bucketing for existing audit rows.
Overview
Fixes EE analytics undercounting MCP usage after audit events started using the canonical
sourcebot-mcp-serverlabel while the dashboard still only counted legacymcp.The analytics SQL now derives a
source_category(mcp,api, orsourcebot) in thecoreCTE and uses it for MCP/API/non-web metrics instead of scatteredmetadata->>'source'checks. MCP activity includes bothsourcebot-mcp-serverand legacymcp.MCP server labeling is centralized via
MCP_SERVER_SOURCE/LEGACY_MCP_SERVER_SOURCEinconstants.ts;ask_codebaseand tool telemetry now emit the canonical source consistently.Adds a Vitest regression asserting the raw analytics query binds both source values and filters on
source_category. Changelog notes the fix for 5.1.12.Reviewed by Cursor Bugbot for commit e0f376a. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Fixes MCP analytics so both the canonical
sourcebot-mcp-serverand legacymcpaudit sources are classified as MCP activity, preventing MCP usage from being undercounted after the shared MCP tool refactor.ask_codebaseto use the canonical MCP source.Written for commit e0f376a. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Consistency