Skip to content

feat: Annotation panel#4

Open
tommyjtl wants to merge 2 commits into
mainfrom
feat/annotations
Open

feat: Annotation panel#4
tommyjtl wants to merge 2 commits into
mainfrom
feat/annotations

Conversation

@tommyjtl

@tommyjtl tommyjtl commented May 23, 2026

Copy link
Copy Markdown
Member

Note

Medium Risk
Adds a new persisted annotations feature (new storage keyspace + export) and threads it through the replay workspace/menubar, plus touches multiple timing-sensitive video/IK hooks (seeking, memoization, effects) which could affect interaction correctness.

Overview
Adds an Annotations workflow to replay comparisons: users can create per-video annotations, set frame/time ranges via sliders, apply label taxonomy + notes, and filter the list by frame range in the new AnnotationPanel.

Introduces useAnnotations with localStorage persistence keyed by the comparison storageKey, plus a new app/lib/annotations export schema and menubar actions to toggle annotation mode and export annotations per video.

Includes several interaction/stability refactors: converts a number of “latest props in refs” patterns to useLayoutEffect (e.g., TrajectoryOverlay, VideoDropzone, useIKDrag), restructures MouseControlPanel canvas animation handling, improves useKeyMoments active-slider resolution, and normalizes useVideoFps/useOverlaySettings state derivations.

Reviewed by Cursor Bugbot for commit 1b5cd2c. Bugbot is set up for automated code reviews on this repo. Configure here.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 1b5cd2c. Configure here.

availableTrajectoryTrackNames.filter((trackName) => !hiddenTrajectoryTrackNames.includes(trackName))
), [availableTrajectoryTrackNames, hiddenTrajectoryTrackNames]);
availableTrajectoryTrackNames.filter((trackName) => !effectiveHiddenTrajectoryTrackNames.includes(trackName))
), [availableTrajectoryTrackNames, effectiveHiddenTrajectoryTrackNames]);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale hidden tracks cause wrong toggle button label

Medium Severity

The hook returns the raw hiddenTrajectoryTrackNames state instead of effectiveHiddenTrajectoryTrackNames. The old useEffect that pruned stale entries from state was replaced by a derived useMemo, but only visibleTrajectoryTrackNames uses the filtered value internally. OverlaySettingsPanel checks hiddenTrajectoryTrackNames.length === 0 for allTracksVisible, so stale track names from previously-removed metadata cause the "Show All"/"Hide All" button to display the wrong label even when all available tracks are visible.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 1b5cd2c. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant