Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import {
import { cn } from "@/lib/utils";
import { runAsynchronously } from "@stackframe/stack-shared/dist/utils/promises";
import { stringCompare } from "@stackframe/stack-shared/dist/utils/strings";
import { ArrowsClockwiseIcon, FastForwardIcon, GearIcon, MonitorPlayIcon, PauseIcon, PlayIcon } from "@phosphor-icons/react";
import { ArrowsClockwiseIcon, CursorClickIcon, FastForwardIcon, GearIcon, MonitorPlayIcon, PauseIcon, PlayIcon } from "@phosphor-icons/react";
import { Panel, PanelGroup, PanelResizeHandle } from "react-resizable-panels";
import React, { useCallback, useEffect, useMemo, useRef, useState } from "react";
import { AppEnabledGuard } from "../../app-enabled-guard";
Expand Down Expand Up @@ -113,6 +113,32 @@ function formatTimelineMs(ms: number) {
return `${m}:${s.toString().padStart(2, "0")}`;
}

type TimelineEvent = {
eventType: string,
eventAtMs: number,
data: Record<string, unknown>,
};

type TimelineMarker = {
timeMs: number,
eventType: string,
label: string,
};

function formatEventTooltip(event: TimelineEvent): string {
const d = event.data;
if (event.eventType === "$click") {
const tag = (d.tag_name as string) || "element";
return `Clicked ${tag}`;
}
if (event.eventType === "$page-view") {
const path = (d.path as string | undefined) ?? (d.url as string | undefined) ?? "/";
const truncated = path.length > 30 ? path.slice(0, 27) + "..." : path;
return truncated;
}
return event.eventType;
}
Comment on lines +128 to +140
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Replace as casts with runtime type guards; use ?? instead of ||.

Three as casts and one boolean-coercion fallback violate the project guidelines:

  • (d.tag_name as string) — no runtime guarantee tag_name is a string; cast should be a type guard.
  • || "element"|| swallows empty string; use ?? per the explicit-null-check rule.
  • (d.path as string | undefined) and (d.url as string | undefined) — same issue on line 135.
🔧 Proposed fix
 function formatEventTooltip(event: TimelineEvent): string {
   const d = event.data;
   if (event.eventType === "$click") {
-    const tag = (d.tag_name as string) || "element";
+    const tag = typeof d.tag_name === "string" ? d.tag_name : "element";
     return `Clicked ${tag}`;
   }
   if (event.eventType === "$page-view") {
-    const path = (d.path as string | undefined) ?? (d.url as string | undefined) ?? "/";
+    const path = (typeof d.path === "string" ? d.path : undefined)
+      ?? (typeof d.url === "string" ? d.url : undefined)
+      ?? "/";
     const truncated = path.length > 30 ? path.slice(0, 27) + "..." : path;
     return truncated;
   }
   return event.eventType;
 }

As per coding guidelines: "Do not use as, any, type casts, or other type system bypasses" and "Prefer explicit null/undefinedness checks over boolean checks (e.g., foo == null instead of !foo)".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/analytics/replays/page-client.tsx
around lines 128 - 140, In formatEventTooltip(TimelineEvent) remove the
TypeScript casts and boolean-fallback: check at runtime that d.tag_name is a
string (e.g., d.tag_name != null && typeof d.tag_name === "string") and use that
value or the literal "element" via nullish-coalescing logic; similarly for
d.path and d.url ensure they are strings at runtime before using them and
combine them with ?? (not ||) to pick the first defined string or "/" as the
final path; preserve the existing truncation and return behavior but replace all
uses of (d.tag_name as string), || "element", and (d.path as string |
undefined)/(d.url as string | undefined) with these explicit runtime guards.


function DisplayDate({ date }: { date: Date }) {
const fromNow = useFromNow(date);
return <span>{fromNow}</span>;
Expand Down Expand Up @@ -175,6 +201,7 @@ function Timeline({
onSeek,
playerSpeed,
onSpeedChange,
markers,
}: {
getCurrentTimeMs: () => number,
playerIsPlaying: boolean,
Expand All @@ -183,8 +210,10 @@ function Timeline({
onSeek: (timeOffset: number) => void,
playerSpeed: number,
onSpeedChange: (speed: number) => void,
markers?: TimelineMarker[],
}) {
const [currentTime, setCurrentTime] = useState(0);
const [hoveredMarkerIndex, setHoveredMarkerIndex] = useState<number | null>(null);
const trackRef = useRef<HTMLDivElement | null>(null);
const rafRef = useRef<number>(0);

Expand All @@ -208,8 +237,11 @@ function Timeline({
onSeek(timeOffset);
}, [totalTimeMs, onSeek]);

const hasMarkers = (markers?.length ?? 0) > 0;
const hoveredMarker = hoveredMarkerIndex !== null ? markers?.[hoveredMarkerIndex] ?? null : null;

return (
<div className="border-t border-border/30 bg-background px-3 py-2 flex items-center gap-3">
<div className={cn("border-t border-border/30 bg-background px-3 flex items-center gap-3", hasMarkers ? "py-1.5" : "py-2")}>
<Button
variant="ghost"
size="icon"
Expand All @@ -223,16 +255,62 @@ function Timeline({
{formatTimelineMs(currentTime)}
</span>

<div
ref={trackRef}
onClick={handleTrackClick}
className="flex-1 h-5 flex items-center cursor-pointer group"
>
<div className="w-full h-1.5 rounded-full bg-muted relative overflow-hidden">
<div
className="absolute inset-y-0 left-0 bg-foreground/60 group-hover:bg-foreground/80 rounded-full transition-colors"
style={{ width: `${progress * 100}%` }}
/>
<div className="flex-1 flex flex-col justify-center">
{/* Event markers lane */}
{hasMarkers && (
<div className="relative h-3.5 mb-0.5">
{markers?.map((marker, i) => {
const left = totalTimeMs > 0 ? (marker.timeMs / totalTimeMs) * 100 : 0;
if (left < 0 || left > 100) return null;
const isClick = marker.eventType === "$click";
return (
<div
key={i}
className={cn(
"absolute bottom-0 w-[3px] h-3 rounded-sm cursor-pointer",
"transition-colors",
isClick
? "bg-blue-500/70 hover:bg-blue-400"
: "bg-emerald-500/70 hover:bg-emerald-400",
)}
Comment on lines +269 to +275
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Add hover:transition-none to avoid a hover-enter fade.

The "transition-colors" class alone applies a transition on both enter and exit. The rest of the file (e.g., the recording list buttons on line 1345) already follows the correct pattern.

🔧 Proposed fix
-            "transition-colors",
+            "transition-colors hover:transition-none",

As per coding guidelines: "For hover transitions, avoid hover-enter transitions and use only hover-exit transitions (e.g., transition-colors hover:transition-none)".

📝 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.

Suggested change
className={cn(
"absolute bottom-0 w-[3px] h-3 rounded-sm cursor-pointer",
"transition-colors",
isClick
? "bg-blue-500/70 hover:bg-blue-400"
: "bg-emerald-500/70 hover:bg-emerald-400",
)}
className={cn(
"absolute bottom-0 w-[3px] h-3 rounded-sm cursor-pointer",
"transition-colors hover:transition-none",
isClick
? "bg-blue-500/70 hover:bg-blue-400"
: "bg-emerald-500/70 hover:bg-emerald-400",
)}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/analytics/replays/page-client.tsx
around lines 269 - 275, Update the JSX className that builds the timeline marker
(the cn(...) expression using isClick) to include the hover:transition-none
utility alongside transition-colors so hover-enter fades are disabled; locate
the cn call that currently contains "transition-colors" and add
"hover:transition-none" while keeping the conditional bg-blue-*/bg-emerald-*
classes unchanged.

style={{ left: `${left}%`, marginLeft: "-1.5px" }}
onMouseEnter={() => setHoveredMarkerIndex(i)}
onMouseLeave={() => setHoveredMarkerIndex((prev) => prev === i ? null : prev)}
onClick={() => onSeek(marker.timeMs)}
/>
);
})}

{/* Custom tooltip */}
{hoveredMarker && (() => {
const left = totalTimeMs > 0 ? (hoveredMarker.timeMs / totalTimeMs) * 100 : 0;
return (
<div
className="absolute bottom-full mb-1.5 -translate-x-1/2 pointer-events-none z-50"
style={{ left: `${left}%` }}
>
<div className="rounded-md bg-primary px-3 py-1.5 text-xs text-primary-foreground whitespace-nowrap max-w-52">
<div className="truncate">{hoveredMarker.label}</div>
<div className="text-[10px] opacity-70">{formatTimelineMs(hoveredMarker.timeMs)}</div>
</div>
</div>
);
})()}
</div>
)}

{/* Progress bar track (clickable) */}
<div
ref={trackRef}
onClick={handleTrackClick}
className="h-5 flex items-center cursor-pointer group"
>
<div className="w-full h-1.5 rounded-full bg-muted relative overflow-hidden">
<div
className="absolute inset-y-0 left-0 bg-foreground/60 group-hover:bg-foreground/80 rounded-full transition-colors"
style={{ width: `${progress * 100}%` }}
/>
</div>
</div>
</div>

Expand Down Expand Up @@ -346,6 +424,8 @@ export default function PageClient() {
const [loadingInitial, setLoadingInitial] = useState(true);
const [loadingMore, setLoadingMore] = useState(false);
const [listError, setListError] = useState<string | null>(null);
const [clickCountsByReplayId, setClickCountsByReplayId] = useState<Map<string, number>>(new Map());
const [timelineEvents, setTimelineEvents] = useState<TimelineEvent[]>([]);

const listBoxRef = useRef<HTMLDivElement | null>(null);

Expand Down Expand Up @@ -392,6 +472,28 @@ export default function PageClient() {
runAsynchronously(() => loadPage(null), { noErrorLogging: true });
}, [loadPage]);

useEffect(() => {
if (recordings.length === 0) return;
const ids = recordings.map(r => r.id);
runAsynchronously(async () => {
const res = await adminApp.queryAnalytics({
query: `SELECT session_replay_id, count() as cnt
FROM default.events
WHERE event_type = '$click'
AND session_replay_id IN ({ids:Array(String)})
GROUP BY session_replay_id`,
params: { ids },
include_all_branches: false,
timeout_ms: 15000,
});
const map = new Map<string, number>();
for (const row of res.result) {
map.set(row.session_replay_id as string, Number(row.cnt));
}
setClickCountsByReplayId(map);
}, { noErrorLogging: true });
Comment thread
BilalG1 marked this conversation as resolved.
}, [recordings, adminApp]);
Comment on lines +475 to +495
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Race condition: stale query can overwrite complete data; also has an as cast.

Race condition: The recordings array grows with every pagination fetch. When a second page loads, this effect re-fires with the full ID list and starts a new query. If the earlier (smaller) query resolves last (network race), it calls setClickCountsByReplayId with a map that only covers the original IDs, silently wiping the counts for all newer recordings. The timeline-events effect (line 1077) avoids the same problem with a cancelled flag — apply the same pattern here.

as cast (line 491): row.session_replay_id as string bypasses the type system; use a runtime check.

🔧 Proposed fix
 useEffect(() => {
   if (recordings.length === 0) return;
   const ids = recordings.map(r => r.id);
+  let cancelled = false;
   runAsynchronously(async () => {
     const res = await adminApp.queryAnalytics({
       query: `SELECT session_replay_id, count() as cnt
               FROM default.events
               WHERE event_type = '$click'
                 AND session_replay_id IN ({ids:Array(String)})
               GROUP BY session_replay_id`,
       params: { ids },
       include_all_branches: false,
       timeout_ms: 15000,
     });
+    if (cancelled) return;
     const map = new Map<string, number>();
     for (const row of res.result) {
-      map.set(row.session_replay_id as string, Number(row.cnt));
+      const replayId = typeof row.session_replay_id === "string" ? row.session_replay_id : String(row.session_replay_id);
+      map.set(replayId, Number(row.cnt));
     }
     setClickCountsByReplayId(map);
   }, { noErrorLogging: true });
+  return () => { cancelled = true; };
 }, [recordings, adminApp]);

As per coding guidelines: "Do not use as, any, type casts, or other type system bypasses".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/analytics/replays/page-client.tsx
around lines 475 - 495, Snapshot the current recordings IDs and guard the async
query with a cancelled flag so late responses cannot overwrite newer data:
inside the useEffect that references recordings and adminApp, create a local
cancelled boolean (let cancelled = false) and return a cleanup that sets
cancelled = true; capture const ids = recordings.map(r => r.id) before the async
call; inside runAsynchronously, after receiving res.result, build the Map only
for rows whose session_replay_id is a runtime-verified string (do not use "as
string") and only call setClickCountsByReplayId(map) if not cancelled; also
ensure you only populate entries for ids present in the local ids snapshot so
older queries cannot wipe counts for newer recordings (use the local ids set to
filter rows).


const onListScroll = useCallback(() => {
const el = listBoxRef.current;
if (!el) return;
Expand Down Expand Up @@ -967,6 +1069,41 @@ export default function PageClient() {
runAsynchronously(() => loadChunksAndDownload(selectedRecordingId), { noErrorLogging: true });
}, [loadChunksAndDownload, selectedRecordingId, selectedRecording]);

useEffect(() => {
if (!selectedRecordingId) {
setTimelineEvents([]);
return;
}
let cancelled = false;
setTimelineEvents([]);
runAsynchronously(async () => {
const res = await adminApp.queryAnalytics({
query: `SELECT event_type,
toUnixTimestamp64Milli(event_at) as event_at_ms,
data
FROM default.events
WHERE session_replay_id = {id:String}
AND event_type IN ('$click', '$page-view')
ORDER BY event_at ASC
LIMIT 2000`,
params: { id: selectedRecordingId },
include_all_branches: false,
timeout_ms: 15000,
});
if (cancelled) return;
setTimelineEvents(res.result.map((r: any) => ({
eventType: r.event_type as string,
eventAtMs: Number(r.event_at_ms),
data: typeof r.data === "string"
? JSON.parse(r.data)
: (r.data ?? {}),
})));
Comment thread
BilalG1 marked this conversation as resolved.
}, { noErrorLogging: true });
return () => {
cancelled = true;
};
}, [selectedRecordingId, adminApp]);
Comment on lines +1072 to +1105
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

any type, as cast, and silent JSON.parse failure in the timeline events effect.

Three issues in the result-mapping callback:

  1. (r: any)any type with no explanatory comment (guideline: "Avoid the any type; if necessary, leave a comment explaining why").
  2. r.event_type as string — bypasses the type system.
  3. JSON.parse(r.data) — if any row contains malformed JSON, the entire async function throws, runAsynchronously catches it, and noErrorLogging: true suppresses it completely. The result: all timeline markers silently disappear for that replay with no user feedback.

The data field should be parsed per-row with a safe fallback, and the any should be replaced with an explicit type annotation and runtime access.

🔧 Proposed fix
+    type AnalyticsEventRow = { event_type: unknown, event_at_ms: unknown, data: unknown };
     setTimelineEvents(res.result.map((r: AnalyticsEventRow) => ({
-      eventType: r.event_type as string,
+      eventType: typeof r.event_type === "string" ? r.event_type : String(r.event_type),
       eventAtMs: Number(r.event_at_ms),
-      data: typeof r.data === "string"
-        ? JSON.parse(r.data)
-        : (r.data ?? {}),
+      data: (() => {
+        if (typeof r.data !== "string") return (r.data != null && typeof r.data === "object") ? r.data as Record<string, unknown> : {};
+        try { const parsed = JSON.parse(r.data); return (parsed != null && typeof parsed === "object") ? parsed as Record<string, unknown> : {}; } catch { return {}; }
+      })(),
     })));

As per coding guidelines: "Avoid the any type", "Do not use as, any, type casts", and "Always carefully deal with loading and error states in frontend code; be explicit with these and never silently swallow errors".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/analytics/replays/page-client.tsx
around lines 1072 - 1105, The mapping callback should stop using (r: any) and
r.event_type as string and must safely parse per-row JSON without throwing;
define a local type/interface (e.g., ReplayEventRow with event_type: string,
event_at_ms: number|string, data?: string|Record<string, any>) and use that
instead of any, call adminApp.queryAnalytics inside the existing useEffect as
before, then map res.result as ReplayEventRow[] and for each row parse the data
field inside a try/catch (or use a safe-parse helper) to return {} on failure
while logging the parse error, convert event_at_ms to Number safely, and pass
objects to setTimelineEvents; this preserves non-fatal failures, removes 'as'
casts, and avoids silent swallowing by logging per-row parse issues while
keeping runAsynchronously({ noErrorLogging: true }) as-is.


useEffect(() => {
return () => {
genCounterRef.current += 1;
Expand Down Expand Up @@ -1144,6 +1281,15 @@ export default function PageClient() {

const showMainTabLabel = renderableStreamCount > 1;

const timelineMarkers = useMemo(() => {
if (timelineEvents.length === 0 || ms.globalTotalMs <= 0) return [];
return timelineEvents.map((e): TimelineMarker => ({
timeMs: e.eventAtMs - ms.globalStartTs,
eventType: e.eventType,
label: formatEventTooltip(e),
})).filter(m => m.timeMs >= 0 && m.timeMs <= ms.globalTotalMs);
}, [timelineEvents, ms.globalStartTs, ms.globalTotalMs]);

// ---- Rendering ----

return (
Expand Down Expand Up @@ -1208,8 +1354,14 @@ export default function PageClient() {
{duration}
</span>
</div>
<div className="text-xs text-muted-foreground">
<div className="flex items-center justify-between gap-2 text-xs text-muted-foreground">
<DisplayDate date={r.lastEventAt} />
{(clickCountsByReplayId.get(r.id) ?? 0) > 0 && (
<span className="flex items-center gap-0.5 text-[10px] text-muted-foreground/70">
<CursorClickIcon className="h-3 w-3" />
{clickCountsByReplayId.get(r.id)}
</span>
)}
</div>
</button>
);
Expand Down Expand Up @@ -1430,6 +1582,7 @@ export default function PageClient() {
onSeek={handleSeek}
playerSpeed={ms.settings.playerSpeed}
onSpeedChange={updateSpeed}
markers={timelineMarkers}
/>
)}
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -532,14 +532,15 @@ export class _StackClientAppImplIncomplete<HasTokenStore extends boolean, Projec
}

this._analyticsOptions = resolvedOptions.analytics;
const getAnalyticsAccessToken = async (): Promise<string | null> => {
this._ensurePersistentTokenStore();
return await (await this.getUser({ or: "anonymous" })).getAccessToken();
};

if (isBrowserLike() && this._analyticsOptions?.replays?.enabled === true) {
this._sessionRecorder = new SessionRecorder({
projectId: this.projectId,
getAccessToken: async () => {
const session = await this._getSession();
const tokens = await session.getOrFetchLikelyValidTokens(20_000, 75_000);
return tokens?.accessToken.token ?? null;
},
getAccessToken: getAnalyticsAccessToken,
sendBatch: async (body, opts) => {
return await this._interface.sendSessionReplayBatch(body, await this._getSession(), opts);
},
Expand All @@ -551,11 +552,7 @@ export class _StackClientAppImplIncomplete<HasTokenStore extends boolean, Projec
if (isBrowserLike() && this.projectId === "internal") {
this._eventTracker = new EventTracker({
projectId: this.projectId,
getAccessToken: async () => {
const session = await this._getSession();
const tokens = await session.getOrFetchLikelyValidTokens(20_000, 75_000);
return tokens?.accessToken.token ?? null;
},
getAccessToken: getAnalyticsAccessToken,
sendBatch: async (body, opts) => {
return await this._interface.sendAnalyticsEventBatch(body, await this._getSession(), opts);
},
Expand Down Expand Up @@ -2392,7 +2389,8 @@ export class _StackClientAppImplIncomplete<HasTokenStore extends boolean, Projec
case undefined:
case "anonymous-if-exists[deprecated]":
case "return-null": {
// do nothing
crud = null;
break;
}
}
}
Expand Down Expand Up @@ -2890,6 +2888,10 @@ export class _StackClientAppImplIncomplete<HasTokenStore extends boolean, Projec
}

protected async _signOut(session: InternalSession, options?: { redirectUrl?: URL | string }): Promise<void> {
// Clear analytics buffers before sign-out to prevent cross-user event leakage
this._eventTracker?.clearBuffer();
this._sessionRecorder?.clearBuffer();

await storeLock.withWriteLock(async () => {
await this._interface.signOut(session);
if (options?.redirectUrl) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,6 @@ const FLUSH_INTERVAL_MS = 10_000;
const MAX_EVENTS_PER_BATCH = 50;
const MAX_APPROX_BYTES_PER_BATCH = 64_000;

const MAX_PREAUTH_BUFFER_EVENTS = 500;
const MAX_PREAUTH_BUFFER_BYTES = 500_000;

export type EventTrackerDeps = {
projectId: string,
getAccessToken: () => Promise<string | null>,
Expand All @@ -30,7 +27,6 @@ export class EventTracker {
private _events: TrackedEvent[] = [];
private _approxBytes = 0;
private _lastKnownAccessToken: string | null = null;
private _wasAuthenticated = false;
private _lastUrl: string | null = null;
private readonly _sessionReplaySegmentId: string;
private readonly _deps: EventTrackerDeps;
Expand Down Expand Up @@ -65,18 +61,17 @@ export class EventTracker {
this._teardown();
}

clearBuffer() {
this._events = [];
this._approxBytes = 0;
}

private _pushEvent(event: TrackedEvent) {
this._events.push(event);
this._approxBytes += JSON.stringify(event).length;
if (this._events.length >= MAX_EVENTS_PER_BATCH || this._approxBytes >= MAX_APPROX_BYTES_PER_BATCH) {
runAsynchronously(() => this._flush({ keepalive: false }), { noErrorLogging: true });
}

// Cap pre-auth buffer
if (!this._lastKnownAccessToken && (this._events.length > MAX_PREAUTH_BUFFER_EVENTS || this._approxBytes > MAX_PREAUTH_BUFFER_BYTES)) {
this._events = [];
this._approxBytes = 0;
}
}

private _capturePageView(entryType: "initial" | "push" | "replace" | "pop") {
Expand Down Expand Up @@ -266,12 +261,6 @@ export class EventTracker {
}, { noErrorLogging: true });

const hasAuth = !!this._lastKnownAccessToken;
// Clear buffer on logout to prevent cross-user event leakage
if (this._wasAuthenticated && !hasAuth) {
this._events = [];
this._approxBytes = 0;
}
this._wasAuthenticated = hasAuth;
if (hasAuth && this._events.length > 0) {
runAsynchronously(() => this._flush({ keepalive: false }), { noErrorLogging: true });
}
Expand Down
Loading
Loading