Conversation
…d client-side ask events Renames server-side chat events to drop the wa_ prefix (since they fire from multiple sources), adds a new tool_used event for unified tool call tracking across the ask agent and MCP server, and introduces client-side wa_ask_* events for accurate web-only usage dashboards. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughPostHog analytics events were renamed from Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
Add isAnonymous to wa_ask_thread_created and messageCount to wa_ask_message_sent so they carry the same data as the server-side events. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
packages/web/src/features/mcp/server.ts (1)
60-67: Capture failure telemetry forlist_language_modelstoo.Right now only success is tracked; thrown failures won’t be counted in
tool_used.Suggested refactor
async () => { - const models = await getConfiguredLanguageModelsInfo(); - captureEvent('tool_used', { - toolName: 'list_language_models', - source: 'sourcebot-mcp-server', - success: true, - }); - return { content: [{ type: "text", text: JSON.stringify(models) }] }; + try { + const models = await getConfiguredLanguageModelsInfo(); + captureEvent('tool_used', { + toolName: 'list_language_models', + source: 'sourcebot-mcp-server', + success: true, + }); + return { content: [{ type: "text", text: JSON.stringify(models) }] }; + } catch (error) { + captureEvent('tool_used', { + toolName: 'list_language_models', + source: 'sourcebot-mcp-server', + success: false, + }); + throw error; + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/features/mcp/server.ts` around lines 60 - 67, Wrap the call to getConfiguredLanguageModelsInfo inside a try/catch in the handler that currently calls captureEvent('tool_used', { toolName: 'list_language_models', ... }) so that failures are also reported; on success keep the existing captureEvent with success: true, and in the catch call captureEvent('tool_used', { toolName: 'list_language_models', source: 'sourcebot-mcp-server', success: false, error: <error message/stack> }) before rethrowing or returning an error response—locate this behavior around the getConfiguredLanguageModelsInfo invocation and the captureEvent calls to implement the telemetry on failure.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/web/src/app/api/`(server)/chat/route.ts:
- Around line 94-102: The code emits ask_message_sent even when
X-Sourcebot-Client-Source header is missing; update the block around
captureEvent to only call captureEvent('ask_message_sent', ...) when source is
defined (or set an explicit allowed default source per guidelines), ensuring the
event payload always includes the source property; reference the
variables/identifiers captureEvent, source, id, messages, expandedRepos, and
env.EXPERIMENT_ASK_GH_ENABLED so you add a guard (if (source) { ... }) or
provide a permitted default before invoking captureEvent.
In `@packages/web/src/features/chat/actions.ts`:
- Around line 52-56: The event capture for 'ask_thread_created' is sending
source optionally and may be emitted without origin metadata; update the call to
captureEvent('ask_thread_created', ...) in actions.ts to always include the
source property (use the existing source variable or a default like 'web' when
undefined) so the payload includes chatId (chat.id), isAnonymous (isGuestUser)
and source consistently; ensure captureEvent and the 'ask_thread_created'
invocation pass the non-optional source value.
In `@packages/web/src/features/tools/adapters.ts`:
- Around line 23-27: The telemetry calls to captureEvent (used in the tools
adapter around the handler where def.name and context are passed, and the other
two occurrences) are unawaited and can produce unhandled promise rejections;
update each call to await captureEvent(...) inside a try/catch (or attach
.catch) so failures are swallowed and logged without affecting tool
execution—specifically modify the captureEvent invocations referenced by
def.name, and the other two captureEvent calls in this file to be awaited with
error handling that logs the error via the existing logger instead of leaving
the promise unhandled.
In `@packages/web/src/lib/posthogEvents.ts`:
- Around line 169-179: The schema for ask_thread_created and ask_message_sent
currently marks source as optional; change both event type definitions (symbols:
ask_thread_created, ask_message_sent in posthogEvents.ts) to require source
(remove the optional ?), then update any callers that emit these events to pass
a source value so TypeScript stays satisfied; ensure related comments/validation
reflect that multi-source events must include source and avoid using the wa_
prefix if fired outside the web app.
---
Nitpick comments:
In `@packages/web/src/features/mcp/server.ts`:
- Around line 60-67: Wrap the call to getConfiguredLanguageModelsInfo inside a
try/catch in the handler that currently calls captureEvent('tool_used', {
toolName: 'list_language_models', ... }) so that failures are also reported; on
success keep the existing captureEvent with success: true, and in the catch call
captureEvent('tool_used', { toolName: 'list_language_models', source:
'sourcebot-mcp-server', success: false, error: <error message/stack> }) before
rethrowing or returning an error response—locate this behavior around the
getConfiguredLanguageModelsInfo invocation and the captureEvent calls to
implement the telemetry on failure.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 11a0b7d9-653b-4152-82c4-951b00e9adbd
📒 Files selected for processing (11)
CHANGELOG.mdCLAUDE.mdpackages/web/src/app/api/(server)/chat/route.tspackages/web/src/features/chat/actions.tspackages/web/src/features/chat/agent.tspackages/web/src/features/chat/components/chatThread/chatThread.tsxpackages/web/src/features/chat/useCreateNewChatThread.tspackages/web/src/features/mcp/askCodebase.tspackages/web/src/features/mcp/server.tspackages/web/src/features/tools/adapters.tspackages/web/src/lib/posthogEvents.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/web/src/features/chat/components/chatThread/chatThread.tsx (1)
129-172:⚠️ Potential issue | 🟡 MinorMissing
captureEventin dependency array.The
captureEventfunction is used insidesendMessagebut is not included in its dependency array (lines 164-172). This is inconsistent with theonSubmitcallback which correctly includescaptureEventin its dependencies (line 331). WhilecaptureEventis likely a stable reference, adding it ensures correctness and silences potential lint warnings.Proposed fix
}, [ selectedLanguageModel, _sendMessage, selectedSearchScopes, messages.length, toast, chatId, router, + captureEvent, ]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/features/chat/components/chatThread/chatThread.tsx` around lines 129 - 172, The callback that sends messages (the useCallback containing captureEvent, _sendMessage and generateAndUpdateChatNameFromMessage) uses captureEvent but doesn't list it in its dependency array; update that dependency array to include captureEvent so React hooks are correct and lint warnings are silenced—add captureEvent alongside selectedLanguageModel, _sendMessage, selectedSearchScopes, messages.length, toast, chatId, and router.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/web/src/features/chat/components/chatThread/chatThread.tsx`:
- Around line 129-172: The callback that sends messages (the useCallback
containing captureEvent, _sendMessage and generateAndUpdateChatNameFromMessage)
uses captureEvent but doesn't list it in its dependency array; update that
dependency array to include captureEvent so React hooks are correct and lint
warnings are silenced—add captureEvent alongside selectedLanguageModel,
_sendMessage, selectedSearchScopes, messages.length, toast, chatId, and router.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 329f0f7c-0975-4892-9d07-7e5dc4481b68
📒 Files selected for processing (3)
packages/web/src/features/chat/components/chatThread/chatThread.tsxpackages/web/src/features/chat/useCreateNewChatThread.tspackages/web/src/lib/posthogEvents.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/web/src/features/chat/useCreateNewChatThread.ts
- packages/web/src/lib/posthogEvents.ts
This event is fully covered by api_request which fires for every API call and carries path + source. Updated the Any Feature Usage PostHog action to remove the api_code_search_request step. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Redundant with ask_thread_created which fires server-side for the same thread creation (including from the web UI via createChat server action). Dashboards track ask usage via wa_ask_message_sent, not thread creation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/web/src/lib/posthogEvents.ts (1)
171-180:⚠️ Potential issue | 🟠 MajorMake
sourcerequired on the shared ask events.
ask_thread_createdandask_message_sentare the multi-source variants, so keepingsourceoptional here lets emitters silently skip the origin and breaks the source breakdown this PR is introducing.Suggested schema fix
ask_thread_created: { chatId: string, isAnonymous: boolean, - source?: string, + source: string, }, ask_message_sent: { chatId: string, messageCount: number, selectedReposCount: number, - source?: string, + source: string, /**Based on learnings, "PostHog events using the
wa_prefix can ONLY be fired from the web app; events fired from multiple sources must NOT use thewa_prefix" and "Multi-source PostHog events should include asourceproperty to identify the origin (e.g., 'sourcebot-web-client', 'sourcebot-mcp-server', 'sourcebot-ask-agent')."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/lib/posthogEvents.ts` around lines 171 - 180, The event schemas ask_thread_created and ask_message_sent currently define source as optional, which allows callers to omit it and breaks multi-source analytics; update the type definitions for ask_thread_created and ask_message_sent to make source a required string (remove the optional marker) so every emitter must supply a source value (e.g., 'sourcebot-web-client' etc.), and run a quick grep for other "ask_" multi-source events to ensure they also require source if shared across origins.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/web/src/lib/posthogEvents.ts`:
- Around line 171-180: The event schemas ask_thread_created and ask_message_sent
currently define source as optional, which allows callers to omit it and breaks
multi-source analytics; update the type definitions for ask_thread_created and
ask_message_sent to make source a required string (remove the optional marker)
so every emitter must supply a source value (e.g., 'sourcebot-web-client' etc.),
and run a quick grep for other "ask_" multi-source events to ensure they also
require source if shared across origins.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bbeb595a-dab3-4f1b-92ee-b77b2feb8bce
📒 Files selected for processing (3)
packages/web/src/app/api/(server)/search/route.tspackages/web/src/app/api/(server)/stream_search/route.tspackages/web/src/lib/posthogEvents.ts
💤 Files with no reviewable changes (2)
- packages/web/src/app/api/(server)/search/route.ts
- packages/web/src/app/api/(server)/stream_search/route.ts
…transport Instead of a separate client-side event, the web client now sets the X-Sourcebot-Client-Source header on DefaultChatTransport. This makes ask_message_sent fire with source='sourcebot-web-client' for web UI calls, enabling web-only filtering directly on the server-side event. Updated Ask Usage and Any Feature Usage PostHog actions to filter ask_message_sent by source=sourcebot-web-client. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/web/src/lib/posthogEvents.ts (1)
162-172:⚠️ Potential issue | 🟠 MajorRequire
sourceforask_*multi-source events.At Line 165 and Line 171,
sourceis optional. For multi-source events this should be required to keep telemetry filters and dashboards consistent.Suggested fix
ask_thread_created: { chatId: string, isAnonymous: boolean, - source?: string, + source: string, }, ask_message_sent: { chatId: string, messageCount: number, selectedReposCount: number, - source?: string, + source: string, /**As per coding guidelines, "Events fired from multiple sources (web app, MCP server, API) must NOT use the
wa_prefix. Multi-source events should include asourceproperty".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/lib/posthogEvents.ts` around lines 162 - 172, The types for the multi-source events ask_thread_created and ask_message_sent currently mark source as optional; change their definitions so source is required (remove the `?` on `source`) in the ask_thread_created and ask_message_sent event shapes, update any call sites that construct these events to supply a `source` value, and run TS to fix any resulting type errors (look for usages of ask_thread_created and ask_message_sent to add the appropriate source strings).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/web/src/lib/posthogEvents.ts`:
- Around line 162-172: The types for the multi-source events ask_thread_created
and ask_message_sent currently mark source as optional; change their definitions
so source is required (remove the `?` on `source`) in the ask_thread_created and
ask_message_sent event shapes, update any call sites that construct these events
to supply a `source` value, and run TS to fix any resulting type errors (look
for usages of ask_thread_created and ask_message_sent to add the appropriate
source strings).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 17069cf1-6a5c-47dc-8842-d6cba7873642
📒 Files selected for processing (3)
CHANGELOG.mdpackages/web/src/features/chat/components/chatThread/chatThread.tsxpackages/web/src/lib/posthogEvents.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- CHANGELOG.md
- packages/web/src/features/chat/components/chatThread/chatThread.tsx
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…nt calls
Add .catch(() => {}) to all fire-and-forget captureEvent('tool_used')
calls in adapters.ts and server.ts, matching the existing pattern in
apiHandler.ts. Prevents unhandled promise rejections if telemetry fails.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
4817d7e to
39aaac0
Compare
…l telemetry captureEvent() relies on tryGetPostHogDistinctId() which reads cookies, session, or API key headers. In the MCP server and ask agent contexts, none of these are available, so every tool_used event got a random distinct_id — inflating DAU counts (e.g., 8 tool calls = 8 "users"). Fix: add optional distinctId to ToolContext and captureEvent(). Thread user.id from the auth context through: - MCP route → createMcpServer(userId) → ToolContext.distinctId - chat route / askCodebase → createMessageStream(distinctId) → createAgentStream → createTools Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The field carries a user ID, not a PostHog-specific concept. The captureEvent options parameter remains distinctId since that's the PostHog term, but the application-level interfaces use userId. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ed event Match the same pattern used in toVercelAITool — single captureEvent call in a finally block with a success boolean, instead of duplicated calls in try and catch. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
captureEvent now wraps its body in try/catch so callers are guaranteed it won't throw. Removed all .catch() handlers from call sites in adapters.ts, server.ts, and apiHandler.ts. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
tryGetPostHogDistinctId() was missing OAuth Bearer token resolution, causing all MCP tool calls to get random distinct_ids (inflated DAU). Fix: replace the manual cookie/session/API key checks with a call to getAuthenticatedUser(), which already handles all auth methods (session, OAuth, API keys). This removes the need to thread userId through ToolContext, agent options, and all callers. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
Renames server-side chat events, adds unified tool usage tracking, sets the source header on the web client chat transport, and cleans up redundant events.
Code changes
Events renamed (dropped
wa_prefix since they fire from multiple sources):wa_chat_thread_created->ask_thread_created(now includessourceproperty)wa_chat_message_sent->ask_message_sent(now includessourceproperty)wa_chat_tool_used->tool_used(moved from agent.tsonStepFinishto the shared tool adapter layer)Events removed:
api_code_search_request-- redundant withapi_requestwhich fires for every API call and carriespath+sourceNew behavior:
tool_usedfires from the shared adapter layer (adapters.ts) for every tool call, covering both ask agent (source: 'sourcebot-ask-agent') and MCP server (source: 'sourcebot-mcp-server') pathstool_usedalso fires for manually registered MCP tools (list_language_models,ask_codebase) inserver.tsDefaultChatTransportnow setsX-Sourcebot-Client-Source: sourcebot-web-clientheader, soask_message_sentfires withsource: 'sourcebot-web-client'for web UI callsCLAUDE.mdFinal event inventory (for search/ask/tool workflows)
search_finishedsearchResultsPage.tsxapi_requestapiHandler.ts(every API route)path,method,sourceask_thread_createdactions.ts,askCodebase.tschatId,isAnonymous,sourceask_message_sentchat/route.ts,askCodebase.tschatId,messageCount,selectedReposCount,sourcetool_usedadapters.ts,server.tstoolName,source,successPostHog dashboard configuration
Actions (shared across all 13 company dashboards):
Any Feature Usage (ID: 258475) -- used for the "All" DAU/WAU line:
search_finished(web search)wa_chat_message_sent(legacy ask, backwards compat)ask_message_sentwheresource = 'sourcebot-web-client'(new ask)api_requestwheresource = 'mcp'(external MCP npm package HTTP calls)tool_usedwheresource = 'sourcebot-mcp-server'(in-process MCP tool calls)Ask Usage (ID: 263321) -- used for the "Ask" DAU/WAU line:
wa_chat_message_sent(legacy, backwards compat)ask_message_sentwheresource = 'sourcebot-web-client'(new)MCP Usage (ID: 263322) -- used for the "MCP" DAU/WAU line:
api_requestwheresource = 'mcp'(external npm package HTTP calls)tool_usedwheresource = 'sourcebot-mcp-server'(in-process MCP tool calls)Insights:
Feature DAU (ID: 7539703) -- 4 series: All (Any Feature Usage action), Search (
search_finished), Ask (Ask Usage action), MCP (MCP Usage action). Daily interval, -90d range. On all 13 dashboards.Feature WAU (ID: 7539704) -- Same as DAU but weekly interval.
Tool Usage (ID: 7919953) --
tool_usedtotal count, breakdown bytoolName. Daily interval, -90d range. On all 13 dashboards. New.Test plan
ask_message_sentfires withsource: 'sourcebot-web-client'when sending a message from the web chat UIask_message_sentfires withsource: 'mcp'when using the in-process MCPask_codebasetoolask_thread_createdfires for both web UI and MCP/API thread creationtool_usedevents fire with correcttoolNameandsourcefor both ask agent and MCP tool callswa_chat_thread_created,wa_chat_message_sent,wa_chat_tool_used,api_code_search_request-- should only appear in CHANGELOG.md