Skip to content

feat: add toolProtocol property to PostHog tool usage telemetry #9374

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
mrubens merged 4 commits into from
Nov 19, 2025
Merged

feat: add toolProtocol property to PostHog tool usage telemetry #9374

mrubens merged 4 commits into from
Nov 19, 2025

Conversation

@roomote
Copy link
Contributor

@roomote roomote bot commented Nov 18, 2025

This PR adds a toolProtocol property to the PostHog tool usage telemetry event to track whether tool calls are using native or XML protocol.

Changes

  • Modified TelemetryService.captureToolUsage() to accept an optional toolProtocol parameter
  • Updated presentAssistantMessage.ts to determine and pass the tool protocol when capturing tool usage
  • The protocol is determined by checking if the tool block has an ID (native protocol) or not (XML protocol)

Benefits

  • Enables tracking and analysis of tool calling protocol usage in PostHog
  • Helps understand adoption and performance of native vs XML tool calling
  • Backward compatible - the toolProtocol parameter is optional

@roomote roomote bot requested review from cte, jr and mrubens as code owners November 18, 2025 23:12
@dosubot dosubot bot added size:XS This PR changes 0-9 lines, ignoring generated files. Enhancement New feature or request labels Nov 18, 2025
@roomote
Copy link
Contributor Author

roomote bot commented Nov 18, 2025 •
edited
Loading

Rooviewer Clock   See task on Roo Cloud

Re-review complete. Previous issue has been resolved, but found one new type safety improvement.

  • Duplicate protocol detection logic in presentAssistantMessage.ts (resolved)
  • Use ToolProtocol type instead of string in TelemetryService.captureToolUsage
Previous reviews

Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues.

@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Nov 18, 2025
roomote[bot]
roomote bot commented Nov 18, 2025
View reviewed changes
@roomote
refactor: eliminate duplicate toolProtocol detection logic and make p…
97fab75 
…arameter required

- Extract toolProtocol variable once at protocol detection point (line 288)
- Remove duplicate detection logic at telemetry capture point (line 434)
- Make toolProtocol parameter required (not optional) in TelemetryService.captureToolUsage()
- Ensures consistent protocol detection and required telemetry data
mrubens
mrubens reviewed Nov 19, 2025
View reviewed changes
@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. and removed size:XS This PR changes 0-9 lines, ignoring generated files. labels Nov 19, 2025
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Nov 19, 2025
@mrubens mrubens merged commit 7c07945 into main Nov 19, 2025
16 checks passed
@mrubens mrubens deleted the feature/add-tool-protocol-telemetry branch November 19, 2025 01:29
@github-project-automation github-project-automation bot moved this from Triage to Done in Roo Code Roadmap Nov 19, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Nov 19, 2025
roomote[bot]
roomote bot commented Nov 19, 2025
View reviewed changes
packages/telemetry/src/TelemetryService.ts

public captureToolUsage(taskId: string, tool: string): void {
this.captureEvent(TelemetryEventName.TOOL_USED, { taskId, tool })
public captureToolUsage(taskId: string, tool: string, toolProtocol: string): void {
Copy link
Contributor Author

@roomote roomote bot Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The toolProtocol parameter uses string type instead of the more specific ToolProtocol type from @roo-code/types. Using the proper type would provide compile-time safety against typos or invalid values (e.g., "nativ" instead of "native"). The ToolProtocol type is already available since @roo-code/types is a dependency of this package.

Suggested change
public captureToolUsage(taskId: string, tool: string, toolProtocol: string): void {
public captureToolUsage(taskId: string, tool: string, toolProtocol: ToolProtocol): void {

Fix it with Roo Code or mention @roomote and request a fix.

mini2s added a commit to zgsm-ai/costrict that referenced this pull request Nov 19, 2025
@mini2s @heyseth
@Maosghoul @mrubens @daniel-lxs @kevint-cerebras @jr @roomote @ellipsis-dev @roomote @elianiva @hannesrudolph @github-actions @brunobergher @Thibault00 @requesty-JohnCosta27 @dmarkey @SannidhyaSah @cte @sebastiand-cerebras @dleffel @ajjuaire @renovate
Roo to main (#670