feat(analytics): video watch percentage and drop-off tracking

[ManthanNimodiya]

Summary

Adds watch engagement analytics to the video share page, requested by the community.

• Tracking: fires video_progress events at 25%, 50%, 75%, 95% milestones per viewer session via sendBeacon, same pattern as existing page_hit tracking, zero extra latency
• Backend: /api/analytics/track handles video_progress separately (no email/notification overhead); skips recording if the viewer is the video owner; stores percent_watched in Tinybird analytics_events
• Query: getVideoEngagement() is auth-gated (owner only) and aggregates per-session max milestone via ClickHouse subquery — counts how many sessions reached each quartile and avg watch %
• UI: engagement section shown in the Analytics tab (owner-only) — avg watched % + 4-step drop-off bars, hidden until first real data arrives

Security

• getVideoEngagement throws Unauthorized if the caller is not the video owner
• Owner self-views are excluded from engagement data

Greptile Summary

This PR adds video watch-engagement analytics: milestone beacons fired from the share page, a new /api/analytics/track branch that records video_progress events to Tinybird (skipping the owner), a getVideoEngagement() server action that aggregates per-session milestones, and a drop-off bar UI visible only to the video owner.

• The new querySql helper in Tinybird has its API-error throw inside the same try/catch block as the TSV fallback, causing Tinybird error responses (wrong token, missing column, etc.) to be swallowed and returned as garbage data instead of propagating.
• Three issues from a prior review round remain unaddressed: non-existent video IDs are still written to Tinybird, null sessions collapse to the shared \"anon\" key (breaking per-session aggregation), and the RAF-based lazy-attach path discards the timeupdate listener cleanup on unmount.

Confidence Score: 3/5

Multiple defects remain across the analytics pipeline, two of which corrupt the data the owner sees.

The querySql error-swallowing bug means Tinybird configuration problems silently produce empty or NaN engagement data with no way to detect the failure. Three issues from the prior review round are still unaddressed: non-existent video IDs pollute Tinybird storage, anonymous sessions share the anon key breaking per-session milestone counts, and the RAF-path cleanup omission leaves a dangling timeupdate listener after unmount.

packages/web-backend/src/Tinybird/index.ts (querySql error handling), apps/web/app/api/analytics/track/route.ts (missing video-existence guard, session-ID fallback), and apps/web/app/s/[videoId]/Share.tsx (RAF listener cleanup).

Important Files Changed

Filename	Overview
apps/web/actions/videos/get-analytics.ts	Adds getVideoEngagement() with owner-only auth, Tinybird SQL aggregation, and a format-validation regex; the regex is positioned after the DB auth check rather than before it.
apps/web/app/api/analytics/track/route.ts	Adds video_progress tracking branch; previously flagged issues remain: non-existent video IDs still written to Tinybird and null sessions fall back to the shared "anon" key.
apps/web/app/s/[videoId]/Share.tsx	Adds milestone progress tracking via timeupdate; the RAF-based lazy-attach path discards the cleanup returned by attach(), leaving a dangling listener on unmount (previously flagged).
apps/web/app/s/[videoId]/_components/tabs/Activity/Analytics.tsx	Adds owner-only DropOffBar engagement UI; renders correctly and hides itself when total === 0.
packages/web-backend/src/Tinybird/index.ts	New querySql method added; its inner try/catch eats intentional Tinybird API error throws, causing error responses to fall through to TSV parsing and silently return garbage data.

Comments Outside Diff (1)

1. apps/desktop/src-tauri/src/auth.rs, line 107 (link)

   organizations_updated_at set unconditionally outside the match

   The unconditional assignment on line 107 is now outside both arms of the match, so it runs on both success (redundant, since the Ok arm already sets it at line 101) and on every error. The original code intentionally skipped updating the timestamp on error when auth.organizations was non-empty, so that a subsequent call would still attempt a refresh. With this change, a transient network failure immediately marks the cache as "just checked", silencing retries for the full backoff window even when the stale data should be re-fetched.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: apps/desktop/src-tauri/src/auth.rs
   Line: 107
   
   Comment:
   **`organizations_updated_at` set unconditionally outside the match**
   
   The unconditional assignment on line 107 is now outside both arms of the `match`, so it runs on both success (redundant, since the `Ok` arm already sets it at line 101) and on every error. The original code intentionally skipped updating the timestamp on error when `auth.organizations` was non-empty, so that a subsequent call would still attempt a refresh. With this change, a transient network failure immediately marks the cache as "just checked", silencing retries for the full backoff window even when the stale data should be re-fetched.
   
   How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
apps/web/actions/videos/get-analytics.ts:111-115
The format guard is placed after the DB ownership check, but `safeId` is then used directly in a Tinybird SQL string via template interpolation. While the regex itself (`^[0-9a-zA-Z_-]+$`) does prevent SQL injection, this ordering means any video whose ID passes the DB lookup but somehow fails the regex would receive an ownership error message rather than a format error — confusing to debug. More importantly, `Video.VideoId.make(videoId)` on line 108 may already validate format at the type level, making this check redundant. If it is needed, place it before the DB call so the intent is clear.

```suggestion
	if (!/^[0-9a-zA-Z_-]+$/.test(videoId))
		throw new Error("Invalid video ID format");

	if (!video || video.ownerId !== user.id) throw new Error("Unauthorized");

	const safeId = videoId;
```

### Issue 2 of 2
packages/web-backend/src/Tinybird/index.ts:250-267
**`querySql` error-check throw is caught by its own TSV fallback**

The `throw new Error(normalizedRes.error)` sits inside the same `try` block that the bare `catch` at line 263 covers. When Tinybird returns an API error (e.g. `{"error":"column percent_watched not found"}`), `JSON.parse` succeeds, the error is detected and thrown — but the `catch` immediately intercepts it and passes the raw JSON error string to `parseTsvToObjects`. The result is garbage data returned as "success," so callers like `getVideoEngagement` see `total: NaN`, the engagement section silently stays hidden, and no diagnostic is surfaced. The fix is to split the `try` into two: one that only wraps `JSON.parse` (falling back to TSV on parse failure), and a separate block for the error-field check that can throw freely.

_{Reviews (2): Last reviewed commit: "fix(analytics): validate videoId format ..." | Re-trigger Greptile}

[greptile-apps]

RAF cleanup discards event-listener detachment

When playerRef.current is null at effect mount time, a requestAnimationFrame is scheduled and the effect returns () => cancelAnimationFrame(raf) as its cleanup. When the RAF fires and attach() successfully adds the timeupdate listener, the returned cleanup closure is thrown away — React's effect cleanup only cancels the RAF (a no-op if it already ran), so the listener is never removed on unmount. This leaves a dangling timeupdate callback that continues firing and sending beacons after the component is gone.

Prompt To Fix With AI

This is a comment left during a code review.
Path: apps/web/app/s/[videoId]/Share.tsx
Line: 397-398

Comment:
**RAF cleanup discards event-listener detachment**

When `playerRef.current` is `null` at effect mount time, a `requestAnimationFrame` is scheduled and the effect returns `() => cancelAnimationFrame(raf)` as its cleanup. When the RAF fires and `attach()` successfully adds the `timeupdate` listener, the returned cleanup closure is thrown away — React's effect cleanup only cancels the RAF (a no-op if it already ran), so the listener is never removed on unmount. This leaves a dangling `timeupdate` callback that continues firing and sending beacons after the component is gone.

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

Non-existent video IDs written to Tinybird

When the DB lookup fails (caught by orElseSucceed) or the video simply doesn't exist, videoRecord is undefined. The combined guard if (videoRecord && userId === videoRecord.ownerId) return evaluates to false, so execution falls through and the event is appended to Tinybird with whatever arbitrary string was in body.videoId. An unauthenticated caller can pollute Tinybird with events for non-existent videos. Add an explicit if (!videoRecord) return before the owner check to prevent writes for unknown video IDs.

Prompt To Fix With AI

This is a comment left during a code review.
Path: apps/web/app/api/analytics/track/route.ts
Line: 121-130

Comment:
**Non-existent video IDs written to Tinybird**

When the DB lookup fails (caught by `orElseSucceed`) or the video simply doesn't exist, `videoRecord` is `undefined`. The combined guard `if (videoRecord && userId === videoRecord.ownerId) return` evaluates to `false`, so execution falls through and the event is appended to Tinybird with whatever arbitrary string was in `body.videoId`. An unauthenticated caller can pollute Tinybird with events for non-existent videos. Add an explicit `if (!videoRecord) return` before the owner check to prevent writes for unknown video IDs.

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

"anon" collapses all sessionless viewers into one ClickHouse row

When sessionId is null, every event is stored with session_id = 'anon'. The aggregation query in getVideoEngagement does GROUP BY session_id, so all these events merge into a single session row. For a video with many anonymous viewers this makes count() (total sessions) report 1 and per-milestone counts wildly wrong. Using crypto.randomUUID() as the fallback would give each request a unique session and keep the per-session aggregation correct.

Prompt To Fix With AI

This is a comment left during a code review.
Path: apps/web/app/api/analytics/track/route.ts
Line: 129

Comment:
**`"anon"` collapses all sessionless viewers into one ClickHouse row**

When `sessionId` is `null`, every event is stored with `session_id = 'anon'`. The aggregation query in `getVideoEngagement` does `GROUP BY session_id`, so all these events merge into a single session row. For a video with many anonymous viewers this makes `count()` (total sessions) report 1 and per-milestone counts wildly wrong. Using `crypto.randomUUID()` as the fallback would give each request a unique session and keep the per-session aggregation correct.

How can I resolve this? If you propose a fix, please make it concise.

[superagent-security]

Superagent didn't find any vulnerabilities or security issues in this PR.

[ManthanNimodiya]

@greptileai please re-review

[greptile-apps]

Want your agent to iterate on Greptile's feedback? Try greploops.