diff --git a/electron/media/cursorSidecar.test.ts b/electron/media/cursorSidecar.test.ts index 88ad3f60e..1c0e46a4d 100644 --- a/electron/media/cursorSidecar.test.ts +++ b/electron/media/cursorSidecar.test.ts @@ -140,7 +140,7 @@ describe("readCursorRecordingFileAt", () => { }); describe("readCursorTelemetryFile", () => { - it("drops interactionType — which is why the digest does not use it", async () => { + it("keeps interactionType — the auto-zoom detector needs the clicks", async () => { const video = path.join(dir, "clicks.mp4"); await writeSidecar(video, { samples: [{ timeMs: 10, cx: 0.5, cy: 0.5, interactionType: "click" }], @@ -149,11 +149,34 @@ describe("readCursorTelemetryFile", () => { const result = await readCursorTelemetryFile(video, {}); expect(result.success).toBe(true); - expect(result.samples).toEqual([{ timeMs: 10, cx: 0.5, cy: 0.5 }]); - // Locked deliberately: this projection is fine for the timeline overlay it - // feeds and useless for "where did the user act". Anything that wants - // clicks must go through `readCursorSidecar`, and this assertion is the - // reminder of why. - expect(Object.keys(result.samples[0])).not.toContain("interactionType"); + expect(result.samples).toEqual([{ timeMs: 10, cx: 0.5, cy: 0.5, interactionType: "click" }]); + // This projection feeds the ai-edition auto-zoom detector, whose click + // candidates ARE the recorded interactions (issue #699). It used to strip + // the field — locked by a test, even — and the detector never saw a click + // however many the take recorded; the digest reads `readCursorSidecar` + // directly, which is how it kept working. Positions-only consumers ignore + // the extra field. + }); + + it("keeps every click kind, not only the plain left click", async () => { + const video = path.join(dir, "kinds.mp4"); + await writeSidecar(video, { + samples: [ + { timeMs: 10, cx: 0.1, cy: 0.1, interactionType: "double-click" }, + { timeMs: 20, cx: 0.2, cy: 0.2, interactionType: "right-click" }, + { timeMs: 30, cx: 0.3, cy: 0.3, interactionType: "middle-click" }, + ], + }); + + const result = await readCursorTelemetryFile(video, {}); + + expect(result.success).toBe(true); + // normalizeCursorSample used to coerce these three kinds to "move", so the + // detector saw a take with no clicks at all. + expect(result.samples.map((sample) => sample.interactionType)).toEqual([ + "double-click", + "right-click", + "middle-click", + ]); }); }); diff --git a/electron/media/cursorSidecar.ts b/electron/media/cursorSidecar.ts index 2b6192f7b..a47f63f5b 100644 --- a/electron/media/cursorSidecar.ts +++ b/electron/media/cursorSidecar.ts @@ -57,6 +57,9 @@ export function normalizeCursorSample(sample: unknown): CursorRecordingSample | const point = sample as Partial; const interactionType = point.interactionType === "click" || + point.interactionType === "double-click" || + point.interactionType === "right-click" || + point.interactionType === "middle-click" || point.interactionType === "mouseup" || point.interactionType === "move" ? point.interactionType @@ -223,12 +226,10 @@ export async function readCursorRecordingFile( return (await readCursorSidecar(targetVideoPath, options)).data; } -/** The renderer's `loadCursorTelemetry`: positions only, no interaction type. - * - * ponytail: this projection DROPS `interactionType`, which means it drops every - * click. That is fine for the timeline overlay it feeds and wrong for anything - * that wants to know where the user acted — the agent digest reads - * `readCursorSidecar` directly for exactly that reason. */ +/** The renderer's `loadCursorTelemetry`. Carries `interactionType` through: the + * ai-edition auto-zoom detector places zooms on recorded clicks (issue #699), so + * this IS a "where did the user act" consumer. The old projection here dropped + * the field — the detector never saw a click, however many the take recorded. */ export async function readCursorTelemetryFile( targetVideoPath: string, options: { recordingsDir?: string }, @@ -241,6 +242,7 @@ export async function readCursorTelemetryFile( timeMs: sample.timeMs, cx: sample.cx, cy: sample.cy, + interactionType: sample.interactionType, })), }; } catch (error) { diff --git a/src/lib/ai-edition/timeline/zoom-suggestions.test.ts b/src/lib/ai-edition/timeline/zoom-suggestions.test.ts index 75e63ce79..f36b1cc32 100644 --- a/src/lib/ai-edition/timeline/zoom-suggestions.test.ts +++ b/src/lib/ai-edition/timeline/zoom-suggestions.test.ts @@ -23,6 +23,16 @@ function dwell( })); } +// A click = a single sample carrying recorded interaction metadata (issue #699). +function click( + timeMs: number, + cx: number, + cy: number, + interactionType: CursorTelemetryPoint["interactionType"] = "click", +): CursorTelemetryPoint { + return { timeMs, cx, cy, interactionType }; +} + describe("detectZoomDwellCandidates", () => { it("finds a dwell where the cursor sits still", () => { const candidates = detectZoomDwellCandidates(dwell(1000, 0.4, 0.6)); @@ -90,6 +100,219 @@ describe("buildAutoZoomSuggestions", () => { }); }); +describe("recorded clicks (issue #699)", () => { + // A fast sweep forms no dwell; before #699 this take produced no suggestion at all. + const sweepWithClickAt = (clickIndex: number): CursorTelemetryPoint[] => + Array.from({ length: 10 }, (_, i) => ({ + timeMs: i * 500, + cx: i / 10, + cy: i / 10, + interactionType: i === clickIndex ? "click" : "move", + })); + + it("zooms on a click made while the pointer is still moving", () => { + const suggestions = buildAutoZoomSuggestions({ + cursorTelemetry: sweepWithClickAt(4), + totalMs: 5000, + existingRegions: [], + defaultDurationMs: 2000, + }); + expect(suggestions).toHaveLength(1); + expect(suggestions[0].focus).toEqual({ cx: 0.4, cy: 0.4 }); + expect(suggestions[0].span).toEqual({ start: 1000, end: 3000 }); + }); + + it("focuses the zoom on the click sample, not the pointer's average position", () => { + // The pointer flew to a toolbar button, clicked, flew back. + const suggestions = buildAutoZoomSuggestions({ + cursorTelemetry: [ + { timeMs: 1000, cx: 0.1, cy: 0.9 }, + click(1200, 0.8, 0.2), + { timeMs: 1400, cx: 0.12, cy: 0.88 }, + ], + totalMs: 5000, + existingRegions: [], + defaultDurationMs: 2000, + }); + expect(suggestions).toHaveLength(1); + expect(suggestions[0].focus).toEqual({ cx: 0.8, cy: 0.2 }); + expect(suggestions[0].span).toEqual({ start: 200, end: 2200 }); + }); + + it("accepts every click kind the telemetry records", () => { + for (const kind of ["click", "double-click", "right-click", "middle-click"] as const) { + const suggestions = buildAutoZoomSuggestions({ + cursorTelemetry: [click(2000, 0.3, 0.7, kind), { timeMs: 4000, cx: 0.4, cy: 0.6 }], + totalMs: 5000, + existingRegions: [], + defaultDurationMs: 2000, + }); + expect(suggestions, kind).toHaveLength(1); + expect(suggestions[0].focus).toEqual({ cx: 0.3, cy: 0.7 }); + } + }); + + it("ignores move and mouseup — they are not clicks", () => { + const telemetry: CursorTelemetryPoint[] = Array.from({ length: 10 }, (_, i) => ({ + timeMs: i * 500, + cx: i / 10, + cy: i / 10, + interactionType: i === 4 ? "mouseup" : "move", + })); + expect( + buildAutoZoomSuggestions({ + cursorTelemetry: telemetry, + totalMs: 5000, + existingRegions: [], + defaultDurationMs: 2000, + }), + ).toEqual([]); + }); + + it("folds a nearby dwell into the click so one moment yields ONE zoom", () => { + // The dwell run 1550..2270ms centres on 1910ms — 490ms from the click at 2400ms, + // inside SUGGESTION_SPACING_MS. The click's own time and position anchor the zoom. + const suggestions = buildAutoZoomSuggestions({ + cursorTelemetry: [...dwell(2000, 0.2, 0.8), click(2400, 0.3, 0.75)], + totalMs: 6000, + existingRegions: [], + defaultDurationMs: 2000, + }); + expect(suggestions).toHaveLength(1); + expect(suggestions[0].focus).toEqual({ cx: 0.3, cy: 0.75 }); + expect(suggestions[0].span).toEqual({ start: 1400, end: 3400 }); + }); + + it("keeps a dwell that sits clear of the click, and both survive", () => { + const suggestions = buildAutoZoomSuggestions({ + cursorTelemetry: [...dwell(2000, 0.2, 0.8), click(6000, 0.9, 0.1)], + totalMs: 9000, + existingRegions: [], + defaultDurationMs: 1500, + }); + // Nobody conflicts here, so the output order is an acceptance-order detail; + // compare the SET of spans and the click-anchored focus. + const spans = suggestions.map((s) => s.span).sort((a, b) => a.start - b.start); + expect(spans).toEqual([ + { start: 1250, end: 2750 }, + { start: 5250, end: 6750 }, + ]); + expect(suggestions.find((s) => s.span.start === 5250)?.focus).toEqual({ cx: 0.9, cy: 0.1 }); + expect(suggestions.find((s) => s.span.start === 1250)?.focus.cx).toBeCloseTo(0.2, 5); + }); + + it("collapses a rapid burst of clicks into the first click's zoom", () => { + const suggestions = buildAutoZoomSuggestions({ + cursorTelemetry: [click(1000, 0.2, 0.2), click(1500, 0.25, 0.25)], + totalMs: 5000, + existingRegions: [], + defaultDurationMs: 2000, + }); + expect(suggestions).toHaveLength(1); + expect(suggestions[0].focus).toEqual({ cx: 0.2, cy: 0.2 }); + expect(suggestions[0].span).toEqual({ start: 0, end: 2000 }); + }); + + it("still drops a click that overlaps an existing zoom region", () => { + expect( + buildAutoZoomSuggestions({ + cursorTelemetry: [click(2000, 0.5, 0.5), { timeMs: 4000, cx: 0.6, cy: 0.4 }], + totalMs: 5000, + existingRegions: [{ startMs: 1500, endMs: 2500 }], + defaultDurationMs: 2000, + }), + ).toEqual([]); + }); + + it("keeps the dwell-only behaviour when the take records no clicks", () => { + // A dwell annotated only with move/mouseup: no click candidate, exactly the + // pre-#699 result — and the mouseup does not break the still run either. + const dwellRun = dwell(2000, 0.5, 0.5).map((sample, i) => ({ + ...sample, + interactionType: (i === 3 ? "mouseup" : "move") as "mouseup" | "move", + })); + const suggestions = buildAutoZoomSuggestions({ + cursorTelemetry: dwellRun, + totalMs: 5000, + existingRegions: [], + defaultDurationMs: 2000, + }); + expect(suggestions).toHaveLength(1); + expect(suggestions[0].span).toEqual({ start: 1000, end: 3000 }); + }); + + it("clamps a click near the start or end of the take to the ruler", () => { + const suggestions = buildAutoZoomSuggestions({ + cursorTelemetry: [click(100, 0.1, 0.1), click(4800, 0.9, 0.9)], + totalMs: 5000, + existingRegions: [], + defaultDurationMs: 2000, + }); + expect(suggestions.map((s) => s.span)).toEqual([ + { start: 0, end: 2000 }, + { start: 3000, end: 5000 }, + ]); + }); + + it("keeps the dwell fallback when an existing region rejects only the nearby click", () => { + // The click at 2700ms sits inside SUGGESTION_SPACING_MS of the dwell at 1000ms, + // but the existing region 2300..2400 overlaps only the CLICK's span. The click + // is rejected, and a rejected click must leave the dwell standing: one zoom at + // the dwell, not zero. (The first cut of this PR removed the dwell up front and + // returned nothing here.) + const suggestions = buildAutoZoomSuggestions({ + cursorTelemetry: [...dwell(1000, 0.2, 0.8), click(2700, 0.6, 0.4)], + totalMs: 5000, + existingRegions: [{ startMs: 2300, endMs: 2400 }], + defaultDurationMs: 1000, + }); + expect(suggestions).toHaveLength(1); + expect(suggestions[0].span).toEqual({ start: 500, end: 1500 }); + expect(suggestions[0].focus.cx).toBeCloseTo(0.2, 5); + expect(suggestions[0].focus.cy).toBeCloseTo(0.8, 5); + }); + + it("lets a second click survive the first being rejected by an existing region", () => { + // Two clicks within spacing of each other; only the first's span overlaps the + // existing region. The rejected first click must not suppress the second. + const suggestions = buildAutoZoomSuggestions({ + cursorTelemetry: [click(1000, 0.2, 0.2), click(2400, 0.8, 0.8)], + totalMs: 5000, + existingRegions: [{ startMs: 700, endMs: 1300 }], + defaultDurationMs: 1000, + }); + expect(suggestions).toHaveLength(1); + expect(suggestions[0].focus).toEqual({ cx: 0.8, cy: 0.8 }); + expect(suggestions[0].span).toEqual({ start: 1900, end: 2900 }); + }); + + it("zooms a take whose telemetry is a single recorded click", () => { + const suggestions = buildAutoZoomSuggestions({ + cursorTelemetry: [click(2000, 0.3, 0.7)], + totalMs: 5000, + existingRegions: [], + defaultDurationMs: 2000, + }); + expect(suggestions).toHaveLength(1); + expect(suggestions[0].focus).toEqual({ cx: 0.3, cy: 0.7 }); + expect(suggestions[0].span).toEqual({ start: 1000, end: 3000 }); + }); + + it("returns nothing for a single move or mouseup sample", () => { + for (const kind of ["move", "mouseup"] as const) { + expect( + buildAutoZoomSuggestions({ + cursorTelemetry: [{ timeMs: 2000, cx: 0.3, cy: 0.7, interactionType: kind }], + totalMs: 5000, + existingRegions: [], + defaultDurationMs: 2000, + }), + kind, + ).toEqual([]); + } + }); +}); + describe("buildAutoZoomSuggestionsForClips", () => { const clip = ( id: string, @@ -199,4 +422,55 @@ describe("buildAutoZoomSuggestionsForClips", () => { }), ).toEqual([]); }); + + // Clicks ride the same per-clip projection dwells do (issue #699). + it("gives a recorded click the same per-clip projection a dwell gets", () => { + // A clip that starts 30s into the recording: a click at source 34s is ruler 4s. + const clips = [clip("clip_1", "a1", 30, 40, 0)]; + const suggestions = buildAutoZoomSuggestionsForClips({ + cursorTelemetry: [click(34000, 0.5, 0.5), { timeMs: 36000, cx: 0.6, cy: 0.4 }], + assetId: "a1", + clips, + existingRegions: [], + defaultDurationMs: 2000, + }); + expect(suggestions.map((s) => s.span)).toEqual([{ start: 3000, end: 5000 }]); + expect(suggestions[0].focus).toEqual({ cx: 0.5, cy: 0.5 }); + }); + + it("gives a click to EVERY clip that replays it, one shifted span each", () => { + const clips = [clip("clip_1", "a1", 0, 10, 0), clip("clip_2", "a1", 0, 10, 10)]; + const suggestions = buildAutoZoomSuggestionsForClips({ + cursorTelemetry: [click(4000, 0.3, 0.7), { timeMs: 6000, cx: 0.4, cy: 0.6 }], + assetId: "a1", + clips, + existingRegions: [], + defaultDurationMs: 2000, + }); + expect(suggestions.map((s) => s.span)).toEqual([ + { start: 3000, end: 5000 }, + { start: 13000, end: 15000 }, + ]); + for (const suggestion of suggestions) { + expect(suggestion.focus.cx).toBeCloseTo(0.3, 5); + expect(suggestion.focus.cy).toBeCloseTo(0.7, 5); + } + }); + + it("zooms a clip whose source window trims the telemetry down to a single click", () => { + // A clip covering 30..40s of the recording; the take's telemetry holds one move + // long before the window and one click inside it. The per-clip filter hands the + // detector a SINGLE sample — still a zoom, correctly projected (source 34s is + // ruler 4s). + const clips = [clip("clip_1", "a1", 30, 40, 0)]; + const suggestions = buildAutoZoomSuggestionsForClips({ + cursorTelemetry: [{ timeMs: 5000, cx: 0.1, cy: 0.9 }, click(34000, 0.5, 0.5)], + assetId: "a1", + clips, + existingRegions: [], + defaultDurationMs: 2000, + }); + expect(suggestions.map((s) => s.span)).toEqual([{ start: 3000, end: 5000 }]); + expect(suggestions[0].focus).toEqual({ cx: 0.5, cy: 0.5 }); + }); }); diff --git a/src/lib/ai-edition/timeline/zoom-suggestions.ts b/src/lib/ai-edition/timeline/zoom-suggestions.ts index 79769b6b7..b82872267 100644 --- a/src/lib/ai-edition/timeline/zoom-suggestions.ts +++ b/src/lib/ai-edition/timeline/zoom-suggestions.ts @@ -2,9 +2,10 @@ // // Ported from main's `src/components/video-editor/timeline/zoomSuggestionUtils.ts` // (the legacy editor's "magic wand" auto-zoom) into the ai-edition timeline -// module. This is NOT an AI feature — it's a deterministic dwell-detector over +// module. This is NOT an AI feature — it's a deterministic detector over // recorded cursor movement: stretches where the cursor sits still become -// zoom-in candidates, focused on the average cursor position during the dwell. +// zoom-in candidates, focused on the average cursor position during the dwell, +// and so do recorded clicks, focused on the click itself (issue #699). import type { CursorTelemetryPoint, ZoomFocus } from "@/components/video-editor/types"; import type { AxcutClip } from "../schema"; @@ -29,6 +30,7 @@ function normalizeTelemetrySample( timeMs: Math.max(0, Math.min(sample.timeMs, totalMs)), cx: Math.max(0, Math.min(sample.cx, 1)), cy: Math.max(0, Math.min(sample.cy, 1)), + interactionType: sample.interactionType, }; } @@ -105,15 +107,44 @@ export function detectZoomDwellCandidates( return dwellCandidates; } +/** The recorded interactions that count as a click; `move` and `mouseup` do not. */ +const CLICK_INTERACTION_TYPES: ReadonlySet> = + new Set(["click", "double-click", "right-click", "middle-click"]); + +/** + * Clicks are zoom candidates in their own right (issue #699): a click made while the + * pointer is still moving forms no dwell, yet it is exactly the moment a viewer wants + * magnified. The candidate is the click sample itself — its time, its position — and + * `strength` is 0 because a click has no duration to rank by; that only ever breaks + * ties between two clicks, where the stable sort keeps the earlier one. + */ +function detectZoomClickCandidates(samples: CursorTelemetryPoint[]): ZoomDwellCandidate[] { + return samples + .filter( + (sample) => + sample.interactionType !== undefined && CLICK_INTERACTION_TYPES.has(sample.interactionType), + ) + .map((sample) => ({ + centerTimeMs: sample.timeMs, + focus: { cx: sample.cx, cy: sample.cy }, + strength: 0, + })); +} + export interface AutoZoomSuggestion { span: { start: number; end: number }; focus: ZoomFocus; } /** - * Build non-overlapping zoom suggestions from cursor telemetry: detect dwell moments, - * rank by duration, space by SUGGESTION_SPACING_MS, drop any overlapping an existing - * region. Pure, shared by the magic-wand toggle and the on-load auto-suggest pass. + * Build non-overlapping zoom suggestions from cursor telemetry: detect dwell moments + * and recorded clicks, space by SUGGESTION_SPACING_MS, drop any overlapping an + * existing region. Clicks are honoured FIRST, so only an ACCEPTED click folds the + * dwells within suggestion spacing of it — the acceptance loop rejects those dwells + * against the accepted click's centre — while a click that spacing or an existing + * region rejects leaves its nearby dwells standing as the fallback. One recorded + * click still yields ONE zoom anchored on the click's own time and position + * (issue #699). Pure, shared by the magic-wand toggle and the on-load auto-suggest pass. */ export function buildAutoZoomSuggestions(options: { cursorTelemetry: CursorTelemetryPoint[]; @@ -122,7 +153,7 @@ export function buildAutoZoomSuggestions(options: { defaultDurationMs: number; }): AutoZoomSuggestion[] { const { cursorTelemetry, totalMs, existingRegions, defaultDurationMs } = options; - if (totalMs <= 0 || cursorTelemetry.length < 2) { + if (totalMs <= 0 || cursorTelemetry.length === 0) { return []; } @@ -132,20 +163,29 @@ export function buildAutoZoomSuggestions(options: { } const normalizedSamples = normalizeCursorTelemetry(cursorTelemetry, totalMs); - if (normalizedSamples.length < 2) { + if (normalizedSamples.length === 0) { return []; } const dwellCandidates = detectZoomDwellCandidates(normalizedSamples); - if (dwellCandidates.length === 0) { + const clickCandidates = detectZoomClickCandidates(normalizedSamples); + if (dwellCandidates.length === 0 && clickCandidates.length === 0) { return []; } + // Clicks are honoured FIRST so that only an accepted click folds its nearby dwells: + // the acceptance loop below rejects any dwell within SUGGESTION_SPACING_MS of an + // accepted click's centre, while a click that spacing or an existing region rejects + // leaves the dwells around it standing. Telemetry without click metadata runs the + // unchanged dwell-only ranking. Within each kind the order is the old one — clicks + // chronological, dwells by duration. + const dwellByDuration = [...dwellCandidates].sort((a, b) => b.strength - a.strength); + const sortedCandidates = [...clickCandidates, ...dwellByDuration]; + const reservedSpans = existingRegions .map((region) => ({ start: region.startMs, end: region.endMs })) .sort((a, b) => a.start - b.start); - const sortedCandidates = [...dwellCandidates].sort((a, b) => b.strength - a.strength); const acceptedCenters: number[] = []; const suggestions: AutoZoomSuggestion[] = []; @@ -195,8 +235,8 @@ export function buildAutoZoomSuggestions(options: { * them a dwell belongs to. It belongs to BOTH, and gets one zoom on each. * * So the projection is per clip, and it is a plain shift: a raw clip is identity between - * its source time and its raw-virtual time (see timeline/timelineMap.ts), so a dwell at - * source `t` on a clip covering `[sourceStartSec, sourceEndSec]` sits at + * its source time and its raw-virtual time (see timeline/timelineMap.ts), so a dwell or + * click at source `t` on a clip covering `[sourceStartSec, sourceEndSec]` sits at * `timelineStartSec + (t - sourceStartSec)`. Each clip is handed only the samples inside * its own source window, so a dwell that a cut split across two clips is no longer one * dwell — which is right: the cursor did not sit still across the cut on the timeline the diff --git a/src/native/contracts.ts b/src/native/contracts.ts index 49687a0dd..0c73c0f3e 100644 --- a/src/native/contracts.ts +++ b/src/native/contracts.ts @@ -33,7 +33,10 @@ export interface CursorRecordingSample extends CursorTelemetryPoint { assetId?: string | null; visible?: boolean; cursorType?: NativeCursorType | null; - interactionType?: "move" | "click" | "mouseup"; + /** The full interaction contract the sidecar may carry; matches the renderer's + * CursorTelemetryPoint. The old narrow override ("move" | "click" | "mouseup") + * legitimized coercing every other click kind to "move" at parse time. */ + interactionType?: "move" | "click" | "double-click" | "right-click" | "middle-click" | "mouseup"; } export interface NativeCursorAsset {