feat: Implement Cap Analytics #1383
Conversation
WalkthroughThis PR introduces a comprehensive analytics system powered by Tinybird, replacing the previous DUB-based integration. It adds new analytics dashboard components, server-side analytics tracking, Tinybird service infrastructure, database schema and setup scripts, refactors existing analytics flags across the codebase, and includes new animated UI components for the dashboard. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser
participant Share as Share.tsx
participant Track as /api/analytics/track
participant Tinybird
User->>Browser: Visits shared video
Browser->>Share: Component mounts
Share->>Share: ensureAnalyticsSessionId()
Share->>Browser: Store session in localStorage
Share->>Track: POST trackVideoView payload<br/>(videoId, sessionId, device, etc.)
Track->>Track: Parse request & auth
Track->>Track: Extract user-agent, location, etc.
Track->>Tinybird: Append page_hit event
Tinybird-->>Track: Success
Track-->>Browser: { success: true }
sequenceDiagram
participant Dashboard as AnalyticsDashboard
participant API as /api/dashboard/analytics
participant Backend as getOrgAnalyticsData
participant DB as Database
participant TB as Tinybird
Dashboard->>API: GET /api/dashboard/analytics?orgId=X&range=7d&spaceId=Y
API->>API: Validate auth, org, range
API->>Backend: Call getOrgAnalyticsData(X, '7d', Y)
Backend->>DB: Fetch videos, comments, reactions
Backend->>TB: queryPipe / querySql for views, geographies, devices
TB-->>Backend: Aggregated results
Backend->>Backend: Normalize, aggregate, compute breakdowns
Backend-->>API: OrgAnalyticsResponse
API-->>Dashboard: { counts, chart, breakdowns, topCaps }
Dashboard->>Dashboard: Render charts & tables
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 22
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
apps/web/app/(org)/dashboard/_components/Navbar/Top.tsx (1)
56-74: Guard dynamic route params before building the title map
useParams()returnsReadonly<Record<string, string | string[]>>, soparams.idis astring | string[] | undefined. Interpolating that union into the template literal keys trips the type checker (thestring[]branch isn’t allowed) and, on routes without anid, we end up registering/dashboard/folder/undefinedand/dashboard/analytics/s/undefined. Build will fail once TS catches this.Please type-narrow the param once and only append the dynamic keys when it’s present. For example:
- const params = useParams(); + const params = useParams<{ id?: string }>(); + const routeId = Array.isArray(params.id) ? params.id[0] : params.id; - const titles: Record<string, string> = { + const titles: Record<string, string> = { "/dashboard/caps": "Caps", "/dashboard/folder": "Caps", "/dashboard/shared-caps": "Shared Caps", "/dashboard/caps/record": "Record a Cap", "/dashboard/settings/organization": "Organization Settings", "/dashboard/settings/account": "Account Settings", "/dashboard/spaces": "Spaces", "/dashboard/spaces/browse": "Browse Spaces", - "/dashboard/analytics": "Analytics", - [`/dashboard/folder/${params.id}`]: "Caps", - [`/dashboard/analytics/s/${params.id}`]: "Analytics: Cap video title", - }; + "/dashboard/analytics": "Analytics", + }; + + if (routeId) { + titles[`/dashboard/folder/${routeId}`] = "Caps"; + titles[`/dashboard/analytics/s/${routeId}`] = + "Analytics: Cap video title"; + }This keeps the type checker happy and avoids the unintended
.../undefinedentries.apps/web/app/(org)/dashboard/_components/Navbar/Items.tsx (1)
1-43: Fix import sorting as indicated by pipeline failure.The pipeline reports that imports and exports are not sorted. Run
pnpm formatto fix this automatically.As per coding guidelines.
apps/web/app/(org)/dashboard/caps/components/CapCard/CapCardAnalytics.tsx (1)
1-1: Fix formatting.#!/bin/bash pnpm format apps/web/app/(org)/dashboard/caps/components/CapCard/CapCardAnalytics.tsxapps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx (1)
140-156: Fix Tinybird download blob retrieval
fetchResponse.arrayBufferis a function; yielding it without calling returns the function itself, so the download flow blows up and never produces a blob. Invoke the method before yielding so you get theArrayBuffer.Apply this diff to fix it:
- const blob = yield* fetchResponse.arrayBuffer; + const blob = yield* fetchResponse.arrayBuffer();
🧹 Nitpick comments (11)
apps/web/app/globals.css (1)
54-74: Remove duplicate.dashboard-gridCSS rule.The
.dashboard-gridclass is defined twice identically (lines 54–63 and 65–74). Remove one of these duplicate definitions..dashboard-grid { display: grid; grid-template-columns: auto 1fr 1fr; grid-template-areas: "sidebar main main" "sidebar main main"; height: 100dvh; width: 100vw; min-height: 100dvh; } -.dashboard-grid { - display: grid; - grid-template-columns: auto 1fr 1fr; - grid-template-areas: - "sidebar main main" - "sidebar main main"; - height: 100dvh; - width: 100vw; - min-height: 100dvh; -} - .left-perspective {apps/web/app/(org)/dashboard/_components/Navbar/Items.tsx (1)
54-82: Consider removing the unusedsubNavproperty.The
subNavproperty is consistently set to an empty array and is never referenced in the component logic. This appears to be dead code.Apply this diff if sub-navigation is not planned:
const manageNavigation = [ { name: "My Caps", href: `/dashboard/caps`, extraText: userCapsCount, icon: <CapIcon />, - subNav: [], }, { name: "Analytics", href: `/dashboard/analytics`, ignoreParams: true, icon: <ChartLineIcon />, - subNav: [], }, { name: "Record a Cap", href: `/dashboard/caps/record`, icon: <RecordIcon />, - subNav: [], }, { name: "Organization Settings", href: `/dashboard/settings/organization`, ownerOnly: true, icon: <CogIcon />, - subNav: [], }, ];apps/web/app/api/analytics/route.ts (1)
28-31: Drop the ad-hoc console debuggingThe new
console.logcalls will spam production logs with video identifiers and result payloads. Please remove them (or replace with structured logging behind the existing logging stack if truly needed).apps/web/app/(org)/dashboard/analytics/components/VideoComponents.tsx (1)
129-158: Forward theidprop to the drop zone
VideoDroppableaccepts anid, but it is never applied to the DOM element. Please wire it through so consumers can target the drop zone reliably.-> ({ droppedValue, onRemove, isDragging, dragPosition, label }, ref) => { +> ({ id, droppedValue, onRemove, isDragging, dragPosition, label }, ref) => { ... - <div + <div + id={id} ref={ref}apps/web/app/s/[videoId]/Share.tsx (3)
38-63: Rename constant to reflect localStorage usage.The constant
SESSION_STORAGE_KEYsuggestssessionStoragebut the implementation useslocalStorage(lines 44, 54). This naming inconsistency can confuse developers.Consider renaming to
ANALYTICS_SESSION_KEYorLOCAL_STORAGE_SESSION_KEY:-const SESSION_STORAGE_KEY = "cap_tb_session_id"; +const ANALYTICS_SESSION_KEY = "cap_tb_session_id";And update the references accordingly.
65-137: Inconsistent return type in trackVideoView.The function returns a cleanup function only when using the fetch fallback (line 136), but returns
undefinedwhen usingsendBeacon(line 115 implicit return). This inconsistency could confuse callers expecting a cleanup function.Consider normalizing the return type:
if (queued) { - return; + return () => {}; // no-op cleanup for sendBeacon path }Alternatively, adjust the return type to explicitly allow
void | (() => void).
278-291: Consider error handling in analytics tracking effect.The
useEffectdoesn't handle potential errors fromtrackVideoView. While the tracking function has internal error handling, any synchronous errors (e.g., fromensureAnalyticsSessionId) would propagate and could break the component.Wrap the tracking call in a try-catch to ensure component stability:
useEffect(() => { if (viewerId && viewerId === data.owner.id) { return; } + try { const dispose = trackVideoView({ videoId: data.id, orgId: data.orgId, ownerId: data.owner.id, }); return () => { dispose?.(); }; + } catch (error) { + console.warn("Failed to initialize analytics tracking", error); + } }, [data.id, data.orgId, data.owner.id, viewerId]);packages/web-backend/src/Tinybird/index.ts (2)
160-173: Hard-coded type conversion limits extensibility.Line 168 special-cases the
"views"key for numeric conversion, but this approach doesn't scale. If additional numeric columns are added (e.g., "clicks", "shares"), they won't be converted unless explicitly added.Consider one of these approaches:
Option 1: Define known numeric columns
+ const NUMERIC_COLUMNS = new Set(["views", "clicks", "shares", "count"]); const parseTsvToObjects = <T>(text: string, aliases: string[]) => { // ... for (let i = 0; i < values.length; i++) { const key = aliases[i] ?? `col_${i}`; const raw = values[i] ?? ""; - obj[key] = key === "views" ? Number(raw) : raw; + obj[key] = NUMERIC_COLUMNS.has(key) ? Number(raw) : raw; }Option 2: Attempt numeric conversion for numeric-looking values
- obj[key] = key === "views" ? Number(raw) : raw; + const num = Number(raw); + obj[key] = !Number.isNaN(num) && raw !== "" ? num : raw;Note: TSV format lacks type metadata, so perfect inference is impossible without schema information.
175-206: Consider omitting optional fields instead of empty strings.Lines 181-191 set optional fields to empty strings (e.g.,
session_id: row.session_id ?? "") rather than omitting them. This can:
- Increase storage size unnecessarily
- Make queries less efficient (empty strings vs. true nulls)
- Complicate data analysis
If Tinybird's schema supports nullable fields, consider sending only defined values:
const body = rows .map((row) => JSON.stringify({ ...row, - session_id: row.session_id ?? "", - user_id: row.user_id ?? "", // only include defined values }), ) .join("\n");Verify with Tinybird documentation whether nullable fields are supported in NDJSON ingestion.
apps/web/app/(org)/dashboard/analytics/components/ChartArea.tsx (2)
52-63: Non-null assertion could be unsafe.Line 56 uses a non-null assertion (
data[1]!) which could throw ifdata.length === 1. While line 55 checksdata.length > 1, the assertion bypasses TypeScript's safety.Use optional chaining instead:
const bucketDuration = data.length > 1 - ? new Date(data[1]!.bucket).getTime() - new Date(data[0]!.bucket).getTime() + ? new Date(data[1].bucket).getTime() - new Date(data[0].bucket).getTime() : 0;The check on line 55 already ensures
data[1]exists, so the non-null assertion is redundant and removes TypeScript's safety net.
148-155: Potential X-axis label overlap with large datasets.Line 152 sets
interval={0}which forces rendering all X-axis labels. With many data points (e.g., 24 hours of hourly data), labels could overlap and become unreadable.Consider one of these approaches:
Option 1: Dynamic interval based on data length
+ const tickInterval = Math.max(0, Math.floor(chartData.length / 10)); <XAxis dataKey="label" axisLine={false} tickMargin={10} - interval={0} + interval={tickInterval} height={40} tick={{ fontSize: 11 }} />Option 2: Use angle for rotated labels
<XAxis dataKey="label" axisLine={false} tickMargin={10} interval={0} height={40} - tick={{ fontSize: 11 }} + tick={{ fontSize: 11, angle: -45, textAnchor: 'end' }} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (22)
apps/web/public/logos/browsers/aloha.svgis excluded by!**/*.svgapps/web/public/logos/browsers/bing.svgis excluded by!**/*.svgapps/web/public/logos/browsers/brave.svgis excluded by!**/*.svgapps/web/public/logos/browsers/chronium.svgis excluded by!**/*.svgapps/web/public/logos/browsers/duckduckgo.svgis excluded by!**/*.svgapps/web/public/logos/browsers/explorer.svgis excluded by!**/*.svgapps/web/public/logos/browsers/firefox.svgis excluded by!**/*.svgapps/web/public/logos/browsers/google-chrome.svgis excluded by!**/*.svgapps/web/public/logos/browsers/maxthron.svgis excluded by!**/*.svgapps/web/public/logos/browsers/opera-gx.svgis excluded by!**/*.svgapps/web/public/logos/browsers/opera.svgis excluded by!**/*.svgapps/web/public/logos/browsers/safari.svgis excluded by!**/*.svgapps/web/public/logos/browsers/tor.svgis excluded by!**/*.svgapps/web/public/logos/browsers/vivaldi.svgis excluded by!**/*.svgapps/web/public/logos/browsers/waterfox.svgis excluded by!**/*.svgapps/web/public/logos/browsers/yandex.svgis excluded by!**/*.svgapps/web/public/logos/os/fedora.svgis excluded by!**/*.svgapps/web/public/logos/os/ios.svgis excluded by!**/*.svgapps/web/public/logos/os/linux.svgis excluded by!**/*.svgapps/web/public/logos/os/ubuntu.svgis excluded by!**/*.svgapps/web/public/logos/os/windows.svgis excluded by!**/*.svgpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (66)
.gitignore(1 hunks)README.md(1 hunks)apps/web/actions/videos/get-analytics.ts(1 hunks)apps/web/app/(org)/dashboard/_components/AnimatedIcons/ChartLine.tsx(1 hunks)apps/web/app/(org)/dashboard/_components/AnimatedIcons/Clap.tsx(1 hunks)apps/web/app/(org)/dashboard/_components/AnimatedIcons/Reaction.tsx(1 hunks)apps/web/app/(org)/dashboard/_components/AnimatedIcons/index.ts(2 hunks)apps/web/app/(org)/dashboard/_components/Navbar/Items.tsx(2 hunks)apps/web/app/(org)/dashboard/_components/Navbar/Top.tsx(3 hunks)apps/web/app/(org)/dashboard/analytics/components/AnalyticsDashboard.tsx(1 hunks)apps/web/app/(org)/dashboard/analytics/components/ChartArea.tsx(1 hunks)apps/web/app/(org)/dashboard/analytics/components/CompareFilters.tsx(1 hunks)apps/web/app/(org)/dashboard/analytics/components/FiltersList.tsx(1 hunks)apps/web/app/(org)/dashboard/analytics/components/Header.tsx(1 hunks)apps/web/app/(org)/dashboard/analytics/components/OtherStatBox.tsx(1 hunks)apps/web/app/(org)/dashboard/analytics/components/OtherStats.tsx(1 hunks)apps/web/app/(org)/dashboard/analytics/components/StatsChart.tsx(1 hunks)apps/web/app/(org)/dashboard/analytics/components/TableCard.tsx(1 hunks)apps/web/app/(org)/dashboard/analytics/components/VideoComponents.tsx(1 hunks)apps/web/app/(org)/dashboard/analytics/components/VideoFilters.tsx(1 hunks)apps/web/app/(org)/dashboard/analytics/components/VideosPicker.tsx(1 hunks)apps/web/app/(org)/dashboard/analytics/data.ts(1 hunks)apps/web/app/(org)/dashboard/analytics/page.tsx(1 hunks)apps/web/app/(org)/dashboard/analytics/s/[id]/page.tsx(1 hunks)apps/web/app/(org)/dashboard/analytics/types.ts(1 hunks)apps/web/app/(org)/dashboard/caps/Caps.tsx(3 hunks)apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx(3 hunks)apps/web/app/(org)/dashboard/caps/components/CapCard/CapCardAnalytics.tsx(3 hunks)apps/web/app/(org)/dashboard/caps/page.tsx(1 hunks)apps/web/app/(org)/dashboard/folder/[id]/components/FolderVideosSection.tsx(2 hunks)apps/web/app/(org)/dashboard/folder/[id]/page.tsx(1 hunks)apps/web/app/(org)/dashboard/spaces/[spaceId]/SharedCaps.tsx(2 hunks)apps/web/app/(org)/dashboard/spaces/[spaceId]/folder/[folderId]/page.tsx(1 hunks)apps/web/app/(org)/dashboard/spaces/[spaceId]/page.tsx(2 hunks)apps/web/app/(org)/dashboard/spaces/browse/page.tsx(1 hunks)apps/web/app/api/analytics/route.ts(1 hunks)apps/web/app/api/analytics/track/route.ts(1 hunks)apps/web/app/api/dashboard/analytics/route.ts(1 hunks)apps/web/app/globals.css(1 hunks)apps/web/app/layout.tsx(0 hunks)apps/web/app/s/[videoId]/Share.tsx(5 hunks)apps/web/app/s/[videoId]/page.tsx(1 hunks)apps/web/components/Tooltip.tsx(1 hunks)apps/web/components/ui/chart.tsx(1 hunks)apps/web/lib/Queries/Analytics.ts(1 hunks)apps/web/lib/server.ts(2 hunks)apps/web/package.json(5 hunks)package.json(2 hunks)packages/database/auth/auth-options.ts(0 hunks)packages/env/server.ts(1 hunks)packages/ui/src/components/Button.tsx(1 hunks)packages/ui/src/components/Select.tsx(9 hunks)packages/web-backend/src/Tinybird/index.ts(1 hunks)packages/web-backend/src/Videos/VideosRpcs.ts(1 hunks)packages/web-backend/src/Videos/index.ts(4 hunks)packages/web-backend/src/index.ts(1 hunks)scripts/analytics/check-analytics.js(1 hunks)scripts/analytics/delete-all-data.js(1 hunks)scripts/analytics/populate-test-data.js(1 hunks)scripts/analytics/setup-analytics.js(1 hunks)scripts/analytics/shared.js(1 hunks)scripts/analytics/tinybird/datasources/analytics_events.datasource(1 hunks)scripts/analytics/tinybird/datasources/analytics_pages_mv.datasource(1 hunks)scripts/analytics/tinybird/datasources/analytics_sessions_mv.datasource(1 hunks)scripts/analytics/tinybird/pipes/analytics_pages_mv_pipe.pipe(1 hunks)scripts/analytics/tinybird/pipes/analytics_sessions_mv_pipe.pipe(1 hunks)
💤 Files with no reviewable changes (2)
- packages/database/auth/auth-options.ts
- apps/web/app/layout.tsx
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
apps/web/app/(org)/dashboard/analytics/components/VideosPicker.tsxapps/web/app/(org)/dashboard/analytics/page.tsxapps/web/app/(org)/dashboard/folder/[id]/components/FolderVideosSection.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/SharedCaps.tsxpackages/env/server.tspackages/ui/src/components/Button.tsxapps/web/components/Tooltip.tsxapps/web/lib/Queries/Analytics.tsapps/web/app/(org)/dashboard/_components/AnimatedIcons/Clap.tsxapps/web/app/(org)/dashboard/analytics/components/VideoComponents.tsxapps/web/app/api/analytics/route.tsapps/web/app/(org)/dashboard/analytics/data.tspackages/web-backend/src/index.tsapps/web/app/(org)/dashboard/folder/[id]/page.tsxapps/web/app/(org)/dashboard/caps/page.tsxapps/web/app/(org)/dashboard/analytics/components/AnalyticsDashboard.tsxapps/web/app/(org)/dashboard/caps/Caps.tsxpackages/web-backend/src/Videos/VideosRpcs.tsapps/web/app/(org)/dashboard/analytics/components/VideoFilters.tsxpackages/web-backend/src/Videos/index.tspackages/web-backend/src/Tinybird/index.tsapps/web/app/(org)/dashboard/spaces/[spaceId]/folder/[folderId]/page.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/page.tsxapps/web/app/(org)/dashboard/analytics/components/OtherStats.tsxapps/web/app/(org)/dashboard/_components/AnimatedIcons/Reaction.tsxapps/web/app/(org)/dashboard/_components/Navbar/Top.tsxapps/web/app/s/[videoId]/Share.tsxapps/web/app/(org)/dashboard/analytics/components/Header.tsxapps/web/app/(org)/dashboard/analytics/components/StatsChart.tsxapps/web/app/(org)/dashboard/analytics/s/[id]/page.tsxapps/web/app/(org)/dashboard/analytics/components/ChartArea.tsxapps/web/app/(org)/dashboard/caps/components/CapCard/CapCardAnalytics.tsxapps/web/lib/server.tsapps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsxapps/web/app/(org)/dashboard/spaces/browse/page.tsxapps/web/app/api/dashboard/analytics/route.tsapps/web/app/(org)/dashboard/analytics/types.tsapps/web/app/api/analytics/track/route.tsapps/web/app/(org)/dashboard/analytics/components/FiltersList.tsxapps/web/app/(org)/dashboard/_components/AnimatedIcons/ChartLine.tsxapps/web/app/(org)/dashboard/_components/Navbar/Items.tsxapps/web/app/s/[videoId]/page.tsxapps/web/app/(org)/dashboard/_components/AnimatedIcons/index.tsapps/web/app/(org)/dashboard/analytics/components/CompareFilters.tsxapps/web/app/(org)/dashboard/analytics/components/OtherStatBox.tsxapps/web/components/ui/chart.tsxapps/web/app/(org)/dashboard/analytics/components/TableCard.tsxapps/web/actions/videos/get-analytics.tspackages/ui/src/components/Select.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
apps/web/app/(org)/dashboard/analytics/components/VideosPicker.tsxapps/web/app/(org)/dashboard/analytics/page.tsxapps/web/app/(org)/dashboard/folder/[id]/components/FolderVideosSection.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/SharedCaps.tsxpackages/env/server.tspackages/ui/src/components/Button.tsxapps/web/components/Tooltip.tsxapps/web/lib/Queries/Analytics.tsapps/web/app/(org)/dashboard/_components/AnimatedIcons/Clap.tsxapps/web/app/(org)/dashboard/analytics/components/VideoComponents.tsxapps/web/app/api/analytics/route.tsapps/web/app/(org)/dashboard/analytics/data.tspackages/web-backend/src/index.tsapps/web/app/(org)/dashboard/folder/[id]/page.tsxapps/web/app/(org)/dashboard/caps/page.tsxapps/web/app/(org)/dashboard/analytics/components/AnalyticsDashboard.tsxapps/web/app/(org)/dashboard/caps/Caps.tsxpackages/web-backend/src/Videos/VideosRpcs.tsapps/web/app/(org)/dashboard/analytics/components/VideoFilters.tsxpackages/web-backend/src/Videos/index.tspackages/web-backend/src/Tinybird/index.tsapps/web/app/(org)/dashboard/spaces/[spaceId]/folder/[folderId]/page.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/page.tsxapps/web/app/(org)/dashboard/analytics/components/OtherStats.tsxapps/web/app/(org)/dashboard/_components/AnimatedIcons/Reaction.tsxapps/web/app/(org)/dashboard/_components/Navbar/Top.tsxapps/web/app/s/[videoId]/Share.tsxapps/web/app/(org)/dashboard/analytics/components/Header.tsxapps/web/app/(org)/dashboard/analytics/components/StatsChart.tsxscripts/analytics/delete-all-data.jsscripts/analytics/shared.jsapps/web/app/(org)/dashboard/analytics/s/[id]/page.tsxapps/web/app/(org)/dashboard/analytics/components/ChartArea.tsxapps/web/app/(org)/dashboard/caps/components/CapCard/CapCardAnalytics.tsxapps/web/lib/server.tsapps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsxapps/web/app/(org)/dashboard/spaces/browse/page.tsxscripts/analytics/populate-test-data.jsapps/web/app/api/dashboard/analytics/route.tsapps/web/app/(org)/dashboard/analytics/types.tsapps/web/app/api/analytics/track/route.tsapps/web/app/(org)/dashboard/analytics/components/FiltersList.tsxapps/web/app/(org)/dashboard/_components/AnimatedIcons/ChartLine.tsxapps/web/app/(org)/dashboard/_components/Navbar/Items.tsxapps/web/app/s/[videoId]/page.tsxscripts/analytics/setup-analytics.jsapps/web/app/(org)/dashboard/_components/AnimatedIcons/index.tsapps/web/app/(org)/dashboard/analytics/components/CompareFilters.tsxapps/web/app/(org)/dashboard/analytics/components/OtherStatBox.tsxapps/web/components/ui/chart.tsxapps/web/app/(org)/dashboard/analytics/components/TableCard.tsxscripts/analytics/check-analytics.jsapps/web/actions/videos/get-analytics.tspackages/ui/src/components/Select.tsx
apps/web/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
On the client, always use
useEffectQueryoruseEffectMutationfrom@/lib/EffectRuntime; never callEffectRuntime.run*directly in components.
Files:
apps/web/app/(org)/dashboard/analytics/components/VideosPicker.tsxapps/web/app/(org)/dashboard/analytics/page.tsxapps/web/app/(org)/dashboard/folder/[id]/components/FolderVideosSection.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/SharedCaps.tsxapps/web/components/Tooltip.tsxapps/web/lib/Queries/Analytics.tsapps/web/app/(org)/dashboard/_components/AnimatedIcons/Clap.tsxapps/web/app/(org)/dashboard/analytics/components/VideoComponents.tsxapps/web/app/api/analytics/route.tsapps/web/app/(org)/dashboard/analytics/data.tsapps/web/app/(org)/dashboard/folder/[id]/page.tsxapps/web/app/(org)/dashboard/caps/page.tsxapps/web/app/(org)/dashboard/analytics/components/AnalyticsDashboard.tsxapps/web/app/(org)/dashboard/caps/Caps.tsxapps/web/app/(org)/dashboard/analytics/components/VideoFilters.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/folder/[folderId]/page.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/page.tsxapps/web/app/(org)/dashboard/analytics/components/OtherStats.tsxapps/web/app/(org)/dashboard/_components/AnimatedIcons/Reaction.tsxapps/web/app/(org)/dashboard/_components/Navbar/Top.tsxapps/web/app/s/[videoId]/Share.tsxapps/web/app/(org)/dashboard/analytics/components/Header.tsxapps/web/app/(org)/dashboard/analytics/components/StatsChart.tsxapps/web/app/(org)/dashboard/analytics/s/[id]/page.tsxapps/web/app/(org)/dashboard/analytics/components/ChartArea.tsxapps/web/app/(org)/dashboard/caps/components/CapCard/CapCardAnalytics.tsxapps/web/lib/server.tsapps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsxapps/web/app/(org)/dashboard/spaces/browse/page.tsxapps/web/app/api/dashboard/analytics/route.tsapps/web/app/(org)/dashboard/analytics/types.tsapps/web/app/api/analytics/track/route.tsapps/web/app/(org)/dashboard/analytics/components/FiltersList.tsxapps/web/app/(org)/dashboard/_components/AnimatedIcons/ChartLine.tsxapps/web/app/(org)/dashboard/_components/Navbar/Items.tsxapps/web/app/s/[videoId]/page.tsxapps/web/app/(org)/dashboard/_components/AnimatedIcons/index.tsapps/web/app/(org)/dashboard/analytics/components/CompareFilters.tsxapps/web/app/(org)/dashboard/analytics/components/OtherStatBox.tsxapps/web/components/ui/chart.tsxapps/web/app/(org)/dashboard/analytics/components/TableCard.tsxapps/web/actions/videos/get-analytics.ts
packages/ui/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Component files in
packages/uishould use PascalCase naming if they define React/Solid components.
Files:
packages/ui/src/components/Button.tsxpackages/ui/src/components/Select.tsx
🧠 Learnings (4)
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to apps/web/**/*.{ts,tsx,js,jsx} : On the client, always use `useEffectQuery` or `useEffectMutation` from `@/lib/EffectRuntime`; never call `EffectRuntime.run*` directly in components.
Applied to files:
apps/web/app/s/[videoId]/Share.tsx
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to apps/web/app/api/* : API routes in Next.js (`apps/web/app/api/*`) must use `effect/platform`'s `HttpApi` builder and follow the existing class/group/endpoint pattern; avoid ad-hoc handlers.
Applied to files:
apps/web/app/api/analytics/track/route.ts
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to apps/web/app/api/* : Convert effectful APIs to Next.js handlers using `apiToHandler(ApiLive)` and export only the returned `handler`; avoid calling `runPromise` directly in route files.
Applied to files:
apps/web/app/api/analytics/track/route.ts
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to packages/ui/**/*.{ts,tsx,js,jsx} : Component files in `packages/ui` should use PascalCase naming if they define React/Solid components.
Applied to files:
packages/ui/src/components/Select.tsx
🧬 Code graph analysis (31)
apps/web/app/(org)/dashboard/analytics/components/VideosPicker.tsx (1)
apps/web/app/(org)/dashboard/analytics/components/VideoComponents.tsx (1)
VideoDroppable(101-159)
apps/web/app/(org)/dashboard/analytics/page.tsx (1)
apps/web/app/(org)/dashboard/analytics/components/AnalyticsDashboard.tsx (1)
AnalyticsDashboard(23-123)
apps/web/lib/Queries/Analytics.ts (2)
apps/web/lib/Requests/AnalyticsRequest.ts (1)
AnalyticsRequest(8-12)apps/web/lib/EffectRuntime.ts (1)
useEffectQuery(22-22)
apps/web/app/(org)/dashboard/_components/AnimatedIcons/Clap.tsx (1)
apps/web/app/(org)/dashboard/_components/AnimatedIcons/index.ts (1)
ClapIcon(28-28)
apps/web/app/api/analytics/route.ts (2)
apps/web/app/api/dashboard/analytics/route.ts (1)
GET(11-41)apps/web/actions/videos/get-analytics.ts (1)
getVideoAnalytics(37-99)
apps/web/app/(org)/dashboard/analytics/data.ts (6)
packages/database/schema.ts (3)
videos(291-363)spaceVideos(665-688)comments(394-420)apps/web/app/(org)/dashboard/analytics/types.ts (3)
AnalyticsRange(1-1)OrgAnalyticsResponse(10-33)BreakdownRow(3-8)apps/web/actions/spaces/get-space-videos.ts (1)
getSpaceVideoIds(9-56)packages/database/index.ts (1)
db(18-25)apps/web/lib/server.ts (1)
runPromise(141-153)packages/web-backend/src/Tinybird/index.ts (1)
Tinybird(28-274)
apps/web/app/(org)/dashboard/caps/page.tsx (1)
packages/env/server.ts (1)
serverEnv(136-140)
apps/web/app/(org)/dashboard/analytics/components/AnalyticsDashboard.tsx (5)
apps/web/app/(org)/dashboard/analytics/types.ts (2)
AnalyticsRange(1-1)OrgAnalyticsResponse(10-33)apps/web/app/(org)/dashboard/Contexts.tsx (1)
useDashboardContext(50-50)packages/web-domain/src/Organisation.ts (1)
Organisation(21-24)apps/web/app/(org)/dashboard/analytics/components/Header.tsx (1)
Header(90-429)apps/web/app/(org)/dashboard/analytics/components/OtherStats.tsx (1)
OtherStats(122-214)
apps/web/app/(org)/dashboard/caps/Caps.tsx (1)
apps/web/app/(org)/dashboard/caps/components/Folder.tsx (1)
FolderDataType(18-25)
apps/web/app/(org)/dashboard/analytics/components/VideoFilters.tsx (1)
apps/web/app/(org)/dashboard/analytics/components/VideoComponents.tsx (1)
DraggableVideoItem(19-90)
packages/web-backend/src/Videos/index.ts (3)
packages/web-backend/src/Tinybird/index.ts (1)
Tinybird(28-274)packages/web-backend/src/Videos/VideosPolicy.ts (1)
VideosPolicy(9-96)packages/web-backend/src/Videos/VideosRepo.ts (1)
VideosRepo(15-106)
packages/web-backend/src/Tinybird/index.ts (3)
packages/web-backend/src/index.ts (1)
Tinybird(18-18)packages/env/server.ts (1)
serverEnv(136-140)scripts/analytics/shared.js (2)
request(134-165)raw(88-88)
apps/web/app/(org)/dashboard/analytics/components/OtherStats.tsx (3)
apps/web/app/(org)/dashboard/analytics/types.ts (1)
BreakdownRow(3-8)apps/web/app/(org)/dashboard/analytics/components/TableCard.tsx (6)
DeviceRowData(100-107)CountryRowData(64-71)CityRowData(73-80)BrowserRowData(82-89)OSRowData(91-98)CapRowData(109-116)apps/web/app/(org)/dashboard/analytics/components/OtherStatBox.tsx (1)
OtherStatBox(12-32)
apps/web/app/(org)/dashboard/analytics/components/Header.tsx (4)
apps/web/app/(org)/dashboard/analytics/types.ts (1)
AnalyticsRange(1-1)packages/web-domain/src/Organisation.ts (1)
Organisation(21-24)apps/web/app/Layout/AuthContext.tsx (1)
useCurrentUser(40-42)apps/web/components/SignedImageUrl.tsx (1)
SignedImageUrl(11-25)
apps/web/app/(org)/dashboard/analytics/components/StatsChart.tsx (2)
apps/web/app/(org)/dashboard/_components/AnimatedIcons/Cap.tsx (1)
CapIconHandle(8-11)apps/web/app/(org)/dashboard/_components/AnimatedIcons/index.ts (4)
ClapIcon(28-28)ChatIcon(29-29)ReactionIcon(30-30)CapIcon(18-18)
scripts/analytics/delete-all-data.js (3)
scripts/analytics/check-analytics.js (2)
client(154-154)datasource(46-46)scripts/analytics/setup-analytics.js (1)
auth(80-80)scripts/analytics/shared.js (1)
auth(132-132)
scripts/analytics/shared.js (2)
scripts/analytics/delete-all-data.js (2)
auth(68-68)payload(8-8)scripts/analytics/setup-analytics.js (1)
auth(80-80)
apps/web/app/(org)/dashboard/analytics/s/[id]/page.tsx (3)
apps/web/app/(org)/dashboard/analytics/page.tsx (1)
AnalyticsPage(3-5)apps/web/app/(org)/dashboard/analytics/components/Header.tsx (1)
Header(90-429)apps/web/app/(org)/dashboard/analytics/components/OtherStats.tsx (2)
OtherStats(122-214)OtherStatsData(20-27)
apps/web/app/(org)/dashboard/analytics/components/ChartArea.tsx (1)
apps/web/components/ui/chart.tsx (4)
ChartConfig(10-18)ChartContainer(354-354)ChartTooltip(355-355)ChartTooltipContent(356-356)
apps/web/lib/server.ts (2)
packages/web-backend/src/Tinybird/index.ts (1)
Tinybird(28-274)packages/web-backend/src/index.ts (1)
Tinybird(18-18)
apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx (2)
apps/web/app/(org)/dashboard/caps/components/CapCard/CapCardButton.tsx (1)
CapCardButton(18-46)packages/ui/src/components/Dropdown.tsx (4)
DropdownMenu(191-191)DropdownMenuTrigger(205-205)DropdownMenuContent(193-193)DropdownMenuItem(195-195)
scripts/analytics/populate-test-data.js (1)
scripts/analytics/migrate-dub-to-tinybird.js (17)
os(5188-5188)search(4736-4736)url(4428-4428)url(4737-4737)response(4433-4435)response(4738-4746)text(4439-4439)text(4747-4747)message(4450-4450)message(4767-4767)videoIds(5345-5345)i(4116-4116)i(4180-4180)i(4721-4721)i(5309-5309)totalWritten(5410-5410)chunks(5308-5308)
apps/web/app/api/dashboard/analytics/route.ts (2)
apps/web/app/(org)/dashboard/analytics/types.ts (1)
AnalyticsRange(1-1)apps/web/app/(org)/dashboard/analytics/data.ts (1)
getOrgAnalyticsData(119-294)
apps/web/app/api/analytics/track/route.ts (3)
apps/web/lib/server.ts (1)
runPromise(141-153)packages/web-backend/src/Tinybird/index.ts (1)
Tinybird(28-274)packages/web-backend/src/Auth.ts (1)
provideOptionalAuth(102-114)
apps/web/app/(org)/dashboard/analytics/components/FiltersList.tsx (1)
apps/web/app/(org)/dashboard/analytics/components/CompareFilters.tsx (2)
FilterValue(33-33)CompareDataFilterItem(44-119)
apps/web/app/(org)/dashboard/_components/AnimatedIcons/ChartLine.tsx (1)
apps/web/app/(org)/dashboard/_components/AnimatedIcons/index.ts (1)
ChartLineIcon(27-27)
apps/web/app/(org)/dashboard/_components/Navbar/Items.tsx (4)
apps/web/app/(org)/dashboard/Contexts.tsx (1)
useDashboardContext(50-50)apps/web/app/(org)/dashboard/_components/AnimatedIcons/index.ts (4)
CapIcon(18-18)ChartLineIcon(27-27)RecordIcon(31-31)CogIcon(20-20)apps/web/components/SignedImageUrl.tsx (1)
SignedImageUrl(11-25)apps/web/app/(org)/dashboard/_components/AnimatedIcons/Cog.tsx (1)
CogIconHandle(8-11)
scripts/analytics/setup-analytics.js (2)
scripts/analytics/shared.js (2)
PROJECT_ROOT(9-9)auth(132-132)scripts/analytics/delete-all-data.js (1)
auth(68-68)
apps/web/app/(org)/dashboard/analytics/components/TableCard.tsx (2)
packages/ui/src/components/Table.tsx (6)
Table(117-117)TableHeader(123-123)TableRow(124-124)TableHead(122-122)TableBody(118-118)TableCell(120-120)packages/ui/src/components/icons/LogoBadge.tsx (1)
LogoBadge(1-28)
scripts/analytics/check-analytics.js (1)
scripts/analytics/shared.js (4)
normalizeWhitespace(74-74)buildSchemaLines(76-76)TABLE_DEFINITIONS(16-67)PIPE_DEFINITIONS(69-72)
apps/web/actions/videos/get-analytics.ts (5)
packages/database/index.ts (1)
db(18-25)packages/database/schema.ts (1)
videos(291-363)packages/web-domain/src/Video.ts (1)
Video(16-59)apps/web/lib/server.ts (1)
runPromise(141-153)packages/web-backend/src/Tinybird/index.ts (1)
Tinybird(28-274)
🪛 ast-grep (0.39.9)
apps/web/components/ui/chart.tsx
[warning] 80-80: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 GitHub Actions: CI
apps/web/app/(org)/dashboard/analytics/data.ts
[error] 1-1: File content differs from formatting output
packages/web-backend/src/index.ts
[error] 1-1: File content differs from formatting output
[error] 1-1: File content differs from formatting output
apps/web/app/(org)/dashboard/analytics/components/AnalyticsDashboard.tsx
[error] 13-14: File content differs from formatting output
packages/web-backend/src/Videos/VideosRpcs.ts
[error] 111-111: File content differs from formatting output
packages/web-backend/src/Videos/index.ts
[error] 17-17: File content differs from formatting output
packages/web-backend/src/Tinybird/index.ts
[error] 1-1: File content differs from formatting output
apps/web/app/(org)/dashboard/analytics/components/Header.tsx
[error] 13-13: File content differs from formatting output
scripts/analytics/delete-all-data.js
[error] 1-1: File content differs from formatting output
apps/web/app/(org)/dashboard/analytics/components/ChartArea.tsx
[error] 54-54: File content differs from formatting output
apps/web/app/(org)/dashboard/caps/components/CapCard/CapCardAnalytics.tsx
[error] 1-1: File content differs from formatting output
[error] 1-1: File content differs from formatting output
scripts/analytics/populate-test-data.js
[error] 6-6: File content differs from formatting output
apps/web/app/(org)/dashboard/_components/Navbar/Items.tsx
[error] 1-1: The imports and exports are not sorted.
scripts/analytics/setup-analytics.js
[error] 17-17: File content differs from formatting output
scripts/analytics/check-analytics.js
[error] 3-3: File content differs from formatting output
apps/web/actions/videos/get-analytics.ts
[error] 1-1: The imports and exports are not sorted.
🪛 GitHub Check: CodeQL
scripts/analytics/populate-test-data.js
[failure] 56-56: Insecure randomness
This uses a cryptographically insecure random number generated at Math.random() in a security context.
🪛 GitHub Check: Typecheck
apps/web/app/(org)/dashboard/analytics/s/[id]/page.tsx
[failure] 46-46:
Type 'string' is not assignable to type 'number'.
[failure] 43-43:
Type 'string' is not assignable to type 'number'.
[failure] 38-38:
Type 'string' is not assignable to type 'number'.
[failure] 35-35:
Type 'string' is not assignable to type 'number'.
[failure] 30-30:
Type 'string' is not assignable to type 'number'.
[failure] 27-27:
Type 'string' is not assignable to type 'number'.
[failure] 13-13:
Type 'string' is not assignable to type 'number'.
[failure] 11-11:
Type 'string' is not assignable to type 'number'.
[failure] 8-8:
Type '{}' is missing the following properties from type 'HeaderProps': options, value, onChange
🪛 LanguageTool
README.md
[typographical] ~66-~66: Consider adding a comma here.
Context: ... in your environment. Once the token is present you can: - Provision the required data...
(IF_THAT_S_NOT_POSSIBLE_COMMA)
[grammar] ~71-~71: The word ‘deploy’ is a verb. Did you mean the noun “deployment” (= release, placement)?
Context: ...able with the destructive nature of the deploy step before running analytics:setup. ...
(PREPOSITION_VERB)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (rust)
🔇 Additional comments (39)
apps/web/app/(org)/dashboard/spaces/browse/page.tsx (1)
3-3: ✓ Avatar import removal looks good.The
Avatarimport has been removed from the "@cap/ui" import statement, and there are no usages ofAvatarin the component. TheSignedImageUrlcomponent is correctly being used for space icon rendering instead. This change is clean and isolated.apps/web/app/globals.css (1)
339-346: Verify visual impact of removingfont-normal.Removing the
font-normalutility from this selector group means these elements (a, p, span, input, label, button) will now inherit font-weight from their parent instead of forcing normal weight. This could cause visual inconsistencies if parent elements havefont-weightvalues other thannormalapplied.Confirm that this change has been tested and doesn't cause unintended visual regressions across the UI.
packages/ui/src/components/Button.tsx (1)
24-24: LGTM! White variant styling updated appropriately.The updated styling provides a well-defined hover progression (gray-3 → gray-6) and maintains consistency with the structure of other button variants. The change aligns with the broader UI theming work for the analytics feature.
README.md (1)
64-71: Appreciate the precise Tinybird setup notes. Clear callouts about the destructive deploy make it easy for operators to assess the risk before running the scripts..gitignore (1)
38-41: Good call adding Tinybird credential ignores. This keeps the copied.tinybadmin tokens out of version control.apps/web/components/Tooltip.tsx (1)
15-29: ForwardingdelayDurationlooks solid. Optional prop preserves existing behavior while enabling finer control when needed.scripts/analytics/tinybird/pipes/analytics_pages_mv_pipe.pipe (1)
1-18: Materialized view query looks correct. AggregatinguniqState(session_id)across tenant/path/date will feed the MV as expected.apps/web/app/s/[videoId]/page.tsx (1)
743-752: Thanks for threadingviewerIdthrough. This keeps the page ready for the new analytics hooks without altering existing behavior.scripts/analytics/delete-all-data.js (1)
1-135: Fix formatting to unblock CI. The pipeline is failing with “File content differs from formatting output.” Please runpnpm format(Biome) so the script matches the expected formatting.apps/web/actions/videos/get-analytics.ts (1)
3-12: Fix import ordering to satisfy Biome & unblock CICI is red with “The imports and exports are not sorted.” Please run the project formatter (
pnpm format/ Biome) so the import block is reordered as expected before rebasing.apps/web/app/(org)/dashboard/analytics/page.tsx (1)
1-5: LGTM!Clean implementation of the analytics page entry point following React and Next.js conventions.
packages/env/server.ts (1)
95-98: LGTM!Properly configured optional Tinybird environment variables with clear documentation.
apps/web/lib/server.ts (1)
20-20: LGTM!Correctly integrates Tinybird service into the Effect runtime dependencies layer.
Also applies to: 117-117
apps/web/app/(org)/dashboard/spaces/[spaceId]/folder/[folderId]/page.tsx (1)
106-108: LGTM!Correctly gates analytics on both Tinybird environment variables, aligning with the migration from Dub to Tinybird-based analytics.
apps/web/package.json (1)
29-30: LGTM!Appropriate dependency additions for the analytics dashboard functionality, including charting (recharts), internationalization (country-flag-icons), drag-and-drop (@dnd-kit), and user agent parsing (ua-parser-js).
Also applies to: 87-87, 125-125, 134-134, 150-150
apps/web/app/(org)/dashboard/folder/[id]/components/FolderVideosSection.tsx (1)
19-19: LGTM!Clean refactor renaming
dubApiKeyEnabledtoanalyticsEnabled, aligning with the broader migration from Dub-based analytics to Tinybird integration.Also applies to: 24-24, 112-112
apps/web/app/(org)/dashboard/caps/components/CapCard/CapCardAnalytics.tsx (1)
35-82: LGTM!Making analytics metrics clickable is a good UX improvement. The consistent routing to
/dashboard/analytics?capId=${capId}enables users to drill down into detailed analytics for each cap.apps/web/app/(org)/dashboard/caps/page.tsx (1)
269-271: LGTM!The migration from
dubApiKeyEnabledtoanalyticsEnabledwith Tinybird credentials correctly gates the analytics feature based on the new Tinybird integration.scripts/analytics/tinybird/datasources/analytics_sessions_mv.datasource (1)
1-16: LGTM!The session-level analytics datasource is well-configured with appropriate partitioning by month, a composite sorting key optimized for tenant-scoped queries, and a 90-day TTL for data retention. The use of
LowCardinalityfor high-cardinality string fields is a good optimization.apps/web/app/(org)/dashboard/spaces/[spaceId]/SharedCaps.tsx (2)
56-61: LGTM!The prop rename from
dubApiKeyEnabledtoanalyticsEnabledis consistent with the Tinybird migration across the codebase.
99-102: LGTM!The analytics query correctly uses the renamed
analyticsEnabledprop.apps/web/app/(org)/dashboard/spaces/[spaceId]/page.tsx (2)
261-263: LGTM!Consistent migration to Tinybird-based analytics gating for the space variant.
367-369: LGTM!Consistent migration to Tinybird-based analytics gating for the organization variant.
scripts/analytics/tinybird/datasources/analytics_events.datasource (2)
4-24: LGTM!The schema is comprehensive with appropriate field types, LowCardinality optimizations for categorical data, and sensible defaults. The MergeTree configuration with monthly partitioning and 90-day TTL is well-designed.
26-27: The FORWARD_QUERY is valid and complete—no issues to address.Tinybird's FORWARD_QUERY specification requires only a SELECT column-selection part; FROM and WHERE clauses are not supported. The query in the datasource file follows this syntax correctly. The review comment incorrectly assumes a FROM clause is needed; it is not.
Likely an incorrect or invalid review comment.
scripts/analytics/populate-test-data.js (4)
45-56: Cryptographically insecure random is acceptable for test data.The static analysis tool flags
Math.random()usage in session ID generation as cryptographically insecure. For test data generation, this is acceptable since we're not generating security-sensitive values. However, ensure this script is never used to generate production data.
114-116: Verify SSL configuration for database connection.The
rejectUnauthorized: falsesetting bypasses SSL certificate verification, which is insecure. This may be necessary for development environments, but ensure this script is documented as dev-only and never used against production databases.
171-227: LGTM!The main function orchestrates test data generation well with proper error handling, progress logging, and chunked ingestion. The distribution algorithm ensures realistic view counts across videos.
6-6: Fix formatting.#!/bin/bash pnpm format scripts/analytics/populate-test-data.js⛔ Skipped due to learnings
Learnt from: CR Repo: CapSoftware/Cap PR: 0 File: AGENTS.md:0-0 Timestamp: 2025-09-22T14:19:56.010Z Learning: Applies to **/*.{ts,tsx} : Use Biome for formatting and linting TypeScript/JavaScript files by running `pnpm format`.Learnt from: CR Repo: CapSoftware/Cap PR: 0 File: AGENTS.md:0-0 Timestamp: 2025-09-22T14:19:56.010Z Learning: Always run `pnpm db:generate` followed by `pnpm db:push` before relying on new schema changes.packages/web-backend/src/Videos/VideosRpcs.ts (1)
111-125: No actionable issues found; formatting is correct and error handling is valid.The code at lines 111-125 uses proper 2-space indentation and follows the established pattern. While the error handling differs from
VideosGetThumbnails, this is not a defect:
getAnalyticsBulkInternalreturnsArray<Exit.Exit<{ count: number }, unknown>>, meaning per-item errors are already wrapped as Exit objects at the backend- The
catchTaghandlers catch batch-level errors (database unreachable), which is appropriate- Per-item failures remain accessible via the Exit-wrapped results, enabling partial success handling at the client layer (similar to thumbnails)
Both RPC implementations use valid patterns—
VideosGetThumbnailswraps per-call;VideosGetAnalyticswraps per-result in the backend function.Likely an incorrect or invalid review comment.
apps/web/app/(org)/dashboard/caps/Caps.tsx (1)
58-84: LGTM! Clean prop rename with proper loading state handling.The refactoring from
dubApiKeyEnabledtoanalyticsEnabledis well-executed. The derivedisLoadingAnalyticsstate correctly gates loading on both analytics being enabled and the query loading, preventing unnecessary loading indicators when analytics is disabled.apps/web/app/s/[videoId]/Share.tsx (2)
139-155: LGTM! Proper viewer tracking with self-exclusion.The addition of the
viewerIdprop and its use to skip self-tracking (line 279) is well-implemented. The flexible typestring | null | undefinedaccommodates various authentication states.Also applies to: 233-241
41-63: Session ID fallback to "anonymous" impacts data quality.When localStorage operations fail (line 60), the code falls back to
"anonymous"as the session ID. While this prevents tracking errors from breaking functionality, it may reduce analytics accuracy if many users hit this error path, as all failed sessions will be conflated.Consider adding telemetry to monitor how often this fallback occurs:
} catch (error) { console.warn("Failed to persist analytics session id", error); + // Consider: report this to your error tracking service return "anonymous"; }scripts/analytics/setup-analytics.js (2)
76-88: LGTM! Well-structured setup script with proper error handling.The main function orchestrates the setup flow logically: ensure CLI → ensure login → resolve auth → sync credentials → deploy. Error handling via try-catch with clear error messages is appropriate.
1-90: Run formatter to fix formatting issues.The pipeline indicates a formatting error on line 17. Please run the formatter to fix this issue.
#!/bin/bash # Run biome formatter on the file pnpm format⛔ Skipped due to learnings
Learnt from: CR Repo: CapSoftware/Cap PR: 0 File: AGENTS.md:0-0 Timestamp: 2025-09-22T14:19:56.010Z Learning: Applies to **/*.{ts,tsx} : Use Biome for formatting and linting TypeScript/JavaScript files by running `pnpm format`.packages/web-backend/src/Tinybird/index.ts (2)
53-115: LGTM! Robust request helper with comprehensive error handling.The request helper properly handles:
- Early return when disabled (line 54)
- Non-OK responses with detailed logging (lines 71-79)
- JSON parsing failures with response preview (lines 87-98)
- Multiple response formats via normalization (lines 101-105)
1-3: Run formatter to fix formatting issues.The pipeline indicates a formatting error on line 1. Please run the formatter as per the coding guidelines.
#!/bin/bash # Run biome formatter on the file pnpm format⛔ Skipped due to learnings
Learnt from: CR Repo: CapSoftware/Cap PR: 0 File: AGENTS.md:0-0 Timestamp: 2025-09-22T14:19:56.010Z Learning: Applies to **/*.{ts,tsx} : Use Biome for formatting and linting TypeScript/JavaScript files by running `pnpm format`.Learnt from: CR Repo: CapSoftware/Cap PR: 0 File: AGENTS.md:0-0 Timestamp: 2025-09-22T14:19:56.010Z Learning: Applies to apps/web/app/api/* : On the server, always run effects through `EffectRuntime.runPromise` after `provideOptionalAuth` to ensure cookies and per-request context are attached.Learnt from: CR Repo: CapSoftware/Cap PR: 0 File: AGENTS.md:0-0 Timestamp: 2025-09-22T14:19:56.010Z Learning: Applies to apps/web/**/*.{ts,tsx,js,jsx} : On the client, always use `useEffectQuery` or `useEffectMutation` from `@/lib/EffectRuntime`; never call `EffectRuntime.run*` directly in components.apps/web/app/(org)/dashboard/analytics/components/ChartArea.tsx (1)
65-106: LGTM! Well-designed Y-axis scaling logic.The Y-axis calculation intelligently:
- Handles edge cases (no data, no metrics, max = 0)
- Rounds to "nice" values (1, 2, 5, 10 multiples) for readability
- Generates evenly spaced ticks
The magnitude-based rounding ensures axis labels are human-friendly.
apps/web/app/(org)/dashboard/analytics/components/Header.tsx (1)
1-30: Run the formatter to unblock CICI is failing with “File content differs from formatting output.” Please run
pnpm format(Biome) on this file so the pipeline passes.
apps/web/app/(org)/dashboard/analytics/components/AnalyticsDashboard.tsx
Outdated
Show resolved
Hide resolved
| return ( | ||
| <div | ||
| ref={ref} | ||
| className={clsx( | ||
| "h-6 w-[100px] transition-all border rounded-full file:flex items-center", | ||
| isDragging && !isOver && !droppedValue | ||
| ? "bg-gray-5 border-gray-11" | ||
| : isOver && isDragging | ||
| ? "bg-transparent border-blue-500" | ||
| : !droppedValue | ||
| ? "border-dashed border-gray-7" | ||
| : "border-transparent", | ||
| )} |
There was a problem hiding this comment.
Fix droppable layout and forward the id prop.
The droppable wrapper drops the incoming id, so callers (and tests) can’t actually address the zone even though the prop exists. On top of that, file:flex is a Tailwind* variant that never applies here, so the container isn’t flexed and the alignment/hit-box styling regresses.
Apply this diff:
-export const CompareDataDroppable = React.forwardRef<
- HTMLDivElement,
- CompareDataDroppableProps
->(({ droppedValue, onRemove, isDragging, dragPosition }, ref) => {
+export const CompareDataDroppable = React.forwardRef<
+ HTMLDivElement,
+ CompareDataDroppableProps
+>(({ id, droppedValue, onRemove, isDragging, dragPosition }, ref) => {
@@
- <div
- ref={ref}
- className={clsx(
- "h-6 w-[100px] transition-all border rounded-full file:flex items-center",
+ <div
+ ref={ref}
+ id={id}
+ className={clsx(
+ "h-6 w-[100px] transition-all border rounded-full flex items-center",Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/web/app/(org)/dashboard/analytics/components/CompareFilters.tsx around
lines 156 to 168, the droppable wrapper is dropping the incoming id prop and
uses the incorrect Tailwind variant "file:flex" so the container isn't flexed;
fix by forwarding the id prop to the returned div (e.g., <div id={id} ...>) so
callers/tests can target the zone, and replace "file:flex" with "flex" in the
className so the container is actually a flexbox and alignment/hit-box styles
work as intended.
| <SelectPrimitive.Viewport className="p-1"> | ||
| <SelectPrimitive.Item | ||
| key={activeOrganization.organization.id} | ||
| value={activeOrganization.organization.id} | ||
| className="relative flex w-full cursor-pointer items-center gap-2 rounded-lg px-3 py-2 text-sm outline-none transition-colors font-semibold text-gray-12 data-[state=checked]:bg-gray-3 data-[highlighted]:bg-gray-3 data-[highlighted]:text-gray-12" | ||
| > | ||
| <SignedImageUrl | ||
| image={ | ||
| user?.imageUrl || activeOrganization.organization.iconUrl | ||
| } | ||
| name={user?.name || activeOrganization.organization.name} | ||
| className="size-5 flex-shrink-0" | ||
| /> | ||
| <SelectPrimitive.ItemText> | ||
| {user?.name ? `${user.name}'s Caps` : "My Caps"} | ||
| </SelectPrimitive.ItemText> | ||
| </SelectPrimitive.Item> | ||
| {filteredSpaces && filteredSpaces.length > 0 && ( | ||
| <> | ||
| <div className="px-3 py-2 text-xs font-semibold text-gray-9 uppercase tracking-wider"> | ||
| Spaces | ||
| </div> | ||
| {filteredSpaces.map((space) => { | ||
| return ( | ||
| <SelectPrimitive.Item | ||
| key={space.id} | ||
| value={`space:${space.id}`} | ||
| className="relative flex w-full cursor-pointer items-center gap-2 rounded-lg px-3 py-2 text-sm outline-none transition-colors text-gray-12 data-[state=checked]:bg-gray-3 data-[highlighted]:bg-gray-3 data-[highlighted]:text-gray-12" | ||
| > | ||
| <SignedImageUrl | ||
| image={space.iconUrl} | ||
| name={space.name} | ||
| className="size-5 flex-shrink-0" | ||
| /> | ||
| <SelectPrimitive.ItemText> | ||
| {space.name} | ||
| </SelectPrimitive.ItemText> | ||
| </SelectPrimitive.Item> | ||
| ); | ||
| })} | ||
| </> | ||
| )} | ||
| </SelectPrimitive.Viewport> |
There was a problem hiding this comment.
Restore organization switching options
The organization selector now renders only the active organization plus its spaces; the rest of organizations is never surfaced, so users can no longer switch to other orgs. That’s a regression from the previous behavior. Please restore the list of organizations in this menu (e.g., iterate organizations and render each one) before shipping.
🤖 Prompt for AI Agents
In apps/web/app/(org)/dashboard/analytics/components/Header.tsx around lines 303
to 345, the dropdown only lists the active organization and its spaces so users
cannot switch to other organizations; iterate the organizations array and render
a SelectPrimitive.Item for each organization (skipping or de-duplicating the
active org if desired) before rendering the Spaces section: for each org use
key={org.id} value={org.id}, render SignedImageUrl with org.iconUrl and
org.name, and SelectPrimitive.ItemText with org.name; optionally add a small
"Organizations" header above the list to match existing UI patterns.
| <SelectPrimitive.ItemText>{children}</SelectPrimitive.ItemText> | ||
| {icon && React.cloneElement(icon as React.ReactElement<{ className: string }>, { className: cx("size-3", "text-gray-9") })} | ||
| </div> |
There was a problem hiding this comment.
Apply the same safe icon handling here.
The same two issues occur for item icons: cloning a generic React.ReactNode will crash for valid inputs, and we drop any caller-supplied className. Mirror the guarded merge used above.
- {icon && React.cloneElement(icon as React.ReactElement<{ className: string }>, { className: cx("size-3", "text-gray-9") })}
+ {React.isValidElement(icon)
+ ? React.cloneElement(icon, {
+ className: cx("size-3", "text-gray-9", icon.props.className),
+ })
+ : null}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <SelectPrimitive.ItemText>{children}</SelectPrimitive.ItemText> | |
| {icon && React.cloneElement(icon as React.ReactElement<{ className: string }>, { className: cx("size-3", "text-gray-9") })} | |
| </div> | |
| <SelectPrimitive.ItemText>{children}</SelectPrimitive.ItemText> | |
| {React.isValidElement(icon) | |
| ? React.cloneElement(icon, { | |
| className: cx("size-3", "text-gray-9", icon.props.className), | |
| }) | |
| : null} | |
| </div> |
🤖 Prompt for AI Agents
In packages/ui/src/components/Select.tsx around lines 275 to 277, the item icon
is being cloned unconditionally which can crash for non-element ReactNodes and
it overwrites any caller-supplied className; change this to mirror the guarded
pattern used elsewhere: check React.isValidElement(icon) before cloning, merge
the existing icon.props.className with "size-3" and "text-gray-9" via cx and
pass through all other props when cloning, and if icon is not a valid React
element simply render it as-is; ensure types reflect React.ReactNode and no
className from the caller is lost.
packages/web-backend/src/index.ts
Outdated
| export { Videos } from "./Videos/index.ts"; | ||
| export { VideosPolicy } from "./Videos/VideosPolicy.ts"; | ||
| export { VideosRepo } from "./Videos/VideosRepo.ts"; | ||
| export { Tinybird } from "./Tinybird/index.ts"; |
There was a problem hiding this comment.
Run formatter to fix pipeline failure.
The pipeline indicates this file needs formatting.
Run the following command to fix the formatting:
#!/bin/bash
pnpm format🤖 Prompt for AI Agents
In packages/web-backend/src/index.ts around line 18, the file is improperly
formatted causing the CI pipeline to fail; run the repository formatter to fix
whitespace/formatting issues by executing the project's format command (pnpm
format) locally, then stage and commit the resulting changes so the pipeline can
pass.
| export class Tinybird extends Effect.Service<Tinybird>()("Tinybird", { | ||
| effect: Effect.gen(function* () { | ||
| const env = serverEnv(); | ||
| const token = env.TINYBIRD_TOKEN; | ||
| const host = env.TINYBIRD_HOST; | ||
|
|
||
| if (!host) { | ||
| yield* Effect.die( | ||
| new Error("TINYBIRD_HOST must be set"), | ||
| ); | ||
| } | ||
|
|
||
| yield* Effect.logDebug("Initializing Tinybird service", { | ||
| hasToken: Boolean(token), | ||
| host, | ||
| }); | ||
|
|
||
| const enabled = Boolean(token); | ||
|
|
||
| if (!enabled) { | ||
| yield* Effect.logWarning( | ||
| "Tinybird is disabled: TINYBIRD_TOKEN is not set", | ||
| ); | ||
| } |
There was a problem hiding this comment.
Inconsistent environment variable validation.
The service dies if TINYBIRD_HOST is missing (line 35) but only logs a warning if TINYBIRD_TOKEN is missing (lines 48-50). This creates an inconsistency: why require host if the service can operate in a disabled mode without a token?
Consider one of the following approaches:
Option 1: Make both optional for disabled mode
if (!host) {
- yield* Effect.die(
- new Error("TINYBIRD_HOST must be set"),
- );
+ yield* Effect.logWarning(
+ "Tinybird is disabled: TINYBIRD_HOST is not set",
+ );
}
- const enabled = Boolean(token);
+ const enabled = Boolean(token && host);Option 2: Require both or neither
- if (!host) {
+ if (!host || !token) {
yield* Effect.die(
- new Error("TINYBIRD_HOST must be set"),
+ new Error("Both TINYBIRD_HOST and TINYBIRD_TOKEN must be set"),
);
}🤖 Prompt for AI Agents
In packages/web-backend/src/Tinybird/index.ts around lines 28 to 51, the code
currently dies when TINYBIRD_HOST is missing but only logs a warning when
TINYBIRD_TOKEN is missing, causing inconsistent validation; pick one approach
and make the validation consistent: either (Option 1) treat both as optional for
a disabled mode—only log a warning and set enabled = false when token is absent
and do not die for missing host, or (Option 2) require both or neither—if either
token or host is provided, validate that both exist and call Effect.die with a
clear Error when one is missing; update the logging to reflect the chosen
behavior and ensure enabled is computed consistently from token+host presence.
| tenant_id LowCardinality(String), | ||
| date Date, | ||
| pathname String, | ||
| country LowCardinality(String), | ||
| region LowCardinality(String), | ||
| city LowCardinality(String), | ||
| visits AggregateFunction(uniq, String) |
There was a problem hiding this comment.
Realign this schema with the shared Tinybird definitions.
TABLE_DEFINITIONS.analytics_pages_mv in scripts/analytics/shared.js still lists a single location column, but this datasource now exposes country, region, and city. The new check-analytics validator will flag the mismatch and any automated provisioning scripts will fail. Update the shared table definition (and any other schema references) to reflect the new columns before landing this change.
🤖 Prompt for AI Agents
In scripts/analytics/tinybird/datasources/analytics_pages_mv.datasource around
lines 5 to 11, the datasource schema now exposes country, region, and city but
the shared TABLE_DEFINITIONS.analytics_pages_mv in scripts/analytics/shared.js
still declares a single location column; update that shared definition to
replace the single location column with LowCardinality(String) country, region,
and city (and keep other types/columns consistent), and then search the repo for
any other references to analytics_pages_mv or the location column (including
validation/config/provisioning scripts) and update them to use the new
country/region/city columns so the check-analytics validator and automated
provisioning will pass.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/web-backend/src/Videos/VideosRpcs.ts (1)
111-123: MoveprovideOptionalAuthbeforeEffect.map()to fix the type inference error.The
getAnalyticsBulkhandler appliesprovideOptionalAuthafterEffect.map, unlike similar handlers such asGetUploadProgresswhich apply it before any transformations. TheprovideOptionalAuthfunction modifies the error channel by addingDatabaseError | UnknownException, which must be applied before downstream operators for proper type inference.Reorder the pipeline:
VideosGetAnalytics: (videoIds) => videos.getAnalyticsBulk(videoIds).pipe( provideOptionalAuth, Effect.map((results) => results.map((result) => Unify.unify(result))), Effect.catchTag( "DatabaseError", () => new InternalError({ type: "database" }), ), Effect.catchTag( "UnknownException", () => new InternalError({ type: "unknown" }), ), ),
♻️ Duplicate comments (13)
apps/web/app/api/analytics/track/route.ts (2)
25-106: Rebuild the route with HttpApi pattern
This still exports a barePOSThandler and invokesrunPromisedirectly, bypassing the required@effect/platformHttpApibuilder +apiToHandlerflow forapps/web/app/api/*. Please refactor the route into the shared HttpApi describe/create pattern so we inherit the standard middleware and per-request wiring.
Based on learnings
47-100: Guard against invalidoccurredAtbeforetoISOString
If a client sends an invalid timestamp,new Date(body.occurredAt)becomesInvalid Dateandtimestamp.toISOString()throws, returning a 500. Add an explicit guard that returns a 400 whengetTime()isNaN.- const timestamp = body.occurredAt ? new Date(body.occurredAt) : new Date(); + const rawTimestamp = body.occurredAt ? new Date(body.occurredAt) : new Date(); + if (Number.isNaN(rawTimestamp.getTime())) { + return Response.json( + { error: "occurredAt must be a valid ISO timestamp" }, + { status: 400 }, + ); + } + const timestamp = rawTimestamp;apps/web/app/(org)/dashboard/_components/Navbar/Items.tsx (1)
94-95: The issues identified in previous reviews remain unresolved.The past review comments accurately identify two problems with this function:
Misleading parameter name:
ignoreParamssuggests query parameter handling, but it actually controls exact (pathname === path) vs. substring (pathname.includes(path)) pathname matching.Multiple active items: The
pathname.includes(path)logic causes overlapping matches. For example, when viewing/dashboard/caps/record, both "My Caps" (/dashboard/caps) and "Record a Cap" (/dashboard/caps/record) will be active simultaneously, breaking thelayoutId="navlinks"animation.Consider the approach suggested in the previous review—rename the parameter to reflect its actual purpose (e.g.,
exact) and usepathname.startsWith(path + '/')or segment-based matching instead ofincludes:-const isPathActive = (path: string, ignoreParams: boolean = false) => - ignoreParams ? pathname === path : pathname.includes(path); +const isPathActive = (path: string, exact: boolean = false) => { + if (exact) return pathname === path; + return pathname === path || pathname.startsWith(path + '/'); +};Then update the navigation configuration and all call sites accordingly.
packages/ui/src/components/Select.tsx (2)
205-208: Guard icon cloning and preserve caller classes.
iconis typed asReact.ReactNode, so any consumer passing a string/number/fragment will trigger a runtime error when callingReact.cloneElement. Additionally, the code overwrites any existingclassNameset by the caller. Please gate the clone behindReact.isValidElementand merge their classes.Apply this diff:
- {icon && - React.cloneElement(icon as React.ReactElement<{ className: string }>, { - className: cx(iconSizeVariant[size], "text-gray-9"), - })} + {React.isValidElement(icon) + ? React.cloneElement(icon, { + className: cx(iconSizeVariant[size], "text-gray-9", icon.props.className), + }) + : null}
290-294: Apply the same safe icon handling here.The same two issues occur for item icons: cloning a generic
React.ReactNodewill crash for valid inputs, and we drop any caller-suppliedclassName. Mirror the guarded merge pattern.Apply this diff:
- {icon && - React.cloneElement( - icon as React.ReactElement<{ className: string }>, - { className: cx("size-3", "text-gray-9") }, - )} + {React.isValidElement(icon) + ? React.cloneElement(icon, { + className: cx("size-3", "text-gray-9", icon.props.className), + }) + : null}packages/web-backend/src/Tinybird/index.ts (1)
34-49: Don’t crash when Tinybird env vars are absent.Right now we
dieifTINYBIRD_HOSTis missing, whileTINYBIRD_TOKENjust disables the client. BecauseTinybird.Defaultis now wired into the globalDependencies, any environment that hasn’t setTINYBIRD_HOST(fresh dev shells, CI, QA) will fail to boot the entire app. Please align the validation so the service simply disables itself when either value is absent (or require both together) instead of crashing the runtime. Something along these lines keeps the app running while still logging why analytics is disabled:- if (!host) { - yield* Effect.die(new Error("TINYBIRD_HOST must be set")); - } - - const enabled = Boolean(token); - - if (!enabled) { - yield* Effect.logWarning( - "Tinybird is disabled: TINYBIRD_TOKEN is not set", - ); - } + const enabled = Boolean(host && token); + + if (!enabled) { + const missing = [ + !host ? "TINYBIRD_HOST" : null, + !token ? "TINYBIRD_TOKEN" : null, + ].filter(Boolean); + yield* Effect.logWarning( + `Tinybird is disabled: ${missing.join(", ")} not set`, + ); + }This mirrors the earlier feedback and prevents a full outage when analytics isn’t configured.
scripts/analytics/populate-test-data.js (1)
36-42: Math.random() is appropriate for test data generation.The static analysis tool flags this as "insecure randomness," but
Math.random()is perfectly acceptable for generating test analytics data. Cryptographically secure randomness is not required in this context.apps/web/app/(org)/dashboard/analytics/components/ChartArea.tsx (1)
1-256: Run formatter to fix formatting issues.The pipeline indicates a formatting error on line 54. Please run the formatter as per the coding guidelines.
#!/bin/bash # Run biome formatter on the file pnpm formatapps/web/lib/Queries/Analytics.ts (1)
36-38: Fix the request construction.Line 37 incorrectly constructs the request as
new AnalyticsRequest.AnalyticsRequest({ videoId }). SinceAnalyticsRequestis already the exported request class, this should benew AnalyticsRequest({ videoId }).Apply this diff to fix the construction:
uniqueVideoIds.map((videoId) => Effect.request( - new AnalyticsRequest.AnalyticsRequest({ videoId }), + new AnalyticsRequest({ videoId }), dataloader, ).pipe(apps/web/app/(org)/dashboard/analytics/data.ts (2)
95-118: Remove verbose Tinybird logging before release.The
withTinybirdFallbackhelper logs every raw Tinybird response, data array, and filtered rows (lines 104-115). In production, this will:
- Dump sensitive analytics data (countries, cities, devices, user sessions) to stdout
- Overwhelm logs with large JSON payloads
- Degrade performance
Apply this diff to remove or gate the verbose logging:
effect.pipe( Effect.catchAll((e) => { console.error("tinybird query error", e); return Effect.succeed<{ data: Row[] }>({ data: [] as Row[] }); }), Effect.map((res) => { - console.log("tinybird raw response", JSON.stringify(res, null, 2)); const response = res as { data: unknown[] }; const data = response.data ?? []; - console.log("tinybird data array", JSON.stringify(data, null, 2)); const filtered = data.filter((item): item is Row => { - const isObject = typeof item === "object" && item !== null; - if (!isObject) { - console.log("filtered out non-object item", typeof item, item); - } - return isObject; + return typeof item === "object" && item !== null; }) as Row[]; - console.log("tinybird filtered rows", JSON.stringify(filtered, null, 2)); return filtered; }), );
199-301: Remove debug logging from the main data orchestration function.Lines 202-289 contain multiple
console.logstatements that dump the entire analytics payload (orgId, range, timestamps, view series, countries, cities, browsers, devices, OS, and top caps). This verbose logging should be removed before merging.Apply this diff to remove the debug logs:
const tinybirdData = await runPromise( Effect.gen(function* () { const tinybird = yield* Tinybird; - console.log("getOrgAnalyticsData - orgId:", orgId, "range:", range); - console.log( - "getOrgAnalyticsData - from:", - from.toISOString(), - "to:", - to.toISOString(), - ); const viewSeries = yield* queryViewSeries( tinybird, typedOrgId, from, to, rangeConfig.bucket, videoIds, ); - console.log( - "getOrgAnalyticsData - viewSeries:", - JSON.stringify(viewSeries, null, 2), - ); const countries = yield* queryCountries( tinybird, typedOrgId, from, to, videoIds, ); - console.log( - "getOrgAnalyticsData - countries:", - JSON.stringify(countries, null, 2), - ); const cities = yield* queryCities( tinybird, typedOrgId, from, to, videoIds, ); - console.log( - "getOrgAnalyticsData - cities:", - JSON.stringify(cities, null, 2), - ); const browsers = yield* queryBrowsers( tinybird, typedOrgId, from, to, videoIds, ); - console.log( - "getOrgAnalyticsData - browsers:", - JSON.stringify(browsers, null, 2), - ); const devices = yield* queryDevices( tinybird, typedOrgId, from, to, videoIds, ); - console.log( - "getOrgAnalyticsData - devices:", - JSON.stringify(devices, null, 2), - ); const operatingSystems = yield* queryOperatingSystems( tinybird, typedOrgId, from, to, videoIds, ); - console.log( - "getOrgAnalyticsData - operatingSystems:", - JSON.stringify(operatingSystems, null, 2), - ); const topCapsRaw = capId ? [] : yield* queryTopCaps(tinybird, typedOrgId, from, to, videoIds); - console.log( - "getOrgAnalyticsData - topCapsRaw:", - JSON.stringify(topCapsRaw, null, 2), - ); return {apps/web/app/(org)/dashboard/analytics/components/OtherStats.tsx (1)
105-120: Map Android (and other OS values) correctly.The previous review comment about Android mapping is still valid. This function defaults any unrecognized OS to "windows", causing Android, Chrome OS, and other platforms to display incorrectly.
Additionally, note that
OperatingSystemTypein TableCard.tsx only includes"windows" | "ios" | "linux" | "fedora" | "ubuntu". You'll need to:
- Add "android" to the
OperatingSystemTypeunion in TableCard.tsx- Ensure the corresponding icon asset exists at
/logos/os/android.svg- Update this mapping function per the previous review
const osNameToKey = (name: string): OSRowData["os"] => { const normalized = name.toLowerCase().trim(); if (normalized.includes("mac") || normalized === "ios") { return "ios"; } switch (normalized) { + case "android": + return "android"; case "linux": return "linux"; case "ubuntu": return "ubuntu"; case "fedora": return "fedora"; default: return "windows"; } };apps/web/app/(org)/dashboard/analytics/components/Header.tsx (1)
303-345: Restore organization switching capability.The previous review comment is still valid. The dropdown only renders the active organization and its spaces, preventing users from switching to other organizations they belong to. This is a critical UX regression for multi-org users.
As suggested in the previous comment, iterate the
organizationsarray and render aSelectPrimitive.Itemfor each organization. Consider adding an "Organizations" section header similar to the "Spaces" section:<SelectPrimitive.Viewport className="p-1"> + <div className="px-3 py-2 text-xs font-semibold text-gray-9 uppercase tracking-wider"> + Organizations + </div> + {organizations?.map((org) => ( + <SelectPrimitive.Item + key={org.organization.id} + value={org.organization.id} + className="relative flex w-full cursor-pointer items-center gap-2 rounded-lg px-3 py-2 text-sm outline-none transition-colors text-gray-12 data-[state=checked]:bg-gray-3 data-[highlighted]:bg-gray-3 data-[highlighted]:text-gray-12" + > + <SignedImageUrl + image={org.organization.iconUrl} + name={org.organization.name} + className="size-5 flex-shrink-0" + /> + <SelectPrimitive.ItemText> + {org.organization.name} + </SelectPrimitive.ItemText> + </SelectPrimitive.Item> + ))} + <SelectPrimitive.Separator className="h-px bg-gray-4 my-1" /> <SelectPrimitive.Item key={activeOrganization.organization.id} value={activeOrganization.organization.id} - className="relative flex w-full cursor-pointer items-center gap-2 rounded-lg px-3 py-2 text-sm outline-none transition-colors font-semibold text-gray-12 data-[state=checked]:bg-gray-3 data-[highlighted]:bg-gray-3 data-[highlighted]:text-gray-12" + className="relative flex w-full cursor-pointer items-center gap-2 rounded-lg px-3 py-2 text-sm outline-none transition-colors text-gray-12 data-[state=checked]:bg-gray-3 data-[highlighted]:bg-gray-3 data-[highlighted]:text-gray-12" > <SignedImageUrl image={ user?.imageUrl || activeOrganization.organization.iconUrl } name={user?.name || activeOrganization.organization.name} className="size-5 flex-shrink-0" /> <SelectPrimitive.ItemText> {user?.name ? `${user.name}'s Caps` : "My Caps"} </SelectPrimitive.ItemText> </SelectPrimitive.Item>
🧹 Nitpick comments (7)
apps/web/app/api/analytics/route.ts (1)
30-32: Remove noisy debug logs before shipping.The
console.logstatements will spam output on every request and don’t add lasting value. Please drop them before merge to keep server logs clean.scripts/analytics/populate-test-data.js (1)
116-134: Document the SSL configuration for clarity.The
rejectUnauthorized: falsesetting on line 129 is suitable for test/development environments but may cause confusion. Consider adding a comment explaining that this allows self-signed certificates for local/test database connections.Apply this diff to add clarifying documentation:
const config = { host: parsed.hostname, port: parsed.port ? parseInt(parsed.port, 10) : 3306, user: parsed.username, password: parsed.password, database: parsed.pathname.slice(1), ssl: { + // Allow self-signed certificates for test/dev environments rejectUnauthorized: false, }, };packages/web-backend/src/Videos/index.ts (1)
77-90: Guard against potential undefined access.Line 86 accesses
videoIds[index]which could theoretically be undefined if thevideoIdsarray andvideoExitsarray lengths diverge. While this shouldn't happen in practice, adding a safety check would prevent runtime errors.Apply this diff to add a guard:
for (let index = 0; index < videoExits.length; index++) { const exit = videoExits[index]; + const videoId = videoIds[index]; - if (!exit) continue; + if (!exit || !videoId) continue; if (Exit.isSuccess(exit)) { const maybeVideo = exit.value; if (Option.isSome(maybeVideo)) { const [video] = maybeVideo.value; successfulVideos.push({ index, - videoId: videoIds[index] ?? "", + videoId, video, }); } } }apps/web/app/(org)/dashboard/analytics/components/TableCard.tsx (2)
162-177: Remove commented-out code.This commented filter dropdown UI adds clutter without providing value. If the feature is planned for later, track it in an issue instead.
Apply this diff to remove the dead code:
- {/* <div className="flex flex-1 gap-2 justify-between items-center h-[48px]"> - <p className="text-lg font-medium text-gray-12">{title}</p> - <Select - variant="light" - placeholder="Views" - icon={<FontAwesomeIcon icon={faFilter} className="text-gray-11" />} - options={[ - { value: "views", label: "Views" }, - { value: "comments", label: "Comments" }, - { value: "reactions", label: "Reactions" }, - ]} - value="views" - onValueChange={() => {}} - size="sm" - /> - </div> */}
24-29: Add defensive validation for country codes to fail-fast on unexpected data.The data flows from Tinybird's analytics backend (trusted IP geolocation source), where it undergoes no transformation that could corrupt valid ISO codes. However,
getUnicodeFlagIconsilently produces emoji for any two-letter input without validation, meaning invalid codes won't throw but will render unexpected flags. Adding a length check is a low-cost defensive measure:const countryCodeToIcon = (countryCode: string | undefined | null) => { if (!countryCode || countryCode.trim() === "") { return null; } + if (countryCode.length !== 2) { + return null; + } return getUnicodeFlagIcon(countryCode.toUpperCase()); };apps/web/app/(org)/dashboard/analytics/components/StatsChart.tsx (2)
37-42: Align component name with filename.The file is named
StatsChart.tsxbut exportsStatsBox. This inconsistency can cause confusion when importing and navigating the codebase.Either rename the function to
StatsChartor rename the file toStatsBox.tsx:-export default function StatsBox({ +export default function StatsChart({ counts, data, isLoading, capId, }: StatsChartProps) {
197-212: Consider CSS variables instead of inline style manipulation.Directly manipulating
style.backgroundColorin event handlers makes the styling logic harder to maintain and test. CSS variables or state-driven class names would be cleaner.Example refactor using CSS variables:
style={{ '--metric-bg': isSelected ? colors.bgSelected : colors.bg, '--metric-bg-hover': colors.bgHover, '--metric-border': isSelected ? colors.borderSelected : colors.border, } as React.CSSProperties} className={classNames( "transition-all [background-color:var(--metric-bg)] [border-color:var(--metric-border)]", "hover:[background-color:var(--metric-bg-hover)]", // ... )}Then remove the onMouseEnter/onMouseLeave style mutations.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
apps/web/actions/videos/get-analytics.ts(1 hunks)apps/web/app/(org)/dashboard/_components/Navbar/Items.tsx(8 hunks)apps/web/app/(org)/dashboard/analytics/components/AnalyticsDashboard.tsx(1 hunks)apps/web/app/(org)/dashboard/analytics/components/ChartArea.tsx(1 hunks)apps/web/app/(org)/dashboard/analytics/components/Header.tsx(1 hunks)apps/web/app/(org)/dashboard/analytics/components/OtherStats.tsx(1 hunks)apps/web/app/(org)/dashboard/analytics/components/StatsChart.tsx(1 hunks)apps/web/app/(org)/dashboard/analytics/components/TableCard.tsx(1 hunks)apps/web/app/(org)/dashboard/analytics/data.ts(1 hunks)apps/web/app/(org)/dashboard/caps/Caps.tsx(5 hunks)apps/web/app/(org)/dashboard/caps/components/CapCard/CapCardAnalytics.tsx(3 hunks)apps/web/app/(org)/dashboard/caps/page.tsx(1 hunks)apps/web/app/(org)/dashboard/folder/[id]/page.tsx(1 hunks)apps/web/app/(org)/dashboard/spaces/[spaceId]/folder/[folderId]/page.tsx(1 hunks)apps/web/app/(org)/dashboard/spaces/[spaceId]/page.tsx(2 hunks)apps/web/app/api/analytics/route.ts(1 hunks)apps/web/app/api/analytics/track/route.ts(1 hunks)apps/web/app/api/dashboard/analytics/route.ts(1 hunks)apps/web/app/s/[videoId]/Share.tsx(5 hunks)apps/web/lib/Queries/Analytics.ts(1 hunks)apps/web/lib/server.ts(2 hunks)packages/ui/src/components/Select.tsx(9 hunks)packages/web-backend/src/Tinybird/index.ts(1 hunks)packages/web-backend/src/Videos/VideosRpcs.ts(1 hunks)packages/web-backend/src/Videos/index.ts(4 hunks)packages/web-backend/src/index.ts(1 hunks)scripts/analytics/check-analytics.js(1 hunks)scripts/analytics/delete-all-data.js(1 hunks)scripts/analytics/populate-test-data.js(1 hunks)scripts/analytics/setup-analytics.js(1 hunks)scripts/analytics/shared.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- apps/web/app/(org)/dashboard/spaces/[spaceId]/page.tsx
- apps/web/app/(org)/dashboard/caps/components/CapCard/CapCardAnalytics.tsx
- scripts/analytics/check-analytics.js
- apps/web/app/(org)/dashboard/spaces/[spaceId]/folder/[folderId]/page.tsx
- apps/web/app/(org)/dashboard/caps/page.tsx
- apps/web/app/(org)/dashboard/analytics/components/AnalyticsDashboard.tsx
- scripts/analytics/delete-all-data.js
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
apps/web/lib/server.tsapps/web/app/(org)/dashboard/_components/Navbar/Items.tsxapps/web/actions/videos/get-analytics.tsapps/web/app/(org)/dashboard/folder/[id]/page.tsxapps/web/lib/Queries/Analytics.tspackages/web-backend/src/Videos/VideosRpcs.tsapps/web/app/(org)/dashboard/analytics/components/TableCard.tsxapps/web/app/api/analytics/track/route.tsapps/web/app/(org)/dashboard/analytics/components/ChartArea.tsxapps/web/app/(org)/dashboard/analytics/components/StatsChart.tsxpackages/web-backend/src/Tinybird/index.tsapps/web/app/(org)/dashboard/caps/Caps.tsxapps/web/app/(org)/dashboard/analytics/data.tsapps/web/app/(org)/dashboard/analytics/components/OtherStats.tsxapps/web/app/(org)/dashboard/analytics/components/Header.tsxpackages/ui/src/components/Select.tsxpackages/web-backend/src/index.tsapps/web/app/s/[videoId]/Share.tsxapps/web/app/api/dashboard/analytics/route.tspackages/web-backend/src/Videos/index.tsapps/web/app/api/analytics/route.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
apps/web/lib/server.tsapps/web/app/(org)/dashboard/_components/Navbar/Items.tsxapps/web/actions/videos/get-analytics.tsapps/web/app/(org)/dashboard/folder/[id]/page.tsxapps/web/lib/Queries/Analytics.tspackages/web-backend/src/Videos/VideosRpcs.tsapps/web/app/(org)/dashboard/analytics/components/TableCard.tsxapps/web/app/api/analytics/track/route.tsscripts/analytics/setup-analytics.jsapps/web/app/(org)/dashboard/analytics/components/ChartArea.tsxapps/web/app/(org)/dashboard/analytics/components/StatsChart.tsxpackages/web-backend/src/Tinybird/index.tsscripts/analytics/shared.jsapps/web/app/(org)/dashboard/caps/Caps.tsxapps/web/app/(org)/dashboard/analytics/data.tsscripts/analytics/populate-test-data.jsapps/web/app/(org)/dashboard/analytics/components/OtherStats.tsxapps/web/app/(org)/dashboard/analytics/components/Header.tsxpackages/ui/src/components/Select.tsxpackages/web-backend/src/index.tsapps/web/app/s/[videoId]/Share.tsxapps/web/app/api/dashboard/analytics/route.tspackages/web-backend/src/Videos/index.tsapps/web/app/api/analytics/route.ts
apps/web/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
On the client, always use
useEffectQueryoruseEffectMutationfrom@/lib/EffectRuntime; never callEffectRuntime.run*directly in components.
Files:
apps/web/lib/server.tsapps/web/app/(org)/dashboard/_components/Navbar/Items.tsxapps/web/actions/videos/get-analytics.tsapps/web/app/(org)/dashboard/folder/[id]/page.tsxapps/web/lib/Queries/Analytics.tsapps/web/app/(org)/dashboard/analytics/components/TableCard.tsxapps/web/app/api/analytics/track/route.tsapps/web/app/(org)/dashboard/analytics/components/ChartArea.tsxapps/web/app/(org)/dashboard/analytics/components/StatsChart.tsxapps/web/app/(org)/dashboard/caps/Caps.tsxapps/web/app/(org)/dashboard/analytics/data.tsapps/web/app/(org)/dashboard/analytics/components/OtherStats.tsxapps/web/app/(org)/dashboard/analytics/components/Header.tsxapps/web/app/s/[videoId]/Share.tsxapps/web/app/api/dashboard/analytics/route.tsapps/web/app/api/analytics/route.ts
packages/ui/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Component files in
packages/uishould use PascalCase naming if they define React/Solid components.
Files:
packages/ui/src/components/Select.tsx
🧠 Learnings (5)
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to apps/web/app/api/* : API routes in Next.js (`apps/web/app/api/*`) must use `effect/platform`'s `HttpApi` builder and follow the existing class/group/endpoint pattern; avoid ad-hoc handlers.
Applied to files:
apps/web/app/api/analytics/track/route.ts
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to apps/web/app/api/* : Convert effectful APIs to Next.js handlers using `apiToHandler(ApiLive)` and export only the returned `handler`; avoid calling `runPromise` directly in route files.
Applied to files:
apps/web/app/api/analytics/track/route.ts
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to apps/web/app/api/* : On the server, always run effects through `EffectRuntime.runPromise` after `provideOptionalAuth` to ensure cookies and per-request context are attached.
Applied to files:
apps/web/app/api/analytics/track/route.ts
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to **/*.{ts,tsx} : Use Biome for formatting and linting TypeScript/JavaScript files by running `pnpm format`.
Applied to files:
apps/web/app/(org)/dashboard/analytics/components/ChartArea.tsxpackages/web-backend/src/index.ts
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to apps/web/**/*.{ts,tsx,js,jsx} : On the client, always use `useEffectQuery` or `useEffectMutation` from `@/lib/EffectRuntime`; never call `EffectRuntime.run*` directly in components.
Applied to files:
apps/web/app/s/[videoId]/Share.tsx
🧬 Code graph analysis (19)
apps/web/lib/server.ts (2)
packages/web-backend/src/Tinybird/index.ts (1)
Tinybird(28-276)packages/web-backend/src/index.ts (1)
Tinybird(14-14)
apps/web/app/(org)/dashboard/_components/Navbar/Items.tsx (1)
apps/web/app/(org)/dashboard/_components/AnimatedIcons/index.ts (1)
ChartLineIcon(27-27)
apps/web/actions/videos/get-analytics.ts (5)
packages/database/index.ts (1)
db(18-25)packages/database/schema.ts (1)
videos(291-363)packages/web-domain/src/Video.ts (1)
Video(16-59)apps/web/lib/server.ts (1)
runPromise(141-153)packages/web-backend/src/Tinybird/index.ts (1)
Tinybird(28-276)
apps/web/lib/Queries/Analytics.ts (2)
apps/web/lib/Requests/AnalyticsRequest.ts (1)
AnalyticsRequest(8-12)apps/web/lib/EffectRuntime.ts (1)
useEffectQuery(22-22)
apps/web/app/(org)/dashboard/analytics/components/TableCard.tsx (2)
packages/ui/src/components/Table.tsx (6)
Table(117-117)TableHeader(123-123)TableRow(124-124)TableHead(122-122)TableBody(118-118)TableCell(120-120)packages/ui/src/components/icons/LogoBadge.tsx (1)
LogoBadge(1-28)
apps/web/app/api/analytics/track/route.ts (3)
apps/web/lib/server.ts (1)
runPromise(141-153)packages/web-backend/src/Tinybird/index.ts (1)
Tinybird(28-276)packages/web-backend/src/Auth.ts (1)
provideOptionalAuth(102-114)
scripts/analytics/setup-analytics.js (2)
scripts/analytics/migrate-dub-to-tinybird.js (1)
__dirname(8-8)scripts/analytics/shared.js (2)
PROJECT_ROOT(9-9)auth(140-140)
apps/web/app/(org)/dashboard/analytics/components/ChartArea.tsx (1)
apps/web/components/ui/chart.tsx (4)
ChartConfig(10-18)ChartContainer(354-354)ChartTooltip(355-355)ChartTooltipContent(356-356)
apps/web/app/(org)/dashboard/analytics/components/StatsChart.tsx (2)
apps/web/app/(org)/dashboard/_components/AnimatedIcons/Cap.tsx (1)
CapIconHandle(8-11)apps/web/app/(org)/dashboard/_components/AnimatedIcons/index.ts (4)
ClapIcon(28-28)ChatIcon(29-29)ReactionIcon(30-30)CapIcon(18-18)
packages/web-backend/src/Tinybird/index.ts (4)
packages/env/server.ts (1)
serverEnv(136-140)scripts/analytics/shared.js (2)
request(142-177)raw(92-92)scripts/analytics/populate-test-data.js (8)
url(94-94)response(95-103)i(170-170)i(211-211)i(225-225)i(229-229)text(104-104)search(93-93)scripts/analytics/check-analytics.js (4)
errorMessage(146-147)current(27-27)depth(26-26)parts(25-25)
scripts/analytics/shared.js (3)
scripts/analytics/populate-test-data.js (6)
os(62-62)auth(218-218)url(94-94)response(95-103)text(104-104)payload(108-108)scripts/analytics/migrate-dub-to-tinybird.js (11)
os(5480-5480)data(4563-4563)data(4641-4641)data(4734-4734)url(4438-4438)url(4827-4827)response(4443-4445)response(4828-4836)text(4449-4449)text(4837-4837)payload(4462-4462)scripts/analytics/delete-all-data.js (2)
auth(78-78)payload(8-8)
apps/web/app/(org)/dashboard/caps/Caps.tsx (5)
apps/web/app/(org)/dashboard/spaces/[spaceId]/components/AddVideosDialogBase.tsx (1)
VideoData(37-50)apps/web/app/(org)/dashboard/caps/components/Folder.tsx (1)
FolderDataType(18-25)packages/web-domain/src/Video.ts (3)
Video(16-59)VideoId(12-12)VideoId(13-13)apps/web/lib/EffectRuntime.ts (2)
useRpcClient(25-25)useEffectMutation(23-23)apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx (1)
CapCard(101-648)
apps/web/app/(org)/dashboard/analytics/data.ts (5)
packages/database/schema.ts (3)
videos(291-363)spaceVideos(665-688)comments(394-420)apps/web/app/(org)/dashboard/analytics/types.ts (3)
AnalyticsRange(1-1)OrgAnalyticsResponse(10-33)BreakdownRow(3-8)packages/database/index.ts (1)
db(18-25)apps/web/lib/server.ts (1)
runPromise(141-153)packages/web-backend/src/Tinybird/index.ts (1)
Tinybird(28-276)
scripts/analytics/populate-test-data.js (2)
scripts/analytics/migrate-dub-to-tinybird.js (6)
TB_DATASOURCE(16-16)INGEST_CHUNK_SIZE(18-18)i(4117-4117)i(4181-4181)i(4805-4805)i(5610-5610)scripts/analytics/shared.js (1)
auth(140-140)
apps/web/app/(org)/dashboard/analytics/components/OtherStats.tsx (3)
apps/web/app/(org)/dashboard/analytics/types.ts (1)
BreakdownRow(3-8)apps/web/app/(org)/dashboard/analytics/components/TableCard.tsx (6)
DeviceRowData(100-107)CountryRowData(64-71)CityRowData(73-80)BrowserRowData(82-89)OSRowData(91-98)CapRowData(109-116)apps/web/app/(org)/dashboard/analytics/components/OtherStatBox.tsx (1)
OtherStatBox(12-32)
apps/web/app/(org)/dashboard/analytics/components/Header.tsx (4)
apps/web/app/(org)/dashboard/analytics/types.ts (1)
AnalyticsRange(1-1)packages/web-domain/src/Organisation.ts (1)
Organisation(21-24)apps/web/app/Layout/AuthContext.tsx (1)
useCurrentUser(40-42)apps/web/components/SignedImageUrl.tsx (1)
SignedImageUrl(11-25)
apps/web/app/api/dashboard/analytics/route.ts (2)
apps/web/app/(org)/dashboard/analytics/types.ts (1)
AnalyticsRange(1-1)apps/web/app/(org)/dashboard/analytics/data.ts (1)
getOrgAnalyticsData(128-379)
packages/web-backend/src/Videos/index.ts (4)
packages/web-domain/src/Video.ts (3)
Video(16-59)VideoId(12-12)VideoId(13-13)packages/web-backend/src/Tinybird/index.ts (1)
Tinybird(28-276)packages/web-backend/src/Videos/VideosPolicy.ts (1)
VideosPolicy(9-96)packages/web-backend/src/Videos/VideosRepo.ts (1)
VideosRepo(15-106)
apps/web/app/api/analytics/route.ts (2)
apps/web/app/api/dashboard/analytics/route.ts (1)
GET(11-43)apps/web/actions/videos/get-analytics.ts (1)
getVideoAnalytics(34-96)
🪛 GitHub Actions: CI
packages/web-backend/src/Videos/VideosRpcs.ts
[error] 8-8: TypeScript error TS2345: Argument of type 'Effect<{ VideoDelete: (videoId: string & Brand<"VideoId">) => Effect<undefined, InternalError | PolicyDeniedError | NotFoundError, CurrentUser>; ... 6 more ...; VideosGetAnalytics: (videoIds: readonly (string & Brand<...>)[]) => Effect<...>; }, never, Videos>' is not assignable to parameter of type 'HandlersFrom<Rpc<"VideoDelete", brand<typeof String$, "VideoId">, typeof Void, Union<[typeof NotFoundError, typeof InternalError, typeof PolicyDeniedError]>, typeof RpcAuthMiddleware> | ... 6 more ... | Rpc<...>> | Effect<...>'.
🪛 GitHub Check: CodeQL
scripts/analytics/populate-test-data.js
[failure] 63-69: Insecure randomness
This uses a cryptographically insecure random number generated at Math.random() in a security context.
🔇 Additional comments (18)
apps/web/app/(org)/dashboard/_components/Navbar/Items.tsx (2)
39-39: LGTM!The
ChartLineIconimport is correctly added to support the new Analytics navigation item.
62-68: LGTM!The Analytics navigation item is correctly structured and positioned. The use of
ignoreParams: trueensures this item is only active on the exact/dashboard/analyticsroute, not on potential sub-routes.packages/ui/src/components/Select.tsx (7)
1-12: LGTM!The imports, type definitions, and context setup are well-structured and appropriate for a variant-driven component system.
13-102: LGTM!The CVA variant configurations are well-structured with consistent variant names across trigger, content, and item components. The class compositions handle states and transitions appropriately.
104-159: LGTM!The component composition and prop threading are correct. The variant system integration is clean and the options mapping handles both icons and images appropriately.
161-177: LGTM!The wrapper components appropriately delegate to Radix primitives with proper data-slot attributes.
217-252: LGTM!The SelectContent properly provides variant context to descendant items, enabling variant-aware styling throughout the component tree.
254-265: LGTM!The label component correctly applies typography and spacing classes.
300-361: LGTM!The separator and scroll button components are implemented correctly with appropriate styling. The exports are comprehensive and well-organized.
apps/web/app/(org)/dashboard/folder/[id]/page.tsx (1)
87-89: LGTM!The migration from DUB-based analytics to Tinybird is correctly implemented. The prop name change to
analyticsEnabledand the check for bothTINYBIRD_TOKENandTINYBIRD_HOSTensure analytics are only enabled when the backend service is properly configured.apps/web/app/api/dashboard/analytics/route.ts (1)
11-43: LGTM!The route handler correctly implements:
- Authentication (401 for unauthenticated users)
- Authorization (403 if requesting analytics for a different org)
- Input validation (400 for missing org, validated range with fallback)
- Error handling (500 with logging on data fetch failure)
The logic is clean and secure.
apps/web/app/(org)/dashboard/analytics/components/ChartArea.tsx (2)
52-64: LGTM!The bucket duration detection logic is well-implemented. It correctly distinguishes between hourly and daily buckets by comparing the time difference between consecutive data points, enabling appropriate label formatting.
66-107: LGTM!The Y-axis scaling algorithm properly computes "nice" tick values by:
- Finding the max value across selected metrics
- Rounding up by 10% for padding
- Normalizing to clean magnitudes (1, 2, 5, or 10)
- Generating evenly-spaced ticks
This ensures readable, user-friendly axis labels.
packages/web-backend/src/Videos/index.ts (2)
51-91: LGTM! The bulk analytics architecture is well-designed.The implementation correctly:
- Fetches videos with proper access control via
getByIdForViewing- Groups videos by organization for efficient Tinybird queries
- Uses
Exitto preserve per-video success/failure states- Handles concurrent video fetches with bounded concurrency
113-127: LGTM! Error handling is robust.The
runTinybirdQueryhelper gracefully handles Tinybird failures by:
- Catching all errors and logging them
- Returning an empty data array as a fallback
- Ensuring the analytics flow continues even if Tinybird is unavailable
This prevents cascading failures.
apps/web/app/(org)/dashboard/analytics/data.ts (1)
128-379: LGTM! The analytics orchestration is well-architected.The
getOrgAnalyticsDatafunction effectively:
- Computes date ranges and buckets based on the selected analytics range
- Handles optional space/cap filtering with early returns for empty results
- Fetches data from both relational DB (videos, comments, reactions) and Tinybird (views, breakdowns) in parallel
- Normalizes and aggregates breakdown data with proper percentage calculations
- Resolves video names for display
- Returns a complete, well-typed analytics response
The overall structure is solid despite the logging issues noted separately.
apps/web/app/(org)/dashboard/caps/Caps.tsx (1)
79-170: Well-structured analytics integration and deletion flow.The analytics integration with
analyticsEnabledand the new Effect-based deletion mutations are well-implemented:
- Proper conditional query execution based on
analyticsEnabled- Batch deletion with controlled concurrency (10)
- Comprehensive error handling with user feedback via toasts
- Router refresh ensures UI consistency after mutations
apps/web/app/(org)/dashboard/analytics/components/Header.tsx (1)
46-88: Date range mapping logic is well-designed.The bidirectional mapping between UI values (today, yesterday, wtd, mtd) and backend
AnalyticsRangevalues (24h, 7d, 30d) properly preserves user intent vialastUISelection. This ensures the UI reflects what the user selected rather than ambiguously mapping backend values.
| const toCountryRow = (row: BreakdownRow): CountryRowData => ({ | ||
| countryCode: row.name, | ||
| name: row.name, | ||
| views: row.views, | ||
| comments: null, | ||
| reactions: null, | ||
| percentage: row.percentage, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
Verify country code format from analytics data.
toCountryRow assigns row.name to both countryCode and name fields. The countryCode field should contain valid ISO 3166-1 alpha-2 codes (e.g., "US", "GB") for flag rendering to work correctly. If row.name contains full country names instead, flag icons will fail to display.
Run this script to check what format the analytics data provides:
🏁 Script executed:
#!/bin/bash
# Check how country data is structured in the analytics pipeline
rg -n "countries.*=|country.*:" --type=ts -C5 -g 'apps/web/app/(org)/dashboard/analytics/**'
# Also check the Tinybird query definitions
fd -e pipe -e sql . | xargs rg "country" -C3Length of output: 15028
---
🏁 Script executed:
```shell
#!/bin/bash
# Find queryCountries implementation
rg -n "queryCountries" --type=ts -A15 apps/web/app/(org)/dashboard/analytics/
# Find BreakdownRow type definition
rg -n "type BreakdownRow|interface BreakdownRow" --type=ts -A8 apps/web/app/(org)/dashboard/analytics/
Length of output: 164
---
🏁 Script executed:
```shell
#!/bin/bash
# Find queryCountries implementation - escape or avoid parentheses
fd -type f -name "*.ts" -path "*dashboard/analytics*" -exec rg -l "queryCountries" {} \;
# Find BreakdownRow type
fd -type f -name "*.ts" -path "*dashboard/analytics*" -exec rg -n "type BreakdownRow|interface BreakdownRow" {} +
Length of output: 341
---
🏁 Script executed:
```shell
#!/bin/bash
# Find queryCountries function in analytics files
rg -n "queryCountries\s*=" apps/web/app/ -A 20
# Find BreakdownRow type definition
rg -n "type BreakdownRow|interface BreakdownRow" apps/web/app/ -A 8
# Also search for where country data is queried
rg -n "queryCountries\|getCountries\|countries.*=" apps/web/app/\(org\)/dashboard/analytics/ -A 10
Length of output: 2204
---
🏁 Script executed:
```shell
#!/bin/bash
# Find where country data is ingested/sourced in Tinybird
rg -n "country" scripts/analytics/tinybird/ -B2 -A2
# Check if there's any country name to code mapping
rg -n "country.*map\|countryCode\|country.*iso" apps/web/app/ --type=ts -i
# Check the full queryCountries implementation
cat apps/web/app/\(org\)/dashboard/analytics/data.ts | sed -n '642,700p'
Length of output: 4883
---
🏁 Script executed:
```shell
#!/bin/bash
# Find where analytics events are tracked and country is set
rg -n "country\s*:" apps/web/ --type=ts -B3 -A1 | grep -A3 -B3 "track\|analytics\|event"
# Look for event payload construction
rg -n "{\s*action\|EVENT\|track" apps/web/ --type=ts -A5 | grep -A5 "country"
# Search for tracking implementation
fd -name "*track*" -type f apps/web/ --type=ts
Length of output: 688
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for where country field is populated in client tracking
rg "country" apps/web/lib/ -B2 -A2 --type=ts
# Look for analytics or tracking client code
fd "analytics\|track" apps/web/lib/ --type=ts -exec rg -l "country" {} \;
# Search for GeoIP or location detection libraries
rg "geoip\|geolocation\|country" apps/web/ --type=ts -l | head -20
Length of output: 169
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for where country is set in event payload
rg -n "country" apps/web/lib --type=typescript -B2 -A2
# Look for event sending/tracking code
rg -n "country" scripts/ -B2 -A2
# Find country field assignments broadly
rg -n "country\s*=" apps/ -B2 -A2 --type=typescript | head -50
Length of output: 29659
Fix toCountryRow to extract or map country codes instead of using country names.
The analytics pipeline sends full country names (e.g., "United States", "United Kingdom") to Tinybird's country field. The toCountryRow function currently assigns row.name directly to countryCode, but the flag renderer expects ISO 3166-1 alpha-2 codes (e.g., "US", "GB"). This will cause flag icons to fail rendering.
The toCityRow function uses row.subtitle for countryCode, which suggests a different data structure. You need to determine whether:
- Tinybird's country breakdown query should extract ISO codes instead of names, or
toCountryRowshould map country names to ISO codes before assigning tocountryCode.
| const browserNameToSlug = (name: string): BrowserRowData["browser"] => { | ||
| switch (name.toLowerCase()) { | ||
| case "chrome": | ||
| return "google-chrome"; | ||
| case "firefox": | ||
| return "firefox"; | ||
| case "safari": | ||
| return "safari"; | ||
| case "edge": | ||
| case "internet explorer": | ||
| return "explorer"; | ||
| case "opera": | ||
| return "opera"; | ||
| case "brave": | ||
| return "brave"; | ||
| default: | ||
| return "google-chrome"; | ||
| } | ||
| }; |
There was a problem hiding this comment.
Extend browser mapping to cover all BrowserType values.
browserNameToSlug only handles a subset of browsers but defaults to "google-chrome" for anything unrecognized. This will cause browsers like Vivaldi, Yandex, DuckDuckGo, Samsung Internet, UC Browser, QQ Browser, Maxthon, Arora, and Lunascape (all defined in BrowserType) to incorrectly display Chrome's icon.
Either extend the switch statement to handle all supported browsers, or verify which browsers Tinybird actually reports and adjust the BrowserType union accordingly:
switch (name.toLowerCase()) {
case "chrome":
return "google-chrome";
case "firefox":
return "firefox";
case "safari":
return "safari";
case "edge":
case "internet explorer":
return "explorer";
case "opera":
return "opera";
case "brave":
return "brave";
+ case "vivaldi":
+ return "vivaldi";
+ case "yandex":
+ return "yandex";
+ // ... add remaining browsers
default:
return "google-chrome";
}Or run this script to see what browser names Tinybird is actually returning:
#!/bin/bash
# Search for browser data processing in analytics code
ast-grep --pattern 'browsers: $$$'
rg -n "browser.*name|browser.*:" --type=ts -C3 -g 'apps/web/app/(org)/dashboard/analytics/**'| const OperatingSystemIcon = ({ operatingSystem }: OperatingSystemIconProps) => { | ||
| if (operatingSystem === "ios") { | ||
| return <FontAwesomeIcon className="text-gray-12" icon={faAppleWhole} />; | ||
| } | ||
| return ( | ||
| <Image | ||
| src={`/logos/os/${operatingSystem}.svg`} | ||
| alt={operatingSystem} | ||
| width={16} | ||
| height={16} | ||
| className="size-4" | ||
| /> | ||
| ); | ||
| }; | ||
|
|
||
| type BrowserIconProps = { | ||
| browser: BrowserType; | ||
| }; | ||
|
|
||
| const BrowserIcon = ({ browser }: BrowserIconProps) => ( | ||
| <Image | ||
| src={`/logos/browsers/${browser}.svg`} | ||
| alt={browser} | ||
| width={16} | ||
| height={16} | ||
| className="size-4" | ||
| /> | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
Verify OS and browser icon assets exist.
The icon components assume SVG files exist at /logos/os/${operatingSystem}.svg and /logos/browsers/${browser}.svg. Missing assets will cause broken images or 404 errors.
Run this script to verify all expected assets are present:
🏁 Script executed:
#!/bin/bash
# Check for OS icon files
echo "=== Checking OS icons ==="
for os in windows linux fedora ubuntu; do
if [ -f "apps/web/public/logos/os/${os}.svg" ]; then
echo "✓ ${os}.svg exists"
else
echo "✗ ${os}.svg MISSING"
fi
done
# Check for browser icon files
echo -e "\n=== Checking browser icons ==="
for browser in google-chrome firefox safari explorer opera brave vivaldi yandex duckduckgo internet-explorer samsung-internet uc-browser qq-browser maxthon arora lunascape; do
if [ -f "apps/web/public/logos/browsers/${browser}.svg" ]; then
echo "✓ ${browser}.svg exists"
else
echo "✗ ${browser}.svg MISSING"
fi
doneLength of output: 509
🏁 Script executed:
#!/bin/bash
# Find BrowserType definition and enum values
echo "=== Finding BrowserType definition ==="
rg -n "type BrowserType|enum.*Browser|BrowserType\s*=" apps/web --type ts --type tsx -B2 -A10
# Check if any of the missing browser types are referenced in code
echo -e "\n=== Checking for references to missing browser types ==="
for browser in internet-explorer samsung-internet uc-browser qq-browser maxthon arora lunascape; do
count=$(rg -c "$browser" apps/web --type ts --type tsx)
if [ "$count" -gt 0 ]; then
echo "✗ '$browser' is referenced $count times"
else
echo "✓ '$browser' not referenced"
fi
doneLength of output: 905
🏁 Script executed:
#!/bin/bash
# Find BrowserType definition without specifying tsx separately
echo "=== Finding BrowserType definition ==="
rg -n "type BrowserType" apps/web -A 15
# Also search for where browser values are defined/used
echo -e "\n=== Looking for browser data or switch cases ==="
rg -n "browser.*==|case.*safari|case.*firefox" apps/web -C2 | head -50
# Check TableCard.tsx for browser type usage
echo -e "\n=== Checking TableCard.tsx context ==="
grep -n "BrowserType\|browser" apps/web/app/\(org\)/dashboard/analytics/components/TableCard.tsx | head -20Length of output: 2455
Add missing browser icon SVG files or update BrowserType to exclude unsupported browsers.
The BrowserType union (lines 42-57) includes 17 browser types, but only 10 corresponding SVG files exist in apps/web/public/logos/browsers/. The 7 missing files are: internet-explorer.svg, samsung-internet.svg, uc-browser.svg, qq-browser.svg, maxthon.svg, arora.svg, and lunascape.svg.
The BrowserIcon component (lines 321-329) has no error handling for missing assets, so any of these browser types in the data will display broken images at runtime.
🤖 Prompt for AI Agents
In apps/web/app/(org)/dashboard/analytics/components/TableCard.tsx around lines
302 to 329: the BrowserIcon component assumes a matching SVG exists for every
BrowserType but seven browser SVGs are missing causing broken images; either add
the missing SVG files to apps/web/public/logos/browsers/ (internet-explorer.svg,
samsung-internet.svg, uc-browser.svg, qq-browser.svg, maxthon.svg, arora.svg,
lunascape.svg) or prune those values from the BrowserType union so only
supported browsers remain, and add a runtime fallback in BrowserIcon that
detects a missing asset and renders a generic/placeholder icon (or FontAwesome
fallback) instead of a broken <Image>; implement one of the two fixes (prefer
adding assets + fallback) and update any consumers or types accordingly.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/web-backend/src/Videos/VideosRpcs.ts (2)
111-139: Fix unsafe type assertion in error handling.The bulk analytics refactoring is a good improvement over per-item fetching, but line 122 contains a critical type safety issue:
return error as Video.NotFoundError | Policy.PolicyDeniedError | Video.VerifyVideoPasswordError;This catch-all branch forces any unhandled error into the union type without validation. If
getAnalyticsBulkreturns a different error type (e.g.,DatabaseError,S3Error, or any other error), this assertion creates a type lie that could lead to incorrect error handling downstream.Apply this diff to properly handle unknown errors:
- return error as Video.NotFoundError | Policy.PolicyDeniedError | Video.VerifyVideoPasswordError; + // If we encounter an unexpected error type, fail the entire Effect + // rather than incorrectly coercing it + return Effect.die( + new Error(`Unexpected error type in analytics fetch: ${error}`) + );Alternatively, if unknown errors should be treated as NotFoundError or another specific type, make that explicit:
- return error as Video.NotFoundError | Policy.PolicyDeniedError | Video.VerifyVideoPasswordError; + // Treat unknown errors as NotFoundError + return new Video.NotFoundError();Note: The explicit type annotation on lines 125-128 appears redundant and could be removed for cleaner code, as TypeScript should infer the type from the mapping logic.
1-142: Fix formatting to pass CI checks.The CI pipeline detected a formatting issue. Please run the formatter to fix the code style:
pnpm formatapps/web/app/(org)/dashboard/caps/Caps.tsx (1)
1-1: Fix formatting to pass CI.The pipeline is failing due to formatting issues. Run the formatter to resolve this.
pnpm formatAs per coding guidelines.
🧹 Nitpick comments (3)
apps/web/app/(org)/dashboard/_components/Navbar/Items.tsx (1)
55-61: ConsidermatchChildren: truefor "My Caps" if detail views exist.Currently, "My Caps" uses exact path matching, so it won't remain active when viewing individual cap details (e.g.,
/dashboard/caps/[id]). If such detail routes exist, addingmatchChildren: truewould keep the parent nav item highlighted when viewing child resources, which is a common UX pattern.If
/dashboard/caps/recordis intentionally a sibling action (not a child view), the current setup is correct.apps/web/app/s/[videoId]/page.tsx (2)
54-108: Consider combining queries for better performance.The function performs two separate database queries and then merges the results client-side. This could be optimized by using a UNION query to combine both space-level and organization-level sharing in a single database round-trip.
Apply this refactor:
async function getSharedSpacesForVideo(videoId: Video.VideoId) { - // Fetch space-level sharing - const spaceSharing = await db() - .select({ - id: spaces.id, - name: spaces.name, - organizationId: spaces.organizationId, - iconUrl: organizations.iconUrl, - }) - .from(spaceVideos) - .innerJoin(spaces, eq(spaceVideos.spaceId, spaces.id)) - .innerJoin(organizations, eq(spaces.organizationId, organizations.id)) - .where(eq(spaceVideos.videoId, videoId)); - - // Fetch organization-level sharing - const orgSharing = await db() - .select({ - id: organizations.id, - name: organizations.name, - organizationId: organizations.id, - iconUrl: organizations.iconUrl, - }) - .from(sharedVideos) - .innerJoin(organizations, eq(sharedVideos.organizationId, organizations.id)) - .where(eq(sharedVideos.videoId, videoId)); - - const sharedSpaces: Array<{ - id: string; - name: string; - organizationId: string; - iconUrl?: string; - }> = []; - - // Add space-level sharing - spaceSharing.forEach((space) => { - sharedSpaces.push({ - id: space.id, - name: space.name, - organizationId: space.organizationId, - iconUrl: space.iconUrl || undefined, - }); - }); - - // Add organization-level sharing - orgSharing.forEach((org) => { - sharedSpaces.push({ - id: org.id, - name: org.name, - organizationId: org.organizationId, - iconUrl: org.iconUrl || undefined, - }); - }); - - return sharedSpaces; + // Combine both space-level and org-level sharing in a single query + const sharedSpaces = await db().execute(sql` + SELECT + ${spaces.id} as id, + ${spaces.name} as name, + ${spaces.organizationId} as "organizationId", + ${organizations.iconUrl} as "iconUrl" + FROM ${spaceVideos} + INNER JOIN ${spaces} ON ${spaceVideos.spaceId} = ${spaces.id} + INNER JOIN ${organizations} ON ${spaces.organizationId} = ${organizations.id} + WHERE ${spaceVideos.videoId} = ${videoId} + + UNION + + SELECT + ${organizations.id} as id, + ${organizations.name} as name, + ${organizations.id} as "organizationId", + ${organizations.iconUrl} as "iconUrl" + FROM ${sharedVideos} + INNER JOIN ${organizations} ON ${sharedVideos.organizationId} = ${organizations.id} + WHERE ${sharedVideos.videoId} = ${videoId} + `); + + return sharedSpaces.map(space => ({ + id: space.id, + name: space.name, + organizationId: space.organizationId, + iconUrl: space.iconUrl || undefined, + })); }
351-771: Consider breaking down this large function.
AuthorizedContentspans ~420 lines and handles multiple concerns: authentication, notifications, spaces data, AI settings, domain restrictions, transcription, metadata generation, and rendering. This violates the Single Responsibility Principle and makes the code harder to maintain and test.Consider extracting logical sections into smaller focused functions:
fetchSpacesData(user)for lines 392-404checkAiGenerationEnabled(videoOwnerId)for lines 406-419validateDomainAccess(video, user)for lines 421-451handleTranscription(video, aiGenerationEnabled)for lines 453-499initializeAiMetadata(video, metadata, aiGenerationEnabled)for lines 501-542fetchVideoRelatedData(videoId, userId, user, video)for lines 544-699This would improve readability, testability, and maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/web/app/(org)/dashboard/_components/Navbar/Items.tsx(8 hunks)apps/web/app/(org)/dashboard/caps/Caps.tsx(2 hunks)apps/web/app/s/[videoId]/page.tsx(3 hunks)packages/web-backend/src/Videos/VideosRpcs.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
apps/web/app/(org)/dashboard/_components/Navbar/Items.tsxapps/web/app/s/[videoId]/page.tsxpackages/web-backend/src/Videos/VideosRpcs.tsapps/web/app/(org)/dashboard/caps/Caps.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
apps/web/app/(org)/dashboard/_components/Navbar/Items.tsxapps/web/app/s/[videoId]/page.tsxpackages/web-backend/src/Videos/VideosRpcs.tsapps/web/app/(org)/dashboard/caps/Caps.tsx
apps/web/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
On the client, always use
useEffectQueryoruseEffectMutationfrom@/lib/EffectRuntime; never callEffectRuntime.run*directly in components.
Files:
apps/web/app/(org)/dashboard/_components/Navbar/Items.tsxapps/web/app/s/[videoId]/page.tsxapps/web/app/(org)/dashboard/caps/Caps.tsx
🧬 Code graph analysis (4)
apps/web/app/(org)/dashboard/_components/Navbar/Items.tsx (1)
apps/web/app/(org)/dashboard/_components/AnimatedIcons/index.ts (1)
ChartLineIcon(27-27)
apps/web/app/s/[videoId]/page.tsx (13)
packages/database/index.ts (1)
db(18-25)packages/database/schema.ts (5)
spaces(614-641)organizations(173-210)videos(291-363)users(61-120)comments(394-420)packages/web-backend/src/Auth.ts (2)
provideOptionalAuth(102-114)getCurrentUser(15-36)apps/web/lib/EffectRuntime.ts (1)
EffectRuntime(20-20)packages/web-backend/src/Videos/VideosPolicy.ts (1)
VideosPolicy(9-96)apps/web/app/(org)/dashboard/dashboard-data.ts (2)
OrganizationSettings(34-36)getDashboardData(49-391)apps/web/lib/Notification.ts (1)
createNotification(33-219)apps/web/utils/effect.ts (1)
optionFromTOrFirst(3-4)apps/web/lib/transcribe.ts (1)
transcribeVideo(17-229)packages/web-backend/src/Database.ts (1)
Database(7-17)apps/web/actions/videos/get-analytics.ts (1)
getVideoAnalytics(34-96)apps/web/app/s/[videoId]/_components/ShareHeader.tsx (1)
ShareHeader(24-318)apps/web/app/s/[videoId]/Share.tsx (1)
Share(233-504)
packages/web-backend/src/Videos/VideosRpcs.ts (2)
packages/web-domain/src/Video.ts (3)
Video(16-59)NotFoundError(165-169)VerifyVideoPasswordError(133-139)packages/web-domain/src/Policy.ts (2)
Policy(8-12)PolicyDeniedError(20-24)
apps/web/app/(org)/dashboard/caps/Caps.tsx (5)
packages/web-domain/src/Video.ts (3)
Video(16-59)VideoId(12-12)VideoId(13-13)apps/web/app/(org)/dashboard/caps/components/Folder.tsx (1)
FolderDataType(18-25)apps/web/lib/Queries/Analytics.ts (1)
useVideosAnalyticsQuery(10-70)apps/web/lib/EffectRuntime.ts (2)
useRpcClient(25-25)useEffectMutation(23-23)apps/web/app/(org)/dashboard/caps/UploadingContext.tsx (1)
useUploadingStatus(50-62)
🪛 GitHub Actions: CI
apps/web/app/s/[videoId]/page.tsx
[error] 1-1: File content differs from formatting output. Re-run the formatter to fix code style.
packages/web-backend/src/Videos/VideosRpcs.ts
[error] 1-1: File content differs from formatting output. Re-run the formatter to fix code style.
apps/web/app/(org)/dashboard/caps/Caps.tsx
[error] 1-1: File content differs from formatting output. Re-run the formatter to fix code style.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (rust)
🔇 Additional comments (8)
packages/web-backend/src/Videos/VideosRpcs.ts (1)
1-1: LGTM! Policy import supports the analytics error handling.The Policy import is correctly added to support PolicyDeniedError handling in the VideosGetAnalytics method.
apps/web/app/(org)/dashboard/_components/Navbar/Items.tsx (1)
62-68: Previous critical issues resolved - excellent implementation!The renamed
matchChildrenparameter and updated path-matching logic successfully address the critical issues raised in previous reviews:
- ✅ Improved naming:
matchChildrenclearly expresses intent—whentrue, the nav item remains active for child routes.- ✅ Fixed matching logic: The segment-aware approach (
pathname === path || pathname.startsWith(\${path}/`)) prevents multiple nav items from being active simultaneously, which previously broke thelayoutId="navlinks"` animation.The new Analytics navigation item correctly uses
matchChildren: true, which is appropriate for a parent route with nested views.Also applies to: 94-100
apps/web/app/(org)/dashboard/caps/Caps.tsx (5)
55-85: LGTM! Analytics integration is clean.The prop rename from
dubApiKeyEnabledtoanalyticsEnabledaligns with the PR's goal of replacing DUB analytics. The analytics query is correctly gated by the flag, and the loading state computation is appropriate.
103-159: LGTM! Comprehensive batch deletion logic.The mutation handles all scenarios well: full success, partial success, and complete failure. The concurrency limit of 10 is reasonable, and the toast notifications provide clear feedback.
161-170: LGTM!Single deletion mutation is clean and follows the same pattern as the batch deletion.
279-300: LGTM! Analytics properly wired to CapCard.The per-video analytics lookup, fallback to 0, and loading state handling are all implemented correctly. The delete logic appropriately chooses between batch and single deletion based on selection state.
309-314: LGTM!The
SelectedCapsBarintegration correctly combines both deletion states and uses the batch deletion mutation.apps/web/app/s/[videoId]/page.tsx (1)
371-381: This review comment is incorrect.The server-side
createNotification()and client-sidetrackVideoView()are separate systems serving different purposes:
- Server-side notification (page.tsx line 371): Stores a persistent record in the database for the video owner to see who has viewed their video.
- Client-side analytics (Share.tsx line 278): Sends telemetry data to
/api/analytics/trackfor viewing patterns and metrics.Both use the same condition (skip for video owners), but they target different endpoints and systems. This is intentional, not duplicate tracking. There is no double-counting issue.
Likely an incorrect or invalid review comment.
| const rpc = useRpcClient() as { | ||
| VideoDelete: (id: Video.VideoId) => Effect.Effect<void, unknown, never>; | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
Avoid manual type assertion; use proper RPC client typing.
The manual as cast bypasses TypeScript's type safety. If the actual VideoDelete signature differs, this could lead to runtime errors.
Remove the type assertion and rely on the inferred type from useRpcClient():
- const rpc = useRpcClient() as {
- VideoDelete: (id: Video.VideoId) => Effect.Effect<void, unknown, never>;
- };
+ const rpc = useRpcClient();Run the following script to verify the RPC client's actual type definition:
🏁 Script executed:
#!/bin/bash
# Find the VideoDelete RPC method definition
rg -n "VideoDelete" --type=ts -A 5 -B 2Length of output: 5807
🏁 Script executed:
#!/bin/bash
# Find useRpcClient definition
rg -n "useRpcClient" --type=ts -B 3 -A 10 | grep -A 10 "function\|const useRpcClient\|export.*useRpcClient" | head -40Length of output: 3344
🏁 Script executed:
#!/bin/bash
# Check RPC client typing/schema
fd -e ts -e tsx | xargs grep -l "useRpcClient\|RpcClient" | head -5Length of output: 311
🏁 Script executed:
#!/bin/bash
# Look for RPC client types and their effect signatures
rg -n "Effect\.Effect.*VideoDelete\|VideoDelete.*Effect\.Effect" --type=ts -B 2 -A 2Length of output: 0
🏁 Script executed:
#!/bin/bash
# Find where VideoRpcs is typed/exported
rg -n "VideoRpcs\|export.*Video.*Rpc" --type=ts -B 2 -A 5 packages/web-domain/src/Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check the actual Rpc.make implementation to understand its return type
rg -n "class Rpc\|function.*Rpc\.make\|Rpc\.make.*=>" --type=ts -A 8 | head -50Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check EffectRuntime.ts to see what runSync returns and what Rpc is
cat apps/web/lib/EffectRuntime.tsLength of output: 846
🏁 Script executed:
#!/bin/bash
# Find Rpc import/definition in EffectRuntime.ts context
rg -n "import.*Rpc\|from.*rpc\|Rpc.*=" apps/web/lib/EffectRuntime.ts -B 5 -A 5Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check if there's a type definition for the RPC client or similar
rg -n "RpcClient|RpcType" apps/web/lib/ --type=tsLength of output: 658
🏁 Script executed:
#!/bin/bash
# Verify other usages of rpc in Caps.tsx to ensure type safety without cast
rg -n "rpc\." apps/web/app/\(org\)/dashboard/caps/Caps.tsx -B 1 -A 1Length of output: 259
🏁 Script executed:
#!/bin/bash
# Check the imports in Caps.tsx to understand Video type and Rpc setup
head -40 apps/web/app/\(org\)/dashboard/caps/Caps.tsxLength of output: 1428
🏁 Script executed:
[skip_cloning]Length of output: 52
🏁 Script executed:
#!/bin/bash
# Check Rpcs.ts to see what type is exported as Rpc
cat apps/web/lib/Rpcs.tsLength of output: 592
🏁 Script executed:
#!/bin/bash
# Check if VideoDelete error type is truly `never` or has errors
rg -n "VideoDelete\|error:" packages/web-domain/src/Video.ts -B 2 -A 2Length of output: 0
🏁 Script executed:
[skip_cloning]Length of output: 52
🏁 Script executed:
#!/bin/bash
# Get the full VideoDelete RPC definition
sed -n '170,180p' packages/web-domain/src/Video.tsLength of output: 390
🏁 Script executed:
#!/bin/bash
# Check if Rpcs includes VideoRpcs and confirm the structure
rg -n "class Rpcs\|VideoRpcs\|export" packages/web-domain/src/index.ts -A 3 -B 1 | head -50Length of output: 0
🏁 Script executed:
[skip_cloning]Length of output: 52
Remove the unsafe type assertion; TypeScript already infers the correct RPC type.
The manual as cast claims an error type of never, but the actual VideoDelete RPC defines errors as Schema.Union(NotFoundError, InternalError, PolicyDeniedError). This type mismatch bypasses type safety. Other files in the codebase (CustomDomainPage.tsx, FolderVideosSection.tsx, useWebRecorder.ts) successfully use useRpcClient() without type assertions, proving it's unnecessary and the type is already correctly inferred by TypeScript.
- const rpc = useRpcClient() as {
- VideoDelete: (id: Video.VideoId) => Effect.Effect<void, unknown, never>;
- };
+ const rpc = useRpcClient();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const rpc = useRpcClient() as { | |
| VideoDelete: (id: Video.VideoId) => Effect.Effect<void, unknown, never>; | |
| }; | |
| const rpc = useRpcClient(); |
🤖 Prompt for AI Agents
In apps/web/app/(org)/dashboard/caps/Caps.tsx around lines 99 to 101, remove the
unsafe "as" type assertion on useRpcClient() which forces VideoDelete to return
never for errors; TypeScript already infers the correct RPC type (including
Schema.Union error types). Simply call const rpc = useRpcClient(); and use
rpc.VideoDelete(...) without casting so the real error union types
(NotFoundError, InternalError, PolicyDeniedError) are preserved by the compiler.
| useEffect(() => { | ||
| const handleKeyDown = (e: KeyboardEvent) => { | ||
| if (e.key === "Escape" && selectedCaps.length > 0) { | ||
| setSelectedCaps([]); | ||
| } | ||
|
|
||
| return newSelection; | ||
| }); | ||
| }; | ||
| if ( | ||
| (e.key === "Delete" || e.key === "Backspace") && | ||
| selectedCaps.length > 0 | ||
| ) { | ||
| if (e.key === "Backspace") { | ||
| e.preventDefault(); | ||
| } | ||
|
|
||
| const rpc = useRpcClient(); | ||
| if ( | ||
| !["INPUT", "TEXTAREA", "SELECT"].includes( | ||
| document.activeElement?.tagName || "" | ||
| ) | ||
| ) { | ||
| deleteCaps(selectedCaps); | ||
| } | ||
| } | ||
|
|
||
| const { mutate: deleteCaps, isPending: isDeletingCaps } = useEffectMutation({ | ||
| mutationFn: Effect.fn(function* (ids: Video.VideoId[]) { | ||
| if (ids.length === 0) return; | ||
| if (e.key === "a" && (e.ctrlKey || e.metaKey) && data.length > 0) { | ||
| if ( | ||
| !["INPUT", "TEXTAREA", "SELECT"].includes( | ||
| document.activeElement?.tagName || "" | ||
| ) | ||
| ) { | ||
| e.preventDefault(); | ||
| setSelectedCaps(data.map((cap) => cap.id)); | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| const fiber = yield* Effect.gen(function* () { | ||
| const results = yield* Effect.all( | ||
| ids.map((id) => rpc.VideoDelete(id).pipe(Effect.exit)), | ||
| { concurrency: 10 }, | ||
| ); | ||
| window.addEventListener("keydown", handleKeyDown); | ||
|
|
||
| const successCount = results.filter(Exit.isSuccess).length; | ||
| return () => { | ||
| window.removeEventListener("keydown", handleKeyDown); | ||
| }; | ||
| }, [selectedCaps, data, deleteCaps]); |
There was a problem hiding this comment.
Remove mutation function from useEffect dependencies.
The deleteCaps mutation function is stable and doesn't need to be in the dependency array. Including it is unnecessary and deviates from React best practices.
return () => {
window.removeEventListener("keydown", handleKeyDown);
};
- }, [selectedCaps, data, deleteCaps]);
+ }, [selectedCaps, data]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| useEffect(() => { | |
| const handleKeyDown = (e: KeyboardEvent) => { | |
| if (e.key === "Escape" && selectedCaps.length > 0) { | |
| setSelectedCaps([]); | |
| } | |
| return newSelection; | |
| }); | |
| }; | |
| if ( | |
| (e.key === "Delete" || e.key === "Backspace") && | |
| selectedCaps.length > 0 | |
| ) { | |
| if (e.key === "Backspace") { | |
| e.preventDefault(); | |
| } | |
| const rpc = useRpcClient(); | |
| if ( | |
| !["INPUT", "TEXTAREA", "SELECT"].includes( | |
| document.activeElement?.tagName || "" | |
| ) | |
| ) { | |
| deleteCaps(selectedCaps); | |
| } | |
| } | |
| const { mutate: deleteCaps, isPending: isDeletingCaps } = useEffectMutation({ | |
| mutationFn: Effect.fn(function* (ids: Video.VideoId[]) { | |
| if (ids.length === 0) return; | |
| if (e.key === "a" && (e.ctrlKey || e.metaKey) && data.length > 0) { | |
| if ( | |
| !["INPUT", "TEXTAREA", "SELECT"].includes( | |
| document.activeElement?.tagName || "" | |
| ) | |
| ) { | |
| e.preventDefault(); | |
| setSelectedCaps(data.map((cap) => cap.id)); | |
| } | |
| } | |
| }; | |
| const fiber = yield* Effect.gen(function* () { | |
| const results = yield* Effect.all( | |
| ids.map((id) => rpc.VideoDelete(id).pipe(Effect.exit)), | |
| { concurrency: 10 }, | |
| ); | |
| window.addEventListener("keydown", handleKeyDown); | |
| const successCount = results.filter(Exit.isSuccess).length; | |
| return () => { | |
| window.removeEventListener("keydown", handleKeyDown); | |
| }; | |
| }, [selectedCaps, data, deleteCaps]); | |
| useEffect(() => { | |
| const handleKeyDown = (e: KeyboardEvent) => { | |
| if (e.key === "Escape" && selectedCaps.length > 0) { | |
| setSelectedCaps([]); | |
| } | |
| if ( | |
| (e.key === "Delete" || e.key === "Backspace") && | |
| selectedCaps.length > 0 | |
| ) { | |
| if (e.key === "Backspace") { | |
| e.preventDefault(); | |
| } | |
| if ( | |
| !["INPUT", "TEXTAREA", "SELECT"].includes( | |
| document.activeElement?.tagName || "" | |
| ) | |
| ) { | |
| deleteCaps(selectedCaps); | |
| } | |
| } | |
| if (e.key === "a" && (e.ctrlKey || e.metaKey) && data.length > 0) { | |
| if ( | |
| !["INPUT", "TEXTAREA", "SELECT"].includes( | |
| document.activeElement?.tagName || "" | |
| ) | |
| ) { | |
| e.preventDefault(); | |
| setSelectedCaps(data.map((cap) => cap.id)); | |
| } | |
| } | |
| }; | |
| window.addEventListener("keydown", handleKeyDown); | |
| return () => { | |
| window.removeEventListener("keydown", handleKeyDown); | |
| }; | |
| }, [selectedCaps, data]); |
🤖 Prompt for AI Agents
In apps/web/app/(org)/dashboard/caps/Caps.tsx around lines 172-212 the useEffect
dependency array includes the stable mutation function deleteCaps unnecessarily;
remove deleteCaps from the dependency array so it becomes [selectedCaps, data],
and if your linter complains add a single-line exemption (//
eslint-disable-next-line react-hooks/exhaustive-deps) immediately above the
useEffect declaration or alternatively memoize deleteCaps with useCallback where
it's defined and keep the linter happy.
apps/web/app/s/[videoId]/page.tsx
Outdated
| let toplLevelCommentId = Option.none<Comment.CommentId>(); | ||
|
|
||
| if (Option.isSome(replyId)) { | ||
| const [parentComment] = yield* db.use((db) => | ||
| db | ||
| .select({ parentCommentId: comments.parentCommentId }) | ||
| .from(comments) | ||
| .where(eq(comments.id, replyId.value)) | ||
| .limit(1) | ||
| ); | ||
| toplLevelCommentId = Option.fromNullable(parentComment?.parentCommentId); | ||
| } | ||
|
|
||
| const commentToBringToTheTop = Option.orElse( | ||
| toplLevelCommentId, | ||
| () => commentId | ||
| ); |
There was a problem hiding this comment.
Fix typo in variable name.
The variable toplLevelCommentId should be topLevelCommentId (missing the second 'e').
Apply this diff:
- let toplLevelCommentId = Option.none<Comment.CommentId>();
+ let topLevelCommentId = Option.none<Comment.CommentId>();
if (Option.isSome(replyId)) {
const [parentComment] = yield* db.use((db) =>
@@ -631,11 +631,11 @@
.where(eq(comments.id, replyId.value))
.limit(1)
);
- toplLevelCommentId = Option.fromNullable(parentComment?.parentCommentId);
+ topLevelCommentId = Option.fromNullable(parentComment?.parentCommentId);
}
const commentToBringToTheTop = Option.orElse(
- toplLevelCommentId,
+ topLevelCommentId,
() => commentId
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let toplLevelCommentId = Option.none<Comment.CommentId>(); | |
| if (Option.isSome(replyId)) { | |
| const [parentComment] = yield* db.use((db) => | |
| db | |
| .select({ parentCommentId: comments.parentCommentId }) | |
| .from(comments) | |
| .where(eq(comments.id, replyId.value)) | |
| .limit(1) | |
| ); | |
| toplLevelCommentId = Option.fromNullable(parentComment?.parentCommentId); | |
| } | |
| const commentToBringToTheTop = Option.orElse( | |
| toplLevelCommentId, | |
| () => commentId | |
| ); | |
| let topLevelCommentId = Option.none<Comment.CommentId>(); | |
| if (Option.isSome(replyId)) { | |
| const [parentComment] = yield* db.use((db) => | |
| db | |
| .select({ parentCommentId: comments.parentCommentId }) | |
| .from(comments) | |
| .where(eq(comments.id, replyId.value)) | |
| .limit(1) | |
| ); | |
| topLevelCommentId = Option.fromNullable(parentComment?.parentCommentId); | |
| } | |
| const commentToBringToTheTop = Option.orElse( | |
| topLevelCommentId, | |
| () => commentId | |
| ); |
🤖 Prompt for AI Agents
In apps/web/app/s/[videoId]/page.tsx around lines 624 to 640, there's a typo:
the variable is named "toplLevelCommentId" but should be "topLevelCommentId";
rename the variable declaration and all subsequent references (including the
Option.fromNullable assignment and the Option.orElse call) to topLevelCommentId
to fix the spelling and ensure consistent usage.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
apps/web/app/api/analytics/track/route.ts (2)
47-88: ValidateoccurredAtbefore callingtoISOString().If a client passes an invalid
occurredAt,new Date(body.occurredAt)becomesInvalid Date, and callingtoISOString()on line 88 will throw a RangeError, returning a 500 instead of a proper 400 validation error.Apply this diff to add validation:
const timestamp = body.occurredAt ? new Date(body.occurredAt) : new Date(); + +if (Number.isNaN(timestamp.getTime())) { + return Response.json( + { error: "occurredAt must be a valid ISO timestamp" }, + { status: 400 } + ); +}
25-108: Rebuild the route using the required HttpApi builder.Per the established API conventions, this handler must be expressed via
@effect/platform'sHttpApibuilder and exported throughapiToHandlerrather than invokingrunPromisedirectly. Please restructure the route to align with the shared API pattern.Based on learnings
apps/web/app/(org)/dashboard/caps/Caps.tsx (2)
172-212: Remove stable mutation function from dependencies.The
deleteCapsmutation function is stable and doesn't need to be in the dependency array. This deviates from React best practices.return () => { window.removeEventListener("keydown", handleKeyDown); }; - }, [selectedCaps, data, deleteCaps]); + }, [selectedCaps, data]);
99-101: Remove the unsafe type assertion.The manual
ascast bypasses TypeScript's type safety. The actualVideoDeleteRPC defines errors asSchema.Union(NotFoundError, InternalError, PolicyDeniedError), notnever. Simply use:- const rpc = useRpcClient() as { - VideoDelete: (id: Video.VideoId) => Effect.Effect<void, unknown, never>; - }; + const rpc = useRpcClient();
🧹 Nitpick comments (5)
scripts/analytics/tinybird/pipes/analytics_sessions_mv_pipe.pipe (2)
2-3: Consider updating the description to include "os".The description mentions "browser/device metadata" but the query also materializes
os(line 11). For completeness, consider updating to "browser/device/os metadata".
7-7: Prefer deterministic timestamp aggregation for the date field.Using
anyLast(timestamp)picks an arbitrary timestamp from the session, making the date field non-deterministic. Consider usingmin(timestamp)for the session start date ormax(timestamp)for the session end date to ensure consistent, meaningful results.Apply this diff to use the session start date:
- toDate(anyLast(timestamp)) AS date, + toDate(min(timestamp)) AS date,apps/web/app/api/analytics/track/route.ts (1)
84-84: Remove unused@ts-expect-errordirective.Static analysis indicates this directive is no longer needed, suggesting the type issue has been resolved. Please remove it for cleaner code.
Apply this diff:
-// @ts-expect-error - Tinybird service can be yielded directly const tinybird = yield* Tinybird;apps/web/app/(org)/dashboard/caps/Caps.tsx (1)
103-159: Consider dismissing the loading toast explicitly.The
toast.loading()call inonMutateis not explicitly dismissed. While subsequenttoast.success()ortoast.error()calls will display, the loading toast may linger. Consider usingtoast.promise()for automatic lifecycle management or storing the toast ID to dismiss it.Example with
toast.promise():const { mutate: deleteCaps, isPending: isDeletingCaps } = useEffectMutation({ mutationFn: Effect.fn(function* (ids: Video.VideoId[]) { // ... existing logic }), onMutate: (ids: Video.VideoId[]) => { // toast.promise handles loading/success/error automatically }, onSuccess: (data) => { setSelectedCaps([]); router.refresh(); }, // Consider wrapping the mutation call with toast.promise where it's invoked });Alternatively, store and dismiss the toast ID:
let toastId: string | number; onMutate: (ids: Video.VideoId[]) => { toastId = toast.loading(`Deleting ${ids.length} cap${ids.length === 1 ? "" : "s"}...`); }, onSuccess: (data) => { toast.dismiss(toastId); // ... rest of success logic }, onError: (error) => { toast.dismiss(toastId); // ... rest of error logic },apps/web/app/(org)/dashboard/analytics/components/AnalyticsDashboard.tsx (1)
82-98: Consider using consistent fallback for topCaps.Line 89 sets
topCapstonullwhencapIdis present, while line 97 uses an empty array[]as the fallback. Both prevent rendering inOtherStats, but consistency would improve code clarity.: { countries: [], cities: [], browsers: [], operatingSystems: [], deviceTypes: [], - topCaps: [], + topCaps: null, };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/web/public/logos/browsers/maxthon.svgis excluded by!**/*.svg
📒 Files selected for processing (8)
apps/web/app/(org)/dashboard/_components/Navbar/Items.tsx(8 hunks)apps/web/app/(org)/dashboard/analytics/components/AnalyticsDashboard.tsx(1 hunks)apps/web/app/(org)/dashboard/caps/Caps.tsx(5 hunks)apps/web/app/api/analytics/track/route.ts(1 hunks)apps/web/app/s/[videoId]/Share.tsx(5 hunks)apps/web/app/s/[videoId]/page.tsx(6 hunks)packages/web-backend/src/Videos/VideosRpcs.ts(2 hunks)scripts/analytics/tinybird/pipes/analytics_sessions_mv_pipe.pipe(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/web-backend/src/Videos/VideosRpcs.ts
- apps/web/app/s/[videoId]/page.tsx
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
apps/web/app/s/[videoId]/Share.tsxapps/web/app/(org)/dashboard/_components/Navbar/Items.tsxapps/web/app/api/analytics/track/route.tsapps/web/app/(org)/dashboard/analytics/components/AnalyticsDashboard.tsxapps/web/app/(org)/dashboard/caps/Caps.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
apps/web/app/s/[videoId]/Share.tsxapps/web/app/(org)/dashboard/_components/Navbar/Items.tsxapps/web/app/api/analytics/track/route.tsapps/web/app/(org)/dashboard/analytics/components/AnalyticsDashboard.tsxapps/web/app/(org)/dashboard/caps/Caps.tsx
apps/web/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
On the client, always use
useEffectQueryoruseEffectMutationfrom@/lib/EffectRuntime; never callEffectRuntime.run*directly in components.
Files:
apps/web/app/s/[videoId]/Share.tsxapps/web/app/(org)/dashboard/_components/Navbar/Items.tsxapps/web/app/api/analytics/track/route.tsapps/web/app/(org)/dashboard/analytics/components/AnalyticsDashboard.tsxapps/web/app/(org)/dashboard/caps/Caps.tsx
🧠 Learnings (5)
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to apps/web/**/*.{ts,tsx,js,jsx} : On the client, always use `useEffectQuery` or `useEffectMutation` from `@/lib/EffectRuntime`; never call `EffectRuntime.run*` directly in components.
Applied to files:
apps/web/app/s/[videoId]/Share.tsxapps/web/app/(org)/dashboard/analytics/components/AnalyticsDashboard.tsxapps/web/app/(org)/dashboard/caps/Caps.tsx
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to apps/web/app/api/* : API routes in Next.js (`apps/web/app/api/*`) must use `effect/platform`'s `HttpApi` builder and follow the existing class/group/endpoint pattern; avoid ad-hoc handlers.
Applied to files:
apps/web/app/api/analytics/track/route.tsapps/web/app/(org)/dashboard/analytics/components/AnalyticsDashboard.tsx
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to apps/web/app/api/* : Convert effectful APIs to Next.js handlers using `apiToHandler(ApiLive)` and export only the returned `handler`; avoid calling `runPromise` directly in route files.
Applied to files:
apps/web/app/api/analytics/track/route.ts
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to apps/web/app/api/* : On the server, always run effects through `EffectRuntime.runPromise` after `provideOptionalAuth` to ensure cookies and per-request context are attached.
Applied to files:
apps/web/app/api/analytics/track/route.tsapps/web/app/(org)/dashboard/analytics/components/AnalyticsDashboard.tsx
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to **/queries.ts : Do not edit auto-generated files named `queries.ts`.
Applied to files:
apps/web/app/(org)/dashboard/analytics/components/AnalyticsDashboard.tsx
🧬 Code graph analysis (4)
apps/web/app/(org)/dashboard/_components/Navbar/Items.tsx (1)
apps/web/app/(org)/dashboard/_components/AnimatedIcons/index.ts (1)
ChartLineIcon(27-27)
apps/web/app/api/analytics/track/route.ts (3)
apps/web/lib/server.ts (1)
runPromise(141-153)packages/web-backend/src/Tinybird/index.ts (1)
Tinybird(28-276)packages/web-backend/src/Auth.ts (1)
provideOptionalAuth(102-114)
apps/web/app/(org)/dashboard/analytics/components/AnalyticsDashboard.tsx (5)
apps/web/app/(org)/dashboard/analytics/types.ts (2)
AnalyticsRange(1-1)OrgAnalyticsResponse(10-33)apps/web/app/(org)/dashboard/Contexts.tsx (1)
useDashboardContext(50-50)apps/web/lib/EffectRuntime.ts (1)
useEffectQuery(22-22)apps/web/app/(org)/dashboard/analytics/components/Header.tsx (1)
Header(90-429)apps/web/app/(org)/dashboard/analytics/components/OtherStats.tsx (1)
OtherStats(122-202)
apps/web/app/(org)/dashboard/caps/Caps.tsx (5)
apps/web/app/(org)/dashboard/spaces/[spaceId]/components/AddVideosDialogBase.tsx (1)
VideoData(37-50)apps/web/app/(org)/dashboard/caps/components/Folder.tsx (1)
FolderDataType(18-25)packages/web-domain/src/Video.ts (3)
Video(16-59)VideoId(12-12)VideoId(13-13)apps/web/lib/EffectRuntime.ts (2)
useRpcClient(25-25)useEffectMutation(23-23)apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx (1)
CapCard(101-648)
🪛 GitHub Check: Typecheck
apps/web/app/api/analytics/track/route.ts
[failure] 84-84:
Unused '@ts-expect-error' directive.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (rust)
🔇 Additional comments (16)
scripts/analytics/tinybird/pipes/analytics_sessions_mv_pipe.pipe (2)
5-14: Previous critical issue resolved.The query now correctly groups by
tenant_idandsession_id(line 14), ensuring exactly one row per session is materialized. This addresses the duplicate session issue from the previous review.
16-17: LGTM!The materialized view configuration is correct.
apps/web/app/s/[videoId]/Share.tsx (3)
38-63: LGTM! Session management is well-implemented.The session ID generation and persistence logic correctly handles:
- Non-browser environments
- Expiry checking with 30-minute TTL
- Crypto API availability with a safe fallback
- Error recovery with "anonymous" default
65-132: LGTM! Analytics tracking correctly implemented.The beacon-first approach with fetch fallback is appropriate. The previous issue with aborting the keepalive request has been correctly resolved by removing the AbortController.
273-283: LGTM! View tracking with proper privacy guard.The effect correctly skips tracking when the viewer is the video owner, preventing self-inflation of analytics. Dependencies are complete.
apps/web/app/(org)/dashboard/caps/Caps.tsx (5)
58-64: LGTM! Clean prop rename.The transition from
dubApiKeyEnabledtoanalyticsEnabledis consistent and aligns with the Tinybird analytics migration.
79-85: LGTM! Analytics query integration is correct.The query setup properly gates analytics fetching with
analyticsEnabledand correctly computes the loading state.
87-97: LGTM! Selection toggle logic is sound.The toggle logic correctly adds/removes caps from selection and tracks the previous count appropriately.
161-170: LGTM! Single cap deletion is correctly implemented.The mutation uses
useEffectMutationproperly and provides appropriate user feedback.
280-294: LGTM! CapCard integration with analytics is correct.Per-video analytics are properly extracted and wired, and the delete logic appropriately handles both single and batch deletion scenarios.
apps/web/app/(org)/dashboard/analytics/components/AnalyticsDashboard.tsx (1)
56-56: Verify that disabling browser cache is intentional.The
cache: "no-store"option prevents browser caching, ensuring fresh analytics data but potentially increasing server load. While React Query'sstaleTime(60s) provides some caching, every navigation away and back will trigger a new fetch.If this is intentional for data freshness, consider documenting the decision. If not, removing
cache: "no-store"would allow browser caching and reduce unnecessary requests:const response = yield* Effect.tryPromise({ - try: () => fetch(url.toString(), { cache: "no-store" }), + try: () => fetch(url.toString()), catch: (cause: unknown) => cause as Error, });apps/web/app/(org)/dashboard/_components/Navbar/Items.tsx (5)
39-39: LGTM - ChartLineIcon import is correct.The import of
ChartLineIconfrom AnimatedIcons is properly structured and aligns with the new Analytics navigation item.
62-68: LGTM - Analytics navigation item properly configured.The new Analytics nav item is well-structured with appropriate
matchChildren: trueto handle analytics sub-routes.
94-100: Excellent improvements to path matching logic.The refactored
isPathActivefunction successfully addresses the previous review concerns:
- Clear naming:
matchChildrenaccurately describes the prefix-matching behavior (not query param handling).- Correct logic: Exact matching by default prevents multiple active items; optional prefix matching with
pathname.startsWith(\${path}/`)` ensures proper segment boundaries.The implementation correctly prevents
/dashboard/capsfrom matching/dashboard/capsulewhile allowing intentional parent-route matching whenmatchChildren: true.
290-290: LGTM - matchChildren properly propagated through component hierarchy.The
matchChildrenflag is correctly threaded from the navigation configuration throughAdminNavItemsrendering (line 290) to theNavItemcomponent props (lines 319, 409, 423) and final usage in path matching (line 444). Type signatures (line 421) and default value handling (?? false) are consistent throughout.Also applies to: 319-319, 409-409, 421-421, 423-423, 444-444
56-61: The review comment is based on an incorrect assumption about route structure.The codebase has no dynamic cap routes (like
/dashboard/caps/[id]or/dashboard/caps/edit/:id). The only routes under/dashboard/capsare:
/dashboard/caps(main caps list)/dashboard/caps/record(record page, which has its own separate nav item)Since no child routes exist that should keep "My Caps" highlighted, the absence of
matchChildrenis correct. The exact pathname matching works properly for the actual route structure.Likely an incorrect or invalid review comment.
| const query = useEffectQuery({ | ||
| queryKey: ["dashboard-analytics", orgId, selectedSpaceId, range, capId], | ||
| queryFn: () => | ||
| Effect.gen(function* () { | ||
| if (!orgId) return null; | ||
| const url = new URL("/api/dashboard/analytics", window.location.origin); | ||
| url.searchParams.set("orgId", orgId); | ||
| url.searchParams.set("range", range); | ||
| if (selectedSpaceId) { | ||
| url.searchParams.set("spaceId", selectedSpaceId); | ||
| } | ||
| if (capId) { | ||
| url.searchParams.set("capId", capId); | ||
| } | ||
| const response = yield* Effect.tryPromise({ | ||
| try: () => fetch(url.toString(), { cache: "no-store" }), | ||
| catch: (cause: unknown) => cause as Error, | ||
| }); | ||
| if (!response.ok) { | ||
| return yield* Effect.fail(new Error("Failed to load analytics")); | ||
| } | ||
| return yield* Effect.tryPromise({ | ||
| try: () => response.json() as Promise<{ data: OrgAnalyticsResponse }>, | ||
| catch: (cause: unknown) => cause as Error, | ||
| }); | ||
| }), | ||
| enabled: Boolean(orgId), | ||
| staleTime: 60 * 1000, | ||
| }); |
There was a problem hiding this comment.
Consider displaying error state when analytics fetch fails.
The query can fail (line 60), but query.error is never displayed to users. When the analytics API is unavailable, users see only a loading state or stale data without any indication of failure.
Add error handling after line 73:
const analytics = (query.data as { data: OrgAnalyticsResponse } | undefined)
?.data;
+
+ if (query.error) {
+ return (
+ <div className="rounded-xl border border-red-5 bg-red-2 p-6 text-red-11">
+ Failed to load analytics. Please try again later.
+ </div>
+ );
+ }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/web/app/(org)/dashboard/analytics/components/AnalyticsDashboard.tsx
around lines 41 to 69, the data fetch can fail but the component never surfaces
query.error to the UI; after the query is defined (around line 73) add handling
that checks query.isError or query.error and renders a visible error state
(e.g., an error message component or fallback UI) returning early from the
component so users see the failure instead of a perpetual loading/stale view;
include the error message text (or a user-friendly message) and optionally a
retry action that calls query.refetch.
This PR implements Cap analytics.
YOLO. God speed.
Summary by CodeRabbit
New Features
Documentation
Chores