diff --git a/packages/studio/src/components/EditorShell.tsx b/packages/studio/src/components/EditorShell.tsx index 3e0079a13..1bccf8eb9 100644 --- a/packages/studio/src/components/EditorShell.tsx +++ b/packages/studio/src/components/EditorShell.tsx @@ -8,7 +8,6 @@ import { } from "./nle/useTimelineEditCallbacks"; import { NLEProvider, useNLEContext } from "./nle/NLEContext"; import { CaptionTimeline } from "../captions/components/CaptionTimeline"; -import { StudioFeedbackBar } from "./StudioFeedbackBar"; import { useStudioPlaybackContext, useStudioShellContext } from "../contexts/StudioContext"; import { useDomEditActionsContext, useDomEditSelectionContext } from "../contexts/DomEditContext"; import { TimelineEditProvider } from "../contexts/TimelineEditContext"; @@ -189,7 +188,6 @@ export function EditorShell({ /> - ); } diff --git a/packages/studio/src/components/StudioErrorBoundary.test.tsx b/packages/studio/src/components/StudioErrorBoundary.test.tsx new file mode 100644 index 000000000..e18f01568 --- /dev/null +++ b/packages/studio/src/components/StudioErrorBoundary.test.tsx @@ -0,0 +1,97 @@ +// @vitest-environment happy-dom + +import React, { act } from "react"; +import { createRoot } from "react-dom/client"; +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; + +vi.mock("../utils/studioTelemetry", () => ({ trackStudioEvent: vi.fn() })); +vi.mock("../telemetry/policy", () => ({ browserTelemetryAllowed: () => true })); +vi.mock("../telemetry/events", () => ({ + trackStudioFeedback: vi.fn(), + trackStudioFeedbackShown: vi.fn(), + trackStudioFeedbackDismissed: vi.fn(), + trackStudioFeedbackInterviewClick: vi.fn(), +})); + +const { StudioErrorBoundary } = await import("./StudioErrorBoundary"); +const { trackStudioFeedbackShown } = await import("../telemetry/events"); + +function Boom(): React.ReactElement { + throw new Error("timeline exploded"); +} + +let container: HTMLDivElement; +let root: ReturnType; +let consoleError: ReturnType; + +beforeEach(() => { + localStorage.clear(); + sessionStorage.clear(); + vi.clearAllMocks(); + container = document.createElement("div"); + document.body.appendChild(container); + root = createRoot(container); + // React logs the caught error itself; the boundary is the thing under test. + consoleError = vi.spyOn(console, "error").mockImplementation(() => {}); +}); + +afterEach(() => { + act(() => root.unmount()); + container.remove(); + consoleError.mockRestore(); +}); + +const renderCrashed = () => + act(() => { + root.render( + + + , + ); + }); + +describe("StudioErrorBoundary", () => { + it("shows the crash screen with the error message and both recovery paths", () => { + renderCrashed(); + expect(container.textContent).toContain("Something went wrong"); + expect(container.textContent).toContain("timeline exploded"); + expect(container.textContent).toContain("Try again"); + expect(container.textContent).toContain("Reload Studio"); + }); + + it("asks what the user was doing, since the stack trace cannot say", () => { + renderCrashed(); + const card = container.querySelector('[aria-label="Send feedback to the HyperFrames team"]'); + expect(card).not.toBeNull(); + expect(card?.textContent).toContain("Studio crashed. What were you doing?"); + }); + + it("offers one-tap answers instead of a 0-10 score", () => { + renderCrashed(); + const card = container.querySelector('[aria-label="Send feedback to the HyperFrames team"]'); + const chips = [...(card?.querySelectorAll("button") ?? [])] + .map((b) => b.textContent?.trim()) + .filter((t) => t && t !== "Send"); + expect(chips).toContain("Editing the timeline"); + expect(chips).toContain("Just opened it"); + // Rating a crash is a question with no useful answer. + expect(card?.querySelector('input[name="hf-studio-feedback-rating"]')).toBeNull(); + }); + + it("reports the crash prompt to telemetry so the funnel is visible", () => { + renderCrashed(); + expect(trackStudioFeedbackShown).toHaveBeenCalledWith( + expect.objectContaining({ reason: "crash" }), + ); + }); + + it("stays quiet when the user already gave feedback recently", () => { + localStorage.setItem("hyperframes-studio:feedbackAnsweredAt", String(Date.now())); + renderCrashed(); + // The crash screen itself still works; only the ask is suppressed. + expect(container.textContent).toContain("Something went wrong"); + expect( + container.querySelector('[aria-label="Send feedback to the HyperFrames team"]'), + ).toBeNull(); + }); +}); diff --git a/packages/studio/src/components/StudioErrorBoundary.tsx b/packages/studio/src/components/StudioErrorBoundary.tsx index 15228b5cc..5a5924aa0 100644 --- a/packages/studio/src/components/StudioErrorBoundary.tsx +++ b/packages/studio/src/components/StudioErrorBoundary.tsx @@ -1,5 +1,6 @@ import { Component, type ErrorInfo, type ReactNode } from "react"; import { trackStudioEvent } from "../utils/studioTelemetry"; +import { CrashFeedbackPrompt } from "./feedback/CrashFeedbackPrompt"; interface Props { children: ReactNode; @@ -51,6 +52,12 @@ export class StudioErrorBoundary extends Component { Reload Studio + {/* The crash report tells us what broke; only the user can tell us what + they were doing when it did. This is also the one screen where they + have nothing else to get on with. */} +
+ +
); } diff --git a/packages/studio/src/components/StudioFeedbackBar.tsx b/packages/studio/src/components/StudioFeedbackBar.tsx deleted file mode 100644 index 7084c2db9..000000000 --- a/packages/studio/src/components/StudioFeedbackBar.tsx +++ /dev/null @@ -1,217 +0,0 @@ -import { memo, useState, useCallback, useRef, useEffect } from "react"; -import { trackStudioFeedback } from "../telemetry/events"; - -const DEFAULT_FEEDBACK_INTERVAL = 10; -const AUTO_DISMISS_MS = 20_000; - -function isFeedbackDisabled(): boolean { - try { - return import.meta.env.VITE_HYPERFRAMES_NO_FEEDBACK === "1"; - } catch { - return false; - } -} - -// fallow-ignore-next-line complexity -function getFeedbackInterval(): number { - try { - const v = import.meta.env.VITE_HYPERFRAMES_FEEDBACK_INTERVAL as string | undefined; - if (v) { - const n = parseInt(v, 10); - if (Number.isFinite(n) && n > 0) return n; - } - } catch { - // import.meta.env unavailable - } - return DEFAULT_FEEDBACK_INTERVAL; -} - -const STORAGE_KEYS = { - sessionCount: "hyperframes-studio:feedbackSessionCount", - lastPromptedAt: "hyperframes-studio:feedbackLastPromptedAt", -} as const; - -// fallow-ignore-next-line complexity -function shouldShowFeedback(): boolean { - if (isFeedbackDisabled()) return false; - try { - const count = parseInt(localStorage.getItem(STORAGE_KEYS.sessionCount) || "0", 10) || 0; - const lastAt = parseInt(localStorage.getItem(STORAGE_KEYS.lastPromptedAt) || "0", 10) || 0; - return count - lastAt >= getFeedbackInterval(); - } catch { - return false; - } -} - -const SESSION_COUNTED_KEY = "hyperframes-studio:feedbackSessionCounted"; - -// fallow-ignore-next-line complexity -function incrementSessionCount(): void { - try { - if (sessionStorage.getItem(SESSION_COUNTED_KEY)) return; - sessionStorage.setItem(SESSION_COUNTED_KEY, "1"); - const count = parseInt(localStorage.getItem(STORAGE_KEYS.sessionCount) || "0", 10) || 0; - localStorage.setItem(STORAGE_KEYS.sessionCount, String(count + 1)); - } catch { - // storage unavailable - } -} - -function markPrompted(): void { - try { - const count = localStorage.getItem(STORAGE_KEYS.sessionCount) || "0"; - localStorage.setItem(STORAGE_KEYS.lastPromptedAt, count); - } catch { - // localStorage unavailable - } -} - -// fallow-ignore-next-line complexity -export const StudioFeedbackBar = memo(function StudioFeedbackBar() { - const [visible, setVisible] = useState(false); - const [entered, setEntered] = useState(false); - const [rating, setRating] = useState(null); - const [comment, setComment] = useState(""); - const [submitted, setSubmitted] = useState(false); - const [exiting, setExiting] = useState(false); - const inputRef = useRef(null); - const dismissTimerRef = useRef | null>(null); - - // On mount: increment session count, check if we should show - useEffect(() => { - incrementSessionCount(); - // Small delay so the bar doesn't flash on page load - const showTimer = setTimeout(() => { - if (shouldShowFeedback()) { - setVisible(true); - } - }, 3000); - return () => clearTimeout(showTimer); - }, []); - - // Animate height in on entrance — appearing 3s after load, an instant 32px - // bar shoves the whole preview stack up mid-task. - useEffect(() => { - if (!visible) return; - const raf = requestAnimationFrame(() => setEntered(true)); - return () => cancelAnimationFrame(raf); - }, [visible]); - - // Auto-dismiss timer — reset when user interacts (sets rating) - useEffect(() => { - if (!visible || rating !== null || submitted) return; - dismissTimerRef.current = setTimeout(() => { - handleDismiss(); - }, AUTO_DISMISS_MS); - return () => { - if (dismissTimerRef.current) clearTimeout(dismissTimerRef.current); - }; - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [visible, rating, submitted]); - - // Focus text input when rating is selected - useEffect(() => { - if (rating !== null && inputRef.current) { - inputRef.current.focus(); - } - }, [rating]); - - const handleDismiss = useCallback(() => { - setExiting(true); - markPrompted(); - setTimeout(() => setVisible(false), 300); - }, []); - - const handleSubmit = useCallback(() => { - if (rating === null) return; - trackStudioFeedback({ - rating, - comment: comment.trim() || undefined, - }); - setSubmitted(true); - markPrompted(); - setTimeout(() => { - setExiting(true); - setTimeout(() => setVisible(false), 300); - }, 1500); - }, [rating, comment]); - - const handleRating = useCallback((n: number) => { - setRating(n); - // Cancel auto-dismiss — user is engaged - if (dismissTimerRef.current) { - clearTimeout(dismissTimerRef.current); - dismissTimerRef.current = null; - } - }, []); - - if (!visible) return null; - - return ( -
- {submitted ? ( - Thanks for the feedback! - ) : rating !== null ? ( - <> - setComment(e.target.value)} - onKeyDown={(e) => { - if (e.key === "Enter") handleSubmit(); - if (e.key === "Escape") handleDismiss(); - }} - placeholder="Any details? (enter to send, esc to close)" - className="flex-1 bg-transparent border-none text-[11px] text-neutral-300 placeholder-neutral-600 outline-none" - maxLength={500} - /> - - - ) : ( - <> - Recommend HyperFrames? -
- {Array.from({ length: 11 }, (_, n) => n).map((n) => ( - - ))} -
-
- - - )} -
- ); -}); diff --git a/packages/studio/src/components/StudioOverlays.tsx b/packages/studio/src/components/StudioOverlays.tsx index 411f124d5..253bcb210 100644 --- a/packages/studio/src/components/StudioOverlays.tsx +++ b/packages/studio/src/components/StudioOverlays.tsx @@ -3,6 +3,7 @@ import { LintModal } from "./LintModal"; import { AskAgentModal } from "./AskAgentModal"; import { StudioGlobalDragOverlay } from "./StudioGlobalDragOverlay"; import { StudioToast } from "./StudioToast"; +import { StudioFeedbackCard } from "./feedback/StudioFeedbackCard"; import { buildAgentContextPreview } from "./editor/domEditingAgentPrompt"; import type { useDomEditSession } from "../hooks/useDomEditSession"; import type { useToast } from "../hooks/useToast"; @@ -78,19 +79,20 @@ export function StudioOverlays({ /> )} {dragOverlayActive && } - {toasts.length > 0 && ( -
- {toasts.map((toast) => ( - dismissToast(toast.id)} - /> - ))} -
- )} + {/* One bottom-right stack so the feedback card and toasts queue instead + of covering each other. Empty when nothing is showing. */} +
+ {toasts.map((toast) => ( + dismissToast(toast.id)} + /> + ))} + +
); } diff --git a/packages/studio/src/components/feedback/CrashFeedbackPrompt.tsx b/packages/studio/src/components/feedback/CrashFeedbackPrompt.tsx new file mode 100644 index 000000000..6323b5e9b --- /dev/null +++ b/packages/studio/src/components/feedback/CrashFeedbackPrompt.tsx @@ -0,0 +1,27 @@ +import { useEffect } from "react"; +import { StudioFeedbackCard } from "./StudioFeedbackCard"; +import { requestStudioFeedback } from "./feedbackTrigger"; + +/** + * The feedback prompt for the crash screen. + * + * A crash unmounts the whole editor, and with it the card that normally lives + * in the overlay stack, so the request cannot come from there. The error + * boundary's fallback renders this instead: the card mounts and subscribes + * first (child effects run before the parent's), then the effect below asks. + * + * The crash screen is the one place a user is guaranteed to have an opinion + * and nothing else to do with it. Everything else about the ask is unchanged — + * same eligibility, same cooldowns — so a crash cannot be used to talk to + * someone who already said their piece this session. + */ +export function CrashFeedbackPrompt() { + useEffect(() => { + // No `detail`: the crash screen already prints the error message directly + // above this card, and the crash event already carries it. Passing it here + // would quote the same sentence back to the user twice. + requestStudioFeedback({ reason: "crash" }); + }, []); + + return ; +} diff --git a/packages/studio/src/components/feedback/StudioFeedbackCard.tsx b/packages/studio/src/components/feedback/StudioFeedbackCard.tsx new file mode 100644 index 000000000..4158deb88 --- /dev/null +++ b/packages/studio/src/components/feedback/StudioFeedbackCard.tsx @@ -0,0 +1,373 @@ +import { memo, useState, useCallback, useRef, useEffect } from "react"; +import { + trackStudioFeedback, + trackStudioFeedbackShown, + trackStudioFeedbackDismissed, + trackStudioFeedbackInterviewClick, +} from "../../telemetry/events"; +import { + subscribeToFeedbackRequests, + markFeedbackAnswered, + markFeedbackDismissed, + followUpFor, + isProblemReason, + type FeedbackRequest, + type FollowUpQuestion, +} from "./feedbackTrigger"; +import { projectProvenance } from "./projectProvenance"; + +/** A failed render is its own question; nothing is rotated in front of it. */ +const FAILURE_FOLLOW_UP: FollowUpQuestion = { + id: "failure", + prompt: "That export failed.", + placeholder: "What were you exporting?", + presets: [ + { label: "Happens every time", hint: "Every export of this project fails this way" }, + { label: "First time", hint: "Exports normally work for me" }, + { label: "Long composition", hint: "Over a minute, or a lot of scenes" }, + { label: "Big media files", hint: "Large video or image assets in the project" }, + { label: "Has audio", hint: "The composition includes voiceover, music or sound" }, + { label: "Embedded video", hint: "One or more video clips inside the composition" }, + ], +}; + +/** + * The crash screen ask. "What were you doing" beats "what went wrong": the + * user cannot see the stack trace, but they know exactly what they touched, + * and that is the half the crash report is missing. + */ +const CRASH_FOLLOW_UP: FollowUpQuestion = { + id: "crash", + prompt: "Studio crashed. What were you doing?", + placeholder: "The last thing you touched", + presets: [ + { label: "Editing the timeline", hint: "Dragging, trimming or splitting clips" }, + { label: "Playing the preview", hint: "Scrubbing or playing back the composition" }, + { label: "Adding media", hint: "Importing video, images or audio" }, + { label: "Editing properties", hint: "Changing styles, transforms or keyframes" }, + { label: "Exporting", hint: "Starting or watching a render" }, + { label: "Just opened it", hint: "It broke before I did anything" }, + ], +}; + +/** Long enough to read the render result first; short enough to stay polite. */ +const AUTO_DISMISS_MS = 30_000; +/** Matches the .hf-toast-exit animation so the node leaves as it finishes. */ +const EXIT_MS = 160; +/** + * The thanks state carries the interview link, so it has to outlast a glance. + * A 1.4s confirmation would flash a booking link nobody could reach. + */ +const THANKS_MS = 10_000; + +/** + * Someone who just answered is the likeliest person in the product to say yes + * to a call, and this is the only moment we have their attention with goodwill. + */ +const INTERVIEW_URL = "https://calendar.app.google/yRHT7oPsHWcqFfFv5"; + +const RATINGS = Array.from({ length: 11 }, (_, n) => n); + +type Step = "rating" | "comment" | "thanks"; + +/** + * The Studio feedback prompt. Shown by `feedbackTrigger` after a render + * finishes or fails — never on a timer, so the user always knows what they are + * being asked about. + * + * A failed render skips the 0-10 scale: scoring an export you never got is a + * question with no useful answer, and the free-text reply is the whole point. + */ +// fallow-ignore-next-line complexity +export const StudioFeedbackCard = memo(function StudioFeedbackCard() { + const [request, setRequest] = useState(null); + const [step, setStep] = useState("rating"); + const [rating, setRating] = useState(null); + const [followUp, setFollowUp] = useState(FAILURE_FOLLOW_UP); + const [comment, setComment] = useState(""); + /** Explanation of the chip under the pointer, shown on the reserved line. */ + const [hint, setHint] = useState(null); + const [exiting, setExiting] = useState(false); + const inputRef = useRef(null); + const timerRef = useRef | null>(null); + // Read inside close(), which must not re-subscribe every keystroke. + const stateRef = useRef({ request, rating, step }); + stateRef.current = { request, rating, step }; + + const clearTimer = useCallback(() => { + if (timerRef.current) { + clearTimeout(timerRef.current); + timerRef.current = null; + } + }, []); + + const close = useCallback( + (via: "close" | "escape" | "timeout" | "sent") => { + const { request: req, rating: picked, step: at } = stateRef.current; + if (!req) return; + clearTimer(); + if (via === "sent") { + markFeedbackAnswered(); + } else { + markFeedbackDismissed(); + trackStudioFeedbackDismissed({ + reason: req.reason, + render_id: req.renderId, + via, + had_rating: picked !== null || at === "comment", + }); + } + setExiting(true); + setTimeout(() => { + setRequest(null); + setExiting(false); + setRating(null); + setComment(""); + setStep("rating"); + }, EXIT_MS); + }, + [clearTimer], + ); + + useEffect( + () => + subscribeToFeedbackRequests((next) => { + setRequest(next); + setRating(null); + setComment(""); + setHint(null); + setFollowUp(next.reason === "crash" ? CRASH_FOLLOW_UP : FAILURE_FOLLOW_UP); + // A crash or a failed render has nothing to score, so those open + // straight at the ask instead of demanding a number first. + setStep(isProblemReason(next.reason) ? "comment" : "rating"); + trackStudioFeedbackShown({ reason: next.reason, render_id: next.renderId }); + }), + [], + ); + + // Auto-dismiss only while the prompt is still untouched — once the user + // starts typing or picks a score, the card is theirs until they close it. + useEffect(() => { + if (!request || step === "thanks" || rating !== null || comment !== "") return; + timerRef.current = setTimeout(() => close("timeout"), AUTO_DISMISS_MS); + return clearTimer; + }, [request, step, rating, comment, close, clearTimer]); + + useEffect(() => { + if (step === "comment") inputRef.current?.focus(); + }, [step]); + + /** `preset` is the tapped chip's text; absent means the field was typed in. */ + const submit = useCallback( + (preset?: string) => { + const req = stateRef.current.request; + if (!req) return; + const text = (preset ?? comment).trim(); + // Nothing to send yet: a failure prompt with no words is just a dismiss. + if (rating === null && text === "") return close("close"); + trackStudioFeedback({ + reason: req.reason, + render_id: req.renderId, + // Which follow-up this comment answers. Without it, "nothing" against + // "what would you cut?" and "nothing" against "what should we fix?" are + // the same row, and neither means anything. + question: followUp.id, + // Tapped chips cluster into countable buckets; typed answers do not. + // Mixing them would make a chip's popularity look like a written theme. + answer_kind: preset === undefined ? "typed" : "preset", + // Provenance first so a producer's own context wins on a key clash. + context: { ...projectProvenance(), ...req.context }, + ...(rating === null ? {} : { rating }), + ...(text ? { comment: text } : {}), + }); + clearTimer(); + setStep("thanks"); + setTimeout(() => close("sent"), THANKS_MS); + }, + [rating, comment, followUp.id, close, clearTimer], + ); + + const pickRating = useCallback((n: number) => { + setRating(n); + setFollowUp(followUpFor(n)); + setStep("comment"); + }, []); + + if (!request) return null; + + const failed = isProblemReason(request.reason); + // Deliberately the CLI's question, word for word. Studio and CLI ratings only + // sit on one axis if they asked the same thing; what makes them comparable is + // `reason`, which records the moment, not a reworded prompt. + const title = + step === "rating" ? "How likely are you to recommend HyperFrames?" : followUp.prompt; + + return ( +
{ + if (e.key === "Escape") close("escape"); + }} + > +
+
+

+ {step === "thanks" ? "Thanks, that helps." : title} +

+ {step === "thanks" && ( + { + const req = stateRef.current.request; + if (req) trackStudioFeedbackInterviewClick({ reason: req.reason }); + close("sent"); + }} + className="h-6 flex-shrink-0 whitespace-nowrap rounded-full border border-white/20 bg-white/[0.08] px-2.5 text-[11px] leading-6 text-neutral-100 transition-[background-color,border-color,transform] duration-150 ease-out hover:border-white/35 hover:bg-white/[0.16] active:scale-[0.97] motion-reduce:transition-none" + > + Talk to us, 30 min + + )} + +
+ + {/* The failure text the user already saw, quoted back so the report + carries it without asking them to retype the error. */} + {failed && request.detail && step !== "thanks" && ( +

{request.detail}

+ )} + + {step === "rating" && ( + <> + {/* Native radios: free arrow-key navigation, grouping and labels. */} +
+ Rate this export from 0 to 10 + {RATINGS.map((n) => ( + + ))} +
+
+ Not likely + Extremely likely +
+ + )} + + {step === "comment" && ( + <> + {/* One tap is the whole answer. Sends immediately: making someone + tap a chip and then hunt for Send throws away the reason chips + exist. The field below stays for anything the chips miss. */} +
+ {followUp.presets.map((preset) => ( + + ))} +
+ {/* Inline, not a floating tooltip. The card is 340px in a corner, + so a bubble above the chips lands on top of the question and a + bubble below lands on the input. One reserved line occludes + nothing and needs no hover delay to be readable. */} +

+ {hint ?? "Tap one, or write your own."} +

+
+ setComment(e.target.value)} + onKeyDown={(e) => { + if (e.key === "Enter") submit(); + }} + placeholder={followUp.placeholder} + aria-label={followUp.prompt} + maxLength={500} + className="h-7 min-w-0 flex-1 rounded-md border border-white/10 bg-black/20 px-2 text-[11px] text-neutral-100 outline-none transition-colors duration-150 placeholder:text-neutral-500 focus:border-white/25" + /> + +
+ + )} +
+
+ ); +}); diff --git a/packages/studio/src/components/feedback/feedbackTrigger.test.ts b/packages/studio/src/components/feedback/feedbackTrigger.test.ts new file mode 100644 index 000000000..a6f70abd2 --- /dev/null +++ b/packages/studio/src/components/feedback/feedbackTrigger.test.ts @@ -0,0 +1,168 @@ +// @vitest-environment happy-dom + +import { describe, it, expect, beforeEach, vi, afterEach } from "vitest"; + +// The policy module decides whether this profile may be measured at all; the +// trigger refuses to ask anyone it cannot hear, so every test here has to say +// which of those two worlds it is in. +vi.mock("../../telemetry/policy", () => ({ + browserTelemetryAllowed: () => telemetryAllowed, +})); + +let telemetryAllowed = true; + +const { + requestStudioFeedback, + subscribeToFeedbackRequests, + markFeedbackAnswered, + markFeedbackDismissed, + followUpFor, + isProblemReason, +} = await import("./feedbackTrigger"); + +const DAY_MS = 86_400_000; + +function collect() { + const seen: unknown[] = []; + const unsubscribe = subscribeToFeedbackRequests((r) => seen.push(r)); + return { seen, unsubscribe }; +} + +beforeEach(() => { + telemetryAllowed = true; + localStorage.clear(); + sessionStorage.clear(); + vi.useRealTimers(); +}); + +afterEach(() => { + vi.useRealTimers(); +}); + +describe("requestStudioFeedback eligibility", () => { + it("asks on the first eligible request", () => { + const { seen, unsubscribe } = collect(); + requestStudioFeedback({ reason: "render_complete", renderId: "r1" }); + expect(seen).toEqual([{ reason: "render_complete", renderId: "r1" }]); + unsubscribe(); + }); + + it("asks only once per tab session, however many renders finish", () => { + const { seen, unsubscribe } = collect(); + requestStudioFeedback({ reason: "render_complete", renderId: "r1" }); + requestStudioFeedback({ reason: "render_complete", renderId: "r2" }); + requestStudioFeedback({ reason: "render_failed", renderId: "r3" }); + expect(seen).toHaveLength(1); + unsubscribe(); + }); + + it("stays silent for 30 days after an answer", () => { + markFeedbackAnswered(); + const { seen, unsubscribe } = collect(); + requestStudioFeedback({ reason: "render_complete" }); + expect(seen).toHaveLength(0); + unsubscribe(); + }); + + it("asks again once the 30 day answer cooldown has passed", () => { + localStorage.setItem("hyperframes-studio:feedbackAnsweredAt", String(Date.now() - 31 * DAY_MS)); + const { seen, unsubscribe } = collect(); + requestStudioFeedback({ reason: "render_complete" }); + expect(seen).toHaveLength(1); + unsubscribe(); + }); + + it("stays silent for 7 days after a dismissal, then asks again", () => { + markFeedbackDismissed(); + const first = collect(); + requestStudioFeedback({ reason: "render_complete" }); + expect(first.seen).toHaveLength(0); + first.unsubscribe(); + + sessionStorage.clear(); + localStorage.setItem("hyperframes-studio:feedbackDismissedAt", String(Date.now() - 8 * DAY_MS)); + const second = collect(); + requestStudioFeedback({ reason: "render_complete" }); + expect(second.seen).toHaveLength(1); + second.unsubscribe(); + }); + + it("never asks a profile that opted out of telemetry", () => { + telemetryAllowed = false; + const { seen, unsubscribe } = collect(); + requestStudioFeedback({ reason: "crash" }); + expect(seen).toHaveLength(0); + unsubscribe(); + }); + + it("does not burn the session slot on an ineligible request", () => { + telemetryAllowed = false; + const blocked = collect(); + requestStudioFeedback({ reason: "render_complete" }); + blocked.unsubscribe(); + + // Opting back in mid-session must not find the one ask already spent. + telemetryAllowed = true; + const { seen, unsubscribe } = collect(); + requestStudioFeedback({ reason: "render_complete" }); + expect(seen).toHaveLength(1); + unsubscribe(); + }); + + it("drops the request when nothing is listening", () => { + expect(() => requestStudioFeedback({ reason: "crash" })).not.toThrow(); + // The ask was not spent, so the next real subscriber still gets asked. + const { seen, unsubscribe } = collect(); + requestStudioFeedback({ reason: "crash" }); + expect(seen).toHaveLength(1); + unsubscribe(); + }); +}); + +describe("follow-up selection", () => { + it("gives every detractor the complaint question, never a rotated one", () => { + for (let rating = 0; rating <= 6; rating++) { + expect(followUpFor(rating).id).toBe("detractor"); + } + }); + + it("rotates among the three research questions above 6", () => { + const ids = new Set(); + for (let i = 0; i < 200; i++) ids.add(followUpFor(9).id); + expect([...ids].sort()).toEqual(["fix", "remove", "workaround"]); + }); + + it("gives every question scannable, distinct one-tap answers with explanations", () => { + for (const rating of [0, 7, 8, 9, 10]) { + const q = followUpFor(rating); + // Four is the floor for a useful spread; past seven the chips stop being + // scannable in a 340px card and start being a form. + expect(q.presets.length).toBeGreaterThanOrEqual(4); + expect(q.presets.length).toBeLessThanOrEqual(7); + expect(new Set(q.presets.map((p) => p.label)).size).toBe(q.presets.length); + for (const p of q.presets) { + expect(p.hint.length).toBeGreaterThan(0); + // A label that wraps to two lines in a chip row is not scannable. + expect(p.label.length).toBeLessThanOrEqual(24); + } + } + }); + + it("offers no brand names, which are a popularity vote and not an instruction", () => { + const brands = /capcut|premiere|resolve|after effects|final cut|remotion|canva|descript/i; + for (const rating of [0, 7, 8, 9, 10]) { + for (const p of followUpFor(rating).presets) { + expect(p.label).not.toMatch(brands); + expect(p.hint).not.toMatch(brands); + } + } + }); +}); + +describe("isProblemReason", () => { + it("treats everything except a finished render as a problem report", () => { + expect(isProblemReason("render_complete")).toBe(false); + expect(isProblemReason("render_failed")).toBe(true); + expect(isProblemReason("crash")).toBe(true); + }); +}); diff --git a/packages/studio/src/components/feedback/feedbackTrigger.ts b/packages/studio/src/components/feedback/feedbackTrigger.ts new file mode 100644 index 000000000..750690606 --- /dev/null +++ b/packages/studio/src/components/feedback/feedbackTrigger.ts @@ -0,0 +1,269 @@ +// --------------------------------------------------------------------------- +// Single owner of "should Studio ask for feedback right now, and about what". +// +// The prompt used to fire on a session counter (every 10th tab). That collected +// 5 responses in three months, because it asked at an arbitrary moment with no +// subject. It now fires on a render outcome — the moment the user has just +// formed an opinion — which is the same moment the CLI asks. +// +// Producers call `requestStudioFeedback`; the card subscribes. Eligibility +// lives here and nowhere else, so no caller has to know the cooldown rules. +// --------------------------------------------------------------------------- + +import { browserTelemetryAllowed } from "../../telemetry/policy"; + +export type FeedbackReason = "render_complete" | "render_failed" | "crash"; + +/** + * Everything except a finished render is a problem report: no 0-10 score (you + * cannot rate an export you never got, and a fabricated rating poisons the + * average) and the card wears its error styling. + */ +export function isProblemReason(reason: FeedbackReason): boolean { + return reason !== "render_complete"; +} + +/** + * A one-tap answer. `label` has to stand on its own — a chip that only makes + * sense once you hover it is a badly named chip, and most people never hover. + * `hint` carries the nuance a two-word label cannot, for the people who do. + */ +export interface FollowUpPreset { + label: string; + hint: string; +} + +/** + * The follow-up asked after a score. One question per person, rotated across + * people: a corner card that asks three things gets answered by nobody, but the + * same card asking one thing of every third user covers the same ground. + */ +export interface FollowUpQuestion { + /** Sent with the response, so answers stay separable instead of collapsing + * into one undifferentiated comment field. */ + id: string; + prompt: string; + placeholder: string; + /** + * Typing into a corner card is a cost most people decline, so the common + * answers are buttons and the field is the escape hatch. Keep these mutually + * distinct: two chips a user could reasonably read the same way collect a + * number that means nothing. + */ + presets: readonly FollowUpPreset[]; +} + +// The options below are not invented, and none of them is a brand. +// +// `fix` mirrors the themes that actually dominate written feedback: audio and +// encoding first by a wide margin, then media and fonts, then export-versus- +// preview mismatches, then render time. `workaround` mirrors the steps people +// report finishing in other tools. `remove` offers the surfaces that see the +// least real use, so the question contains genuine deletion candidates instead +// of a list nobody would pick from. `detractor` mirrors the failures Studio +// actually emits. +// +// Seven is the ceiling, not a target: past that the chips stop being scannable +// in a corner card and start being a form. Re-derive all of these when the +// shape of the feedback changes, because a stale option list quietly steers +// every answer it collects. +const FOLLOW_UPS: readonly FollowUpQuestion[] = [ + { + id: "remove", + prompt: "What would you cut from Studio?", + placeholder: "The part you never use", + presets: [ + { label: "Layers panel", hint: "The layer tree on the right" }, + { label: "Variables panel", hint: "Composition variables on the right" }, + { label: "Blocks browser", hint: "The block library in the sidebar" }, + { label: "Storyboard mode", hint: "The storyboard view next to Preview" }, + { label: "Caption editing", hint: "Editing caption words and presets in Studio" }, + { label: "Render history", hint: "The list of past exports" }, + { label: "Nothing", hint: "It all earns its place" }, + ], + }, + { + // Replaces an earlier "which editor gets this right?". Brand names are a + // popularity vote, not an instruction: "CapCut" does not say what to build. + // What someone had to leave HyperFrames to do names the missing feature + // exactly, and written feedback is full of these workarounds already. + // + // Every option here is a step Studio genuinely cannot do today. Offering + // one it CAN do (trimming, voiceover, ducking, compositing, grading) would + // collect taps that mean "I could not find it", and there is no way to tell + // those apart from "it does not exist" afterwards. Check before adding. + id: "workaround", + prompt: "What did you have to do outside Studio?", + placeholder: "The step you finished somewhere else", + presets: [ + { label: "Fix the exported file", hint: "Remuxing or re-encoding the output by hand" }, + { label: "Convert source media", hint: "Re-encoding footage before it would import cleanly" }, + { label: "Match loudness", hint: "Normalising levels across the finished audio" }, + { label: "Make images or graphics", hint: "Creating visual assets in another tool" }, + { label: "Record footage", hint: "Capturing screen or camera video" }, + { label: "Nothing, it was all here", hint: "No outside tools needed" }, + ], + }, + { + id: "fix", + prompt: "What should we fix first?", + placeholder: "The thing that slowed you down", + presets: [ + { label: "Audio mixing", hint: "Levels, ducking or the balance in the exported mix" }, + { label: "Export encoding", hint: "The render fails, or the finished file is wrong" }, + { label: "Export matches preview", hint: "The exported video differs from the preview" }, + { label: "Media and fonts", hint: "Adding video, images or fonts to a project" }, + { label: "Render speed", hint: "Exports take too long to finish" }, + { label: "Re-rendering everything", hint: "A small change means rendering the whole thing" }, + { + label: "Checks and warnings", + hint: "Lint flags things that are fine, or misses real ones", + }, + ], + }, +]; + +/** Detractors are not asked a rotated question; the complaint is the answer. */ +const DETRACTOR_FOLLOW_UP: FollowUpQuestion = { + id: "detractor", + prompt: "What went wrong?", + placeholder: "The thing that made that a low score", + presets: [ + { label: "Export failed", hint: "The render did not finish" }, + { label: "Output looked wrong", hint: "It rendered, but not the way I built it" }, + { label: "Crashed or froze", hint: "The editor stopped responding" }, + { label: "Lost my edits", hint: "Changes did not save, or came back changed" }, + { label: "Too slow", hint: "Waiting on renders, or the editor itself lagging" }, + { label: "Could not figure it out", hint: "Could not find or understand something" }, + ], +}; + +const DETRACTOR_MAX_RATING = 6; + +export function followUpFor(rating: number): FollowUpQuestion { + if (rating <= DETRACTOR_MAX_RATING) return DETRACTOR_FOLLOW_UP; + // Rotation only has to spread across users, not be unguessable, so any + // per-prompt jitter works. Studio's determinism rules cover rendered + // compositions, not the editor chrome. + return FOLLOW_UPS[Math.floor(Math.random() * FOLLOW_UPS.length)]; +} + +/** + * Facts about the moment being reported on, sent with the response so a + * complaint arrives with the conditions that produced it instead of needing a + * round trip to ask. Producers fill this; the trigger never inspects it. + * + * Names and enums only, no free text: this rides to the same place the comment + * does, and the user consented to a comment, not to their project's contents. + */ +export interface FeedbackContext { + [key: string]: string | number | boolean | undefined; +} + +export interface FeedbackRequest { + reason: FeedbackReason; + /** Render job the prompt is about — joins the response to the render event. */ + renderId?: string; + /** Failure text shown to the user, sent verbatim so reports are actionable. */ + detail?: string; + /** Reproduction context: settings, counts, outcomes. See FeedbackContext. */ + context?: FeedbackContext; +} + +const STORAGE_KEYS = { + /** Epoch ms of the last prompt the user dismissed or ignored. */ + dismissedAt: "hyperframes-studio:feedbackDismissedAt", + /** Epoch ms of the last prompt the user actually answered. */ + answeredAt: "hyperframes-studio:feedbackAnsweredAt", +} as const; + +/** One prompt per tab, so a batch of renders can't turn into a batch of asks. */ +const SESSION_ASKED_KEY = "hyperframes-studio:feedbackAskedThisSession"; + +const DAY_MS = 86_400_000; +const ANSWERED_COOLDOWN_MS = 30 * DAY_MS; +const DISMISSED_COOLDOWN_MS = 7 * DAY_MS; + +type Listener = (request: FeedbackRequest) => void; + +let listener: Listener | null = null; + +function isDisabled(): boolean { + try { + return import.meta.env.VITE_HYPERFRAMES_NO_FEEDBACK === "1"; + } catch { + return false; + } +} + +function readTimestamp(key: string): number { + try { + return parseInt(window.localStorage.getItem(key) || "0", 10) || 0; + } catch { + return 0; + } +} + +function writeTimestamp(key: string, now: number): void { + try { + window.localStorage.setItem(key, String(now)); + } catch { + /* localStorage may be unavailable or full */ + } +} + +function askedThisSession(): boolean { + try { + return window.sessionStorage.getItem(SESSION_ASKED_KEY) === "1"; + } catch { + // Without sessionStorage there is no way to bound the asks, so don't ask. + return true; + } +} + +function markAskedThisSession(): void { + try { + window.sessionStorage.setItem(SESSION_ASKED_KEY, "1"); + } catch { + /* sessionStorage may be unavailable */ + } +} + +function isEligible(now: number): boolean { + if (isDisabled()) return false; + // Prompting a user whose telemetry is off would collect a response we then + // drop on the floor. Ask only the people we can actually hear. + if (!browserTelemetryAllowed()) return false; + if (askedThisSession()) return false; + if (now - readTimestamp(STORAGE_KEYS.answeredAt) < ANSWERED_COOLDOWN_MS) return false; + if (now - readTimestamp(STORAGE_KEYS.dismissedAt) < DISMISSED_COOLDOWN_MS) return false; + return true; +} + +/** + * Ask for feedback about `request` if the user is due. Safe to call on every + * render outcome: ineligible calls are dropped without touching any state, so + * a user who just answered stays quiet rather than burning their next window. + */ +export function requestStudioFeedback(request: FeedbackRequest): void { + if (!listener) return; + if (!isEligible(Date.now())) return; + markAskedThisSession(); + listener(request); +} + +/** Subscribe the card. Single-subscriber by design — there is one card. */ +export function subscribeToFeedbackRequests(next: Listener): () => void { + listener = next; + return () => { + if (listener === next) listener = null; + }; +} + +export function markFeedbackAnswered(): void { + writeTimestamp(STORAGE_KEYS.answeredAt, Date.now()); +} + +export function markFeedbackDismissed(): void { + writeTimestamp(STORAGE_KEYS.dismissedAt, Date.now()); +} diff --git a/packages/studio/src/components/feedback/projectProvenance.test.ts b/packages/studio/src/components/feedback/projectProvenance.test.ts new file mode 100644 index 000000000..244b7fb38 --- /dev/null +++ b/packages/studio/src/components/feedback/projectProvenance.test.ts @@ -0,0 +1,120 @@ +// @vitest-environment happy-dom + +import { describe, it, expect, beforeEach, vi } from "vitest"; +import { + captureProjectProvenance, + projectProvenance, + resetProjectProvenance, +} from "./projectProvenance"; + +const CONFIG = "hyperframes.json"; + +/** + * The real `/files/*` route answers with an envelope, not the file. Mocking the + * raw config here instead let a broken parse pass the suite and fail live, so + * this helper is deliberately shaped like the server's actual response. + */ +function mockConfigResponse(fileContent: string, ok = true) { + vi.stubGlobal( + "fetch", + vi.fn(async () => ({ + ok, + json: async () => ({ filename: CONFIG, content: fileContent, version: "abc" }), + })), + ); +} + +beforeEach(() => { + resetProjectProvenance(); + vi.unstubAllGlobals(); +}); + +describe("captureProjectProvenance", () => { + it("reads the authoring workflow that built the project", async () => { + mockConfigResponse(JSON.stringify({ authoringSkill: "faceless-explainer" })); + await captureProjectProvenance("p1", [CONFIG, "index.html"], ["index.html"]); + expect(projectProvenance()).toMatchObject({ + project_scaffolded: true, + project_authoring_skill: "faceless-explainer", + }); + }); + + it("counts compositions and media, which is what actually reproduces a bug", async () => { + mockConfigResponse("{}"); + await captureProjectProvenance( + "p1", + [CONFIG, "index.html", "a.html", "assets/clip.mp4", "assets/vo.wav", "assets/logo.png"], + ["index.html", "a.html"], + ); + expect(projectProvenance()).toMatchObject({ + project_composition_count: 2, + project_media_count: 3, + project_file_count: 6, + }); + }); + + it("flags a project that init never scaffolded, and skips the config fetch", async () => { + const fetchSpy = vi.fn(); + vi.stubGlobal("fetch", fetchSpy); + await captureProjectProvenance("p1", ["index.html"], ["index.html"]); + expect(projectProvenance().project_scaffolded).toBe(false); + expect(projectProvenance().project_authoring_skill).toBeUndefined(); + expect(fetchSpy).not.toHaveBeenCalled(); + }); + + // Every way the config can fail to yield a skill must still leave the counts + // behind: a partial report beats no report. + it.each([ + ["the config is corrupt", () => mockConfigResponse("{ not json")], + [ + "the envelope has no content field", + () => + vi.stubGlobal( + "fetch", + vi.fn(async () => ({ ok: true, json: async () => ({ error: "not found" }) })), + ), + ], + [ + "the fetch fails outright", + () => + vi.stubGlobal( + "fetch", + vi.fn(async () => { + throw new Error("offline"); + }), + ), + ], + ["the route answers non-ok", () => mockConfigResponse("{}", false)], + ])("keeps the counts when %s", async (_case, arrange) => { + arrange(); + await expect( + captureProjectProvenance("p1", [CONFIG, "index.html"], ["index.html"]), + ).resolves.toBeUndefined(); + expect(projectProvenance().project_scaffolded).toBe(true); + expect(projectProvenance().project_composition_count).toBe(1); + expect(projectProvenance().project_authoring_skill).toBeUndefined(); + }); + + describe("privacy", () => { + it("drops a hand-edited skill that is not a slug, so no free text escapes", async () => { + mockConfigResponse( + JSON.stringify({ authoringSkill: "my client's secret campaign /Users/me/x" }), + ); + await captureProjectProvenance("p1", [CONFIG], []); + expect(projectProvenance().project_authoring_skill).toBeUndefined(); + }); + + it("never copies file names, paths or the project title", async () => { + mockConfigResponse(JSON.stringify({ authoringSkill: "slideshow", title: "Secret Launch" })); + await captureProjectProvenance( + "p1", + [CONFIG, "assets/unreleased-product-hero.mp4"], + ["index.html"], + ); + const flat = JSON.stringify(projectProvenance()); + expect(flat).not.toContain("unreleased"); + expect(flat).not.toContain("Secret"); + expect(flat).not.toContain("assets/"); + }); + }); +}); diff --git a/packages/studio/src/components/feedback/projectProvenance.ts b/packages/studio/src/components/feedback/projectProvenance.ts new file mode 100644 index 000000000..0bc8b4fd3 --- /dev/null +++ b/packages/studio/src/components/feedback/projectProvenance.ts @@ -0,0 +1,83 @@ +// --------------------------------------------------------------------------- +// How this project came to exist, and roughly what shape it is. +// +// A crash report says what broke. It does not say what the user was working ON, +// and "a project made by /faceless-explainer with 14 compositions and 60 assets" +// reproduces a crash that "Cannot read properties of undefined" never will. +// +// Held at module scope on purpose: a crash unmounts the React tree, so anything +// living in component state is gone by the time the crash prompt renders. This +// survives, because it was captured when the project loaded. +// +// PRIVACY: counts, a skill slug, and booleans. No file names, no paths, no +// project title. The user consented to a comment, not to an inventory of their +// work. +// --------------------------------------------------------------------------- + +import type { FeedbackContext } from "./feedbackTrigger"; + +/** Written by `hyperframes init`; absent in a hand-made or copied project. */ +const CONFIG_FILE = "hyperframes.json"; + +/** Matches the CLI's own slug gate, so a hand-edited value cannot leak text. */ +const SKILL_SLUG = /^[a-z0-9]+(?:-[a-z0-9]+)*$/; + +const MEDIA_EXTENSIONS = /\.(mp4|mov|webm|m4v|mp3|wav|m4a|aac|ogg|png|jpe?g|gif|webp|svg|avif)$/i; + +let snapshot: FeedbackContext = {}; + +function countMedia(files: string[]): number { + return files.filter((f) => MEDIA_EXTENSIONS.test(f)).length; +} + +/** + * Record what we can see from the project listing, then fill in the authoring + * skill from `hyperframes.json` if the project has one. Failure is silent and + * partial: a report with the counts but no skill still beats no report. + */ +export async function captureProjectProvenance( + projectId: string, + files: string[], + compositions: string[], +): Promise { + const scaffolded = files.includes(CONFIG_FILE); + snapshot = { + // False means the project was copied, hand-written, or predates init. + // Those reproduce differently from a scaffolded one, so the flag matters + // even when the skill below is missing. + project_scaffolded: scaffolded, + project_composition_count: compositions.length, + project_file_count: files.length, + project_media_count: countMedia(files), + }; + if (!scaffolded) return; + + try { + const res = await fetch(`/api/projects/${projectId}/files/${encodeURIComponent(CONFIG_FILE)}`); + if (!res.ok) return; + // The route answers with an envelope, not the file: {filename, content, + // version}. The config is the `content` string inside it. + const envelope: unknown = await res.json(); + const raw = (envelope as { content?: unknown }).content; + if (typeof raw !== "string") return; + const parsed: unknown = JSON.parse(raw); + if (typeof parsed !== "object" || parsed === null) return; + const skill = (parsed as { authoringSkill?: unknown }).authoringSkill; + // The workflow that built this project: which of the creation skills the + // agent ran. This is the closest thing to "how was it made" we can get. + if (typeof skill === "string" && SKILL_SLUG.test(skill)) { + snapshot.project_authoring_skill = skill; + } + } catch { + // Missing, unreadable or corrupt config: keep the counts, drop the skill. + } +} + +export function projectProvenance(): FeedbackContext { + return snapshot; +} + +/** Test seam. */ +export function resetProjectProvenance(): void { + snapshot = {}; +} diff --git a/packages/studio/src/components/renders/useRenderQueue.ts b/packages/studio/src/components/renders/useRenderQueue.ts index aae5b5aca..381863508 100644 --- a/packages/studio/src/components/renders/useRenderQueue.ts +++ b/packages/studio/src/components/renders/useRenderQueue.ts @@ -4,6 +4,7 @@ import { trackStudioRenderStart } from "../../telemetry/events"; import { getAnonymousId } from "../../telemetry/config"; import { browserTelemetryAllowed } from "../../telemetry/policy"; import { generateId } from "../../utils/generateId"; +import { requestStudioFeedback, type FeedbackContext } from "../feedback/feedbackTrigger"; export interface RenderJob { id: string; @@ -72,6 +73,23 @@ export function useRenderQueue(projectId: string | null) { const [actionError, setActionError] = useState(null); const eventSourceRef = useRef(null); const activeJobRef = useRef(null); + // Renders started in THIS tab, mapped to the settings they ran with. + // `loadRenders` also injects finished jobs from disk history, and those must + // never trigger a feedback prompt — the user did not just watch them happen. + const sessionJobs = useRef(new Map()); + const promptedJobIds = useRef(new Set()); + + /** + * The one way a render started here enters the list. Every start path — the + * happy one and all three failure shortcuts — goes through here, so both + * "this render belongs to this session" and "these are the settings it ran + * with" have a single owner. A report about a render is only actionable if + * it arrives with the settings that produced it. + */ + const addSessionJob = useCallback((job: RenderJob, settings: FeedbackContext) => { + sessionJobs.current.set(job.id, settings); + setJobs((prev) => [...prev, job]); + }, []); const closeActiveEventSource = useCallback((jobId?: string) => { if (jobId && activeJobRef.current !== jobId) return; @@ -148,6 +166,16 @@ export function useRenderQueue(projectId: string | null) { }); const startTime = Date.now(); + // Travels with any feedback about this render. Settings only: the + // composition path is a name the user chose, not file contents. + const settings: FeedbackContext = { + render_format: format, + render_quality: quality, + render_fps: fps, + render_resolution: resolution ?? "auto", + render_composition: composition ?? "index.html", + render_has_variables: Boolean(opts.variables && Object.keys(opts.variables).length > 0), + }; // "auto" / undefined means "render at the composition's authored size". // Omit the field entirely — sending "auto" would trip the route's // enum validation set. @@ -201,7 +229,7 @@ export function useRenderQueue(projectId: string | null) { filename: "Export failed", createdAt: startTime, }; - setJobs((prev) => [...prev, failedJob]); + addSessionJob(failedJob, settings); return; } if (!res.ok) { @@ -213,7 +241,7 @@ export function useRenderQueue(projectId: string | null) { filename: "Export failed", createdAt: startTime, }; - setJobs((prev) => [...prev, failedJob]); + addSessionJob(failedJob, settings); return; } const { jobId } = await res.json(); @@ -227,7 +255,7 @@ export function useRenderQueue(projectId: string | null) { filename: `${jobId}${ext}`, createdAt: startTime, }; - setJobs((prev) => [...prev, job]); + addSessionJob(job, settings); activeJobRef.current = jobId; // Track progress via SSE @@ -279,7 +307,7 @@ export function useRenderQueue(projectId: string | null) { return jobId; }, - [projectId, closeActiveEventSource], + [projectId, closeActiveEventSource, addSessionJob], ); // Cancel an in-flight render. The job row stays (as "cancelled") so the @@ -352,6 +380,36 @@ export function useRenderQueue(projectId: string | null) { const dismissActionError = useCallback(() => setActionError(null), []); + // Ask for feedback the moment a render this tab started reaches its outcome. + // Watching the list (rather than each of the four places a job can finish) + // keeps one trigger for every path, including SSE drops and cancels-that- + // finished-anyway. `requestStudioFeedback` decides whether to actually ask. + useEffect(() => { + for (const job of jobs) { + if (job.status === "rendering" || job.status === "cancelled") continue; + const settings = sessionJobs.current.get(job.id); + if (!settings || promptedJobIds.current.has(job.id)) continue; + promptedJobIds.current.add(job.id); + requestStudioFeedback({ + reason: job.status === "complete" ? "render_complete" : "render_failed", + renderId: job.id, + detail: job.error, + context: { + ...settings, + // How far it got and how long it took separate "died on frame one" + // from "died during encode", which need different fixes. + render_progress: job.progress, + render_duration_ms: job.durationMs ?? Date.now() - job.createdAt, + render_stage: job.stage, + render_error: job.error, + // Earlier renders this session: a first-render failure and a + // failure after nine successes are different bugs. + renders_this_session: sessionJobs.current.size, + }, + }); + } + }, [jobs]); + // Clean up EventSource on unmount or projectId change useEffect(() => { return () => { diff --git a/packages/studio/src/hooks/useFileTree.ts b/packages/studio/src/hooks/useFileTree.ts index bb4d42227..adc078de6 100644 --- a/packages/studio/src/hooks/useFileTree.ts +++ b/packages/studio/src/hooks/useFileTree.ts @@ -1,6 +1,7 @@ import { useState, useCallback, useEffect, useMemo } from "react"; import { FONT_EXT } from "../utils/mediaTypes"; import { fontFamilyFromAssetPath, type ImportedFontAsset } from "../components/editor/fontAssets"; +import { captureProjectProvenance } from "../components/feedback/projectProvenance"; interface UseFileTreeOptions { projectId: string | null; @@ -24,9 +25,13 @@ export function useFileTree({ projectId, projectIdRef }: UseFileTreeOptions) { fetch(`/api/projects/${projectId}`) .then((r) => r.json()) .then((data: { files?: string[]; dir?: string; compositions?: string[] }) => { - if (!cancelled && data.files) setFileTree(data.files); - if (!cancelled && data.compositions) setCompositionPaths(data.compositions); - if (!cancelled) setProjectDir(typeof data.dir === "string" ? data.dir : null); + if (cancelled) return; + if (data.files) setFileTree(data.files); + if (data.compositions) setCompositionPaths(data.compositions); + setProjectDir(typeof data.dir === "string" ? data.dir : null); + // Snapshot how this project was made, while the listing is in hand and + // the app is still alive. A crash later has no other way to learn it. + void captureProjectProvenance(projectId, data.files ?? [], data.compositions ?? []); }) .catch(() => { if (!cancelled) setProjectDir(null); diff --git a/packages/studio/src/telemetry/breadcrumbs.test.ts b/packages/studio/src/telemetry/breadcrumbs.test.ts new file mode 100644 index 000000000..637e93a9b --- /dev/null +++ b/packages/studio/src/telemetry/breadcrumbs.test.ts @@ -0,0 +1,68 @@ +import { describe, it, expect, beforeEach } from "vitest"; +import { recordBreadcrumb, breadcrumbTrail, resetBreadcrumbs } from "./breadcrumbs"; + +beforeEach(() => resetBreadcrumbs()); + +describe("breadcrumbTrail", () => { + it("is empty before anything happens", () => { + expect(breadcrumbTrail()).toBe(""); + }); + + it("records events oldest first with a seconds stamp", () => { + recordBreadcrumb("studio_session_start", {}); + recordBreadcrumb("studio_render_start", {}); + const trail = breadcrumbTrail(); + expect(trail).toMatch(/^\d+\.\d session_start > \d+\.\d render_start$/); + }); + + it("strips the studio prefix from both event naming styles", () => { + recordBreadcrumb("studio_render_start", {}); + recordBreadcrumb("studio:save_failure", {}); + expect(breadcrumbTrail()).toContain("render_start"); + expect(breadcrumbTrail()).toContain("save_failure"); + expect(breadcrumbTrail()).not.toContain("studio_"); + expect(breadcrumbTrail()).not.toContain("studio:"); + }); + + it("keeps a short discriminator so an event says where it happened", () => { + recordBreadcrumb("studio:tab_switch", { tab: "renders" }); + expect(breadcrumbTrail()).toContain("tab_switch:renders"); + }); + + it("rolls, keeping the most recent run-up rather than the oldest history", () => { + for (let i = 0; i < 40; i++) recordBreadcrumb(`studio_step_${i}`, {}); + const trail = breadcrumbTrail(); + expect(trail).toContain("step_39"); + expect(trail).not.toContain("step_0 "); + expect(trail.split(" > ")).toHaveLength(14); + }); + + describe("privacy", () => { + it("never copies free text, however it is spelled", () => { + recordBreadcrumb("studio_feedback", { + comment: "my secret unreleased product name", + error_message: "/Users/someone/private/path.html", + stack_trace: "at Object. (/Users/someone/project.ts:1:1)", + }); + const trail = breadcrumbTrail(); + expect(trail).not.toContain("secret"); + expect(trail).not.toContain("/Users/"); + expect(trail).toContain("feedback"); + }); + + it("drops an allowlisted value that is long enough to be prose", () => { + recordBreadcrumb("studio_thing", { + action: "a".repeat(200), + }); + expect(breadcrumbTrail()).toContain("thing"); + expect(breadcrumbTrail()).not.toContain("aaaa"); + }); + + it("takes numbers and booleans from allowlisted keys", () => { + recordBreadcrumb("studio_a", { status: 500 }); + recordBreadcrumb("studio_b", { mode: false }); + expect(breadcrumbTrail()).toContain("a:500"); + expect(breadcrumbTrail()).toContain("b:false"); + }); + }); +}); diff --git a/packages/studio/src/telemetry/breadcrumbs.ts b/packages/studio/src/telemetry/breadcrumbs.ts new file mode 100644 index 000000000..a9c110262 --- /dev/null +++ b/packages/studio/src/telemetry/breadcrumbs.ts @@ -0,0 +1,70 @@ +// --------------------------------------------------------------------------- +// A short trail of what the user did before they reported something. +// +// A feedback comment says what went wrong; it almost never says how to get +// there. This is the missing half. Every `studio_*` event already flows through +// one funnel (`trackEvent`), so recording the trail costs one call there and no +// new instrumentation anywhere else — and it stays correct as events are added. +// +// PRIVACY: names and numbers only. Values are copied from a fixed allowlist of +// short, low-cardinality keys, so free text (comments, file paths, composition +// content) can never reach the trail even if some future event carries it. +// --------------------------------------------------------------------------- + +/** Enough to cover the run-up to a failure without bloating the payload. */ +const LIMIT = 14; + +/** + * Keys worth keeping alongside an event name. Each is a short enum-ish + * discriminator: `tab_switch` alone says little, `tab_switch:renders` says + * where they were. Anything not on this list is dropped, not truncated. + */ +const DETAIL_KEYS = [ + "action", + "tab", + "panel", + "status", + "mode", + "reason", + "format", + "via", +] as const; + +interface Crumb { + at: number; + label: string; +} + +const trail: Crumb[] = []; +const startedAt = Date.now(); + +function detailFor(properties: Record): string { + for (const key of DETAIL_KEYS) { + const value = properties[key]; + if (typeof value === "string" && value.length > 0 && value.length <= 24) return `:${value}`; + if (typeof value === "number" || typeof value === "boolean") return `:${String(value)}`; + } + return ""; +} + +export function recordBreadcrumb(event: string, properties: Record): void { + // `studio:` and `studio_` prefixes are noise in a 14-item trail. + const name = event.replace(/^studio[:_]/, ""); + trail.push({ at: Date.now() - startedAt, label: `${name}${detailFor(properties)}` }); + if (trail.length > LIMIT) trail.shift(); +} + +/** + * The trail as one line, oldest first, each entry stamped with seconds since + * the tab opened: `2.1 session_start > 48.7 render_start > 71.2 save_failure`. + * A single string rather than an array so it stays readable in a PostHog cell + * and in whatever the report is pasted into. + */ +export function breadcrumbTrail(): string { + return trail.map((c) => `${(c.at / 1000).toFixed(1)} ${c.label}`).join(" > "); +} + +/** Test seam. Production never clears the trail; it rolls. */ +export function resetBreadcrumbs(): void { + trail.length = 0; +} diff --git a/packages/studio/src/telemetry/client.ts b/packages/studio/src/telemetry/client.ts index 615d497d3..a7068ac54 100644 --- a/packages/studio/src/telemetry/client.ts +++ b/packages/studio/src/telemetry/client.ts @@ -8,6 +8,7 @@ import { getAnonymousId, hasShownNotice, markNoticeShown } from "./config"; import { browserTelemetryAllowed } from "./policy"; import { getBrowserSystemMeta } from "./system"; import { canaryEventProperties } from "./canary"; +import { recordBreadcrumb } from "./breadcrumbs"; // Write-only PostHog project key, safe to embed in client code. const POSTHOG_API_KEY = "phc_zjjbX0PnWxERXrMHhkEJWj9A9BhGVLRReICgsfTMmpx"; @@ -37,6 +38,10 @@ export function shouldTrack(): boolean { export function trackEvent(event: string, properties: EventProperties = {}): void { if (!shouldTrack()) return; + // Every studio event passes through here, so this is the one place that can + // build a repro trail without asking each call site to opt in. + recordBreadcrumb(event, properties); + const sys = getBrowserSystemMeta(); eventQueue.push({ event, diff --git a/packages/studio/src/telemetry/events.ts b/packages/studio/src/telemetry/events.ts index a2f3e4b17..b11be0fb2 100644 --- a/packages/studio/src/telemetry/events.ts +++ b/packages/studio/src/telemetry/events.ts @@ -1,4 +1,5 @@ import { trackEvent } from "./client"; +import { breadcrumbTrail } from "./breadcrumbs"; // Studio frontend events. The corresponding `render_complete` / `render_error` // events are emitted server-side by `packages/cli/src/server/studioServer.ts` @@ -94,14 +95,94 @@ export function trackStudioSegmentEaseEdit(props: { trackEvent("studio_segment_ease_edit", { action: props.action, ease: props.ease }); } -export function trackStudioFeedback(props: { rating: number; comment?: string }): void { +/** + * Context shared by every event in the feedback funnel, so `shown` → + * `dismissed` / `studio_feedback` can be read as one funnel broken down by the + * moment that triggered it. Without `shown` there is no way to tell a prompt + * nobody answers from a prompt that never renders. + */ +interface StudioFeedbackContext { + /** What the prompt is about: "render_complete" | "render_failed". */ + reason: string; + /** Render job the prompt followed — joins the response to that render. */ + render_id?: string; +} + +export function trackStudioFeedbackShown(ctx: StudioFeedbackContext): void { + trackEvent("studio_feedback_shown", { + reason: ctx.reason, + render_id: ctx.render_id, + source: "studio", + }); +} + +export function trackStudioFeedbackDismissed( + ctx: StudioFeedbackContext & { + /** "close" | "escape" | "timeout" — separates rejection from inattention. */ + via: string; + /** A dismiss after picking a rating is an abandon, not a refusal. */ + had_rating: boolean; + }, +): void { + trackEvent("studio_feedback_dismissed", { + reason: ctx.reason, + render_id: ctx.render_id, + via: ctx.via, + had_rating: ctx.had_rating, + source: "studio", + }); +} + +/** + * The booking link offered after a response. Its own event because the thing + * worth measuring is the click, and a link is otherwise invisible to us. + */ +export function trackStudioFeedbackInterviewClick(ctx: { reason: string }): void { + trackEvent("studio_feedback_interview_click", { + reason: ctx.reason, + source: "studio", + }); +} + +export function trackStudioFeedback( + props: StudioFeedbackContext & { + /** + * Absent on the failure prompt, which asks what broke instead of scoring a + * render the user never got. A fabricated rating would poison the average. + */ + rating?: number; + comment?: string; + /** + * Which follow-up the comment answers ("remove" | "borrow" | "fix" | + * "detractor" | "failure"). One card asks one question, rotated across + * users, so this is what makes the free text separable. + */ + question: string; + /** "preset" (a tapped chip) or "typed". Never mix them when counting. */ + answer_kind: string; + /** + * Reproduction context from whatever produced the prompt: render settings, + * outcome, counts. Flattened onto the event so each key is filterable in + * PostHog rather than buried in a JSON blob nobody can group by. + */ + context?: Record; + }, +): void { // Plain product event, not a PostHog survey response: nothing here is served // by the surveys product (no survey definition, no targeting, no popover). trackEvent("studio_feedback", { - rating: props.rating, - rating_scale: 10, + ...(props.rating === undefined ? {} : { rating: props.rating, rating_scale: 10 }), ...(props.comment ? { comment: props.comment } : {}), + ...props.context, + reason: props.reason, + render_id: props.render_id, + question: props.question, + answer_kind: props.answer_kind, doctor_summary: getBrowserDoctorSummary(), + // What the user did in the run-up. A comment says what broke; this says + // how to get there, which is the half a bug report is usually missing. + // Read AFTER the context spread so no caller can shadow it. + breadcrumbs: breadcrumbTrail(), source: "studio", }); } diff --git a/scripts/check-no-main-deletions.mjs b/scripts/check-no-main-deletions.mjs index 183df3046..c04e49651 100644 --- a/scripts/check-no-main-deletions.mjs +++ b/scripts/check-no-main-deletions.mjs @@ -24,6 +24,25 @@ import { execFileSync } from "node:child_process"; const BASE_FLAG = "--base"; +/** + * Deletions this repository has already agreed to, each with the reason. + * + * A blanket escape hatch (a flag, an env var, `--force`) would turn the guard + * off exactly when it matters, because the branch deleting something by + * accident is also the branch that would reach for it. Naming each path here + * instead keeps the default absolute and makes every intentional removal a + * reviewable line in a diff. + * + * Entries are for deletions that are NOT renames — git already pairs those on + * its own. Remove an entry once its deletion has landed on the base. + */ +export const ALLOWED_DELETIONS = new Map([ + [ + "packages/studio/src/components/StudioFeedbackBar.tsx", + "replaced by components/feedback/StudioFeedbackCard.tsx; too little shared content for git to pair as a rename", + ], +]); + export function parseBase(argv, fallback = "origin/main") { const index = argv.indexOf(BASE_FLAG); if (index === -1) return fallback; @@ -71,7 +90,14 @@ function main() { process.exit(2); } - const { deleted, renamed } = classify(diff); + const { deleted: allDeleted, renamed } = classify(diff); + const agreed = allDeleted.filter((path) => ALLOWED_DELETIONS.has(path)); + const deleted = allDeleted.filter((path) => !ALLOWED_DELETIONS.has(path)); + + if (agreed.length > 0) { + console.log(`${agreed.length} deletion(s) agreed in ALLOWED_DELETIONS:`); + for (const path of agreed) console.log(` ${path} — ${ALLOWED_DELETIONS.get(path)}`); + } if (renamed.length > 0) { console.log(`${renamed.length} renamed (allowed):`); for (const { from, to } of renamed.slice(0, 10)) console.log(` ${from} -> ${to}`); diff --git a/scripts/check-no-main-deletions.test.mjs b/scripts/check-no-main-deletions.test.mjs index 48371cb3c..6b9d738f2 100644 --- a/scripts/check-no-main-deletions.test.mjs +++ b/scripts/check-no-main-deletions.test.mjs @@ -1,7 +1,7 @@ import { strict as assert } from "node:assert"; import { test } from "node:test"; -import { classify, parseBase } from "./check-no-main-deletions.mjs"; +import { ALLOWED_DELETIONS, classify, parseBase } from "./check-no-main-deletions.mjs"; test("a branch that only adds reports nothing", () => { const { deleted, renamed } = classify("A\tpackages/cli/src/new.ts\nM\tpackages/cli/src/old.ts\n"); @@ -40,3 +40,22 @@ test("a --base with no value fails rather than silently defaulting", () => { assert.throws(() => parseBase(["--base"]), /needs a ref/); assert.throws(() => parseBase(["--base", "--other"]), /needs a ref/); }); + +test("every agreed deletion names a path and says why", () => { + // The guard has no blanket override on purpose: a flag or an env var would + // be reached for by the branch deleting something by accident. An entry has + // to be written down, so the removal shows up in review. + for (const [path, reason] of ALLOWED_DELETIONS) { + assert.ok(path.length > 0, "an allowed deletion needs a path"); + assert.ok( + reason && reason.length > 10, + `${path} needs a reason, got ${JSON.stringify(reason)}`, + ); + } +}); + +test("the allowlist does not silence an unrelated deletion", () => { + const { deleted } = classify("D\tpackages/cli/src/something-else.ts\n"); + assert.deepEqual(deleted, ["packages/cli/src/something-else.ts"]); + assert.equal(ALLOWED_DELETIONS.has("packages/cli/src/something-else.ts"), false); +});