| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
…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>
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 reviews.auto_review.auto_pause_after_reviewed_commits setting. Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughPostHog analytics events were renamed from wa_chat_* to ask_* or consolidated into tool_used; event payloads now include an optional source where applicable; MCP and tool adapters emit unified tool_used events; search endpoints no longer emit api_code_search_request; web client sets X-Sourcebot-Client-Source on chat requests. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
❌ Failed checks (1 warning)
✏️ 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. ❤️ ShareComment @coderabbitai help to get the list of available commands and usage tips. |
Sorry, something went wrong.
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)🤖 Prompt for all review comments with AI agents60-67: Capture failure telemetry for list_language_models too.
Right now only success is tracked; thrown failures won’t be counted in tool_used.
Suggested refactor🤖 Prompt for AI Agentsasync () => { - 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; + } }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.
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.
Fix all unresolved CodeRabbit comments on this PR:
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 11a0b7d9-653b-4152-82c4-951b00e9adbd
📥 CommitsReviewing files that changed from the base of the PR and between ae4513e and acae640.
📒 Files selected for processing (11)
Sorry, something went wrong.
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 | 🟡 Minor
Missing captureEvent in dependency array.
The captureEvent function is used inside sendMessage but is not included in its dependency array (lines 164-172). This is inconsistent with the onSubmit callback which correctly includes captureEvent in its dependencies (line 331). While captureEvent is likely a stable reference, adding it ensures correctness and silences potential lint warnings.
Proposed fix🤖 Prompt for AI Agents}, [ selectedLanguageModel, _sendMessage, selectedSearchScopes, messages.length, toast, chatId, router, + captureEvent, ]);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.
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.
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 329f0f7c-0975-4892-9d07-7e5dc4481b68
📥 CommitsReviewing files that changed from the base of the PR and between acae640 and df039d0.
📒 Files selected for processing (3)
Sorry, something went wrong.
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.
packages/web/src/lib/posthogEvents.ts (1)🤖 Prompt for all review comments with AI agents171-180: ⚠️ Potential issue | 🟠 Major
Make source required on the shared ask events.
ask_thread_created and ask_message_sent are the multi-source variants, so keeping source optional here lets emitters silently skip the origin and breaks the source breakdown this PR is introducing.
Suggested schema fixask_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 the wa_ prefix" and "Multi-source PostHog events should include a source property to identify the origin (e.g., 'sourcebot-web-client', 'sourcebot-mcp-server', 'sourcebot-ask-agent')."
🤖 Prompt for AI AgentsVerify 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.
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.
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bbeb595a-dab3-4f1b-92ee-b77b2feb8bce
📥 CommitsReviewing files that changed from the base of the PR and between df039d0 and 277ed60.
📒 Files selected for processing (3)
Sorry, something went wrong.
…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.
packages/web/src/lib/posthogEvents.ts (1)🤖 Prompt for all review comments with AI agents162-172: ⚠️ Potential issue | 🟠 Major
Require source for ask_* multi-source events.
At Line 165 and Line 171, source is optional. For multi-source events this should be required to keep telemetry filters and dashboards consistent.
Suggested fixask_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 a source property".
🤖 Prompt for AI AgentsVerify 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).
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).
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 17069cf1-6a5c-47dc-8842-d6cba7873642
📥 CommitsReviewing files that changed from the base of the PR and between 818f237 and 49a1b76.
📒 Files selected for processing (3)
Sorry, something went wrong.
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>
…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>
| Back | FazBrowse Home | New Git URL |
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):
Events removed:
New behavior:
Final event inventory (for search/ask/tool workflows)
PostHog dashboard configuration
Actions (shared across all 13 company dashboards):
Any Feature Usage (ID: 258475) -- used for the "All" DAU/WAU line:
Ask Usage (ID: 263321) -- used for the "Ask" DAU/WAU line:
MCP Usage (ID: 263322) -- used for the "MCP" DAU/WAU line:
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_used total count, breakdown by toolName. Daily interval, -90d range. On all 13 dashboards. New.
Test plan