From db61509ddcdc9897768a9d242855956d21d5dd5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Tue, 30 Jun 2026 10:43:34 -0700 Subject: [PATCH] fix(cli): omit render duration when feedback command has none (#1797) The standalone `feedback` command runs separately from `render`, so it has no access to the prior render's elapsed time, yet it always passed renderDurationMs: 0 to the feedback analytics event. Since that path is the one used in practice (the auto-prompt returns early for agent and non-interactive runtimes), nearly every feedback event recorded a render duration of exactly 0, which is misleading rather than absent. Make renderDurationMs optional and only include render_duration_ms in the event when a real value is supplied. The standalone command no longer passes a duration; the auto-prompt path still forwards the real elapsed time. --- packages/cli/src/commands/feedback.ts | 3 ++- packages/cli/src/telemetry/events.test.ts | 24 +++++++++++++++++++++++ packages/cli/src/telemetry/events.ts | 4 ++-- 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/packages/cli/src/commands/feedback.ts b/packages/cli/src/commands/feedback.ts index 5b71b6fe9..b353a1b25 100644 --- a/packages/cli/src/commands/feedback.ts +++ b/packages/cli/src/commands/feedback.ts @@ -42,9 +42,10 @@ export default defineCommand({ const doctorSummary = await getDoctorSummary(); + // The standalone command runs separately from `render`, so it has no real + // elapsed time to report. Omit it rather than recording a fake duration. trackRenderFeedback({ rating, - renderDurationMs: 0, comment: args.comment || undefined, doctorSummary, }); diff --git a/packages/cli/src/telemetry/events.test.ts b/packages/cli/src/telemetry/events.test.ts index 76e7d9ad5..2dd0f3802 100644 --- a/packages/cli/src/telemetry/events.test.ts +++ b/packages/cli/src/telemetry/events.test.ts @@ -11,6 +11,7 @@ const { trackRenderObservation, trackCommandFailure, trackCliError, + trackRenderFeedback, } = await import("./events.js"); describe("render telemetry events", () => { @@ -99,6 +100,29 @@ describe("render telemetry events", () => { }); }); +describe("trackRenderFeedback", () => { + beforeEach(() => { + trackEvent.mockClear(); + }); + + it("omits render_duration_ms when no duration is known (standalone feedback)", () => { + trackRenderFeedback({ rating: 4, comment: "great" }); + + const [, props] = trackEvent.mock.calls[0] as [string, Record]; + expect(props).not.toHaveProperty("render_duration_ms"); + expect(props.$survey_response).toBe(4); + }); + + it("includes render_duration_ms when a real duration is supplied", () => { + trackRenderFeedback({ rating: 5, renderDurationMs: 6000 }); + + expect(trackEvent).toHaveBeenCalledWith( + "survey sent", + expect.objectContaining({ render_duration_ms: 6000 }), + ); + }); +}); + describe("trackCliError", () => { beforeEach(() => { trackEvent.mockClear(); diff --git a/packages/cli/src/telemetry/events.ts b/packages/cli/src/telemetry/events.ts index bbcc34bf2..f444aa67a 100644 --- a/packages/cli/src/telemetry/events.ts +++ b/packages/cli/src/telemetry/events.ts @@ -350,7 +350,7 @@ export function trackTranscribeUnavailable(props: { optional: boolean }): void { export function trackRenderFeedback(props: { rating: number; - renderDurationMs: number; + renderDurationMs?: number; comment?: string; doctorSummary?: string; }): void { @@ -358,7 +358,7 @@ export function trackRenderFeedback(props: { $survey_id: "render_satisfaction", $survey_response: props.rating, ...(props.comment ? { $survey_response_2: props.comment } : {}), - render_duration_ms: props.renderDurationMs, + ...(props.renderDurationMs !== undefined ? { render_duration_ms: props.renderDurationMs } : {}), ...(props.doctorSummary ? { doctor_summary: props.doctorSummary } : {}), }); }