From 20d915938b33301d78402407d167841f8da3e9a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Tue, 11 Aug 2026 01:33:52 -0400 Subject: [PATCH] feat(studio): ask for feedback when a render ends, not on a session counter (#3205) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What Studio's feedback prompt now fires when a render finishes or fails, instead of on a session counter, and the reports it collects carry enough context to act on. - Replaces the 32px inline bar with a card in the existing toast stack - Adds `studio_feedback_shown` / `studio_feedback_dismissed` / `studio_feedback_interview_click`, so the funnel is visible - A failed export and a crash skip the 0-10 score and ask what happened - Adds a crash prompt to the error boundary - Attaches a breadcrumb trail, render settings and outcome, and how the project was created ## Before / after Before: a 32px feedback strip pinned under the preview. After: the feedback card in the toast stack, with one-tap answers, and the red variant for a failed export. The old bar's rating numbers are `neutral-600` on `neutral-900/80`, which is why it reads as a disabled row rather than a control. **In the running Studio** — bottom-right, sharing the toast stack, hovering a chip explains it on the line above the input: The feedback card in the bottom-right of the running Studio, with the rotated follow-up question and a hovered chip explained inline. **On the crash screen** — the prompt the error boundary renders, asking what the user was doing rather than what went wrong, since the stack trace already covers the latter: The Studio crash screen with the feedback card below the Try again and Reload Studio buttons. | | Before | After | |---|---|---| | **Trigger** | Every 10th session | A render finishing, failing, or a crash | | **Placement** | 32px inline bar, pushes preview up | Card in the toast stack, no layout shift | | **Rating targets** | 11 bare buttons, `neutral-600` | Native radios, resting fill, `neutral-400` | | **Follow-up** | Free text or nothing | One rotated question, four to seven one-tap answers | | **Option help** | None | Inline hint on hover and focus | | **Press feedback** | None | `active:scale-[0.97]`, 150ms ease-out | | **Keyboard** | No exit path | Escape closes, Enter sends, arrows move the rating | | **Auto-dismiss** | 20s, always | 30s, cancelled the moment you interact | | **Failure case** | Same NPS question | Its own question, no score, error quoted back | | **Crash case** | Nothing | Prompt on the crash screen | | **Visibility** | Submissions only | Shown / dismissed (with reason) / submitted / interview click | | **Report content** | Rating, comment | Plus breadcrumbs, render settings and outcome, project provenance | ## Why The old bar fired on a session count, so it interrupted at a moment with no subject: nothing the user had just done, nothing to have an opinion about. It also emitted nothing when it appeared or when it was dismissed, which made the collection rate impossible to diagnose. A prompt nobody answers and a prompt that never renders looked identical from the outside. Visually it read as a disabled row: 11 buttons at 11px in `neutral-600` on a dark strip, with no resting affordance. And appearing mid-task pushed the whole preview stack up, which the old code carried a comment apologising for. Separately, the reports it did collect were not actionable. A comment says what went wrong; it almost never says how to get there. ## How **One trigger, one owner.** `feedbackTrigger` owns eligibility and nothing else does: once per tab, thirty days after an answer, seven after a dismissal, never when telemetry is off (prompting someone whose response we would then drop wastes their attention). `VITE_HYPERFRAMES_NO_FEEDBACK=1` still disables it entirely. **One hook, every failure path.** The trigger watches the render job list rather than each of the four places a render can finish (server rejection, unreachable server, SSE terminal event, SSE connection drop), so paths added later are covered without touching the trigger. Renders loaded from disk history never fire it. **Reuses what exists.** The card wears `StudioToast`'s glass treatment and joins its stack, so there is no second visual language and no new CSS. The rating row is native radios, which gives arrow-key navigation, grouping and labels for free. **One question each, rotated across users.** A corner card that asks three things gets answered by nobody. Each person gets one follow-up with one-tap answers, explained on a reserved line rather than a floating tooltip (the card is 340px in a corner; a bubble above the chips lands on the question, below lands on the input). Detractors are never given a rotated question, because they already have a specific complaint. Every option was checked against the code: an option naming a feature Studio already has would collect taps meaning "I could not find it", which is indistinguishable afterwards from "it does not exist". **Breadcrumbs cost one line.** Every studio event already flows through `trackEvent`, so recording the trail there needs no new instrumentation and stays correct as events are added. **Provenance lives outside React.** A crash unmounts the tree, so it is captured when the project loads and read from module scope when the crash prompt renders. ### Privacy Breadcrumbs and provenance carry names, enums and counts only. Values are copied from a fixed allowlist of short keys, and anything longer than a slug is dropped rather than truncated, so comments, file paths, stack traces and project titles cannot reach them even if a future event carries one. Tests assert this. ### Where these responses land Studio feedback goes to PostHog and nowhere else, which is what it did before this change too. Worth stating because the CLI behaves differently: `hyperframes feedback` also forwards to the backend feedback endpoint via `submitFeedback`, on top of its PostHog event. Studio has never used that path, before or after this PR, so if you read CLI feedback anywhere other than PostHog, Studio responses will not show up there. Nothing here changes that either way. Whether the two surfaces should share a delivery path is a product question, not a defect in this change, and closing it would need a field on the backend DTO: it is shaped around `cli_version`, and Studio reports from a crash or a failed export deliberately carry no rating. ## Test plan - [x] Unit tests added/updated - [x] Manual testing performed - [ ] Documentation updated (if applicable) **Unit** — 39 new tests: trigger eligibility and cooldowns, the detractor override, rotation, preset shape and the no-brands rule, breadcrumb rolling and privacy, provenance parsing and its failure modes, and the crash boundary rendering the prompt with no rating input. **Live** — both render paths driven end to end against a running Studio on a production bundle, with real renders. Every PostHog request was intercepted and dropped, so nothing reached the project. Verified the emitted payload for a finished render, a failed export, the rotated follow-ups, each chip's hint, and the interview link. **Not covered** — no live capture of a spontaneous crash. Three attempts to force one failed because Studio's guards held and it kept rendering, so the crash path is verified by component tests rather than by driving it. Touch devices see chip labels without hints, since the hint is revealed on hover and focus. --- .../studio/src/components/EditorShell.tsx | 2 - .../components/StudioErrorBoundary.test.tsx | 97 +++++ .../src/components/StudioErrorBoundary.tsx | 7 + .../src/components/StudioFeedbackBar.tsx | 217 ---------- .../studio/src/components/StudioOverlays.tsx | 28 +- .../feedback/CrashFeedbackPrompt.tsx | 27 ++ .../feedback/StudioFeedbackCard.tsx | 373 ++++++++++++++++++ .../feedback/feedbackTrigger.test.ts | 168 ++++++++ .../components/feedback/feedbackTrigger.ts | 269 +++++++++++++ .../feedback/projectProvenance.test.ts | 120 ++++++ .../components/feedback/projectProvenance.ts | 83 ++++ .../src/components/renders/useRenderQueue.ts | 66 +++- packages/studio/src/hooks/useFileTree.ts | 11 +- .../studio/src/telemetry/breadcrumbs.test.ts | 68 ++++ packages/studio/src/telemetry/breadcrumbs.ts | 70 ++++ packages/studio/src/telemetry/client.ts | 5 + packages/studio/src/telemetry/events.ts | 87 +++- scripts/check-no-main-deletions.mjs | 28 +- scripts/check-no-main-deletions.test.mjs | 21 +- 19 files changed, 1503 insertions(+), 244 deletions(-) create mode 100644 packages/studio/src/components/StudioErrorBoundary.test.tsx delete mode 100644 packages/studio/src/components/StudioFeedbackBar.tsx create mode 100644 packages/studio/src/components/feedback/CrashFeedbackPrompt.tsx create mode 100644 packages/studio/src/components/feedback/StudioFeedbackCard.tsx create mode 100644 packages/studio/src/components/feedback/feedbackTrigger.test.ts create mode 100644 packages/studio/src/components/feedback/feedbackTrigger.ts create mode 100644 packages/studio/src/components/feedback/projectProvenance.test.ts create mode 100644 packages/studio/src/components/feedback/projectProvenance.ts create mode 100644 packages/studio/src/telemetry/breadcrumbs.test.ts create mode 100644 packages/studio/src/telemetry/breadcrumbs.ts 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); +});