apps/backend/src/app/api/latest/internal/session-recordings/route.tsx (1)

81-83: Prefer ?? throwErr(...) over non-null assertion.

Line 83 uses ! on page[page.length - 1]. While hasMore guarantees page is non-empty, the coding guidelines prefer explicit error messages over non-null assertions.

Suggested fix

-    const nextCursor = hasMore ? page[page.length - 1]!.id : null;
+    const nextCursor = hasMore ? (page[page.length - 1] ?? throwErr("Expected non-empty page when hasMore is true")).id : null;

You'll need to add the import:

import { throwErr } from "@stackframe/stack-shared/dist/utils/errors";

As per coding guidelines: "Prefer ?? throwErr(...) over non-null assertions and fallback values, with good error messages explicitly stating assumptions."

apps/backend/src/app/api/latest/session-recordings/batch/route.tsx (2)

20-37: Add a comment explaining the as any cast.

Line 27 casts to any to access .timestamp on an unknown element. The subsequent type check (typeof ts !== "number") makes it safe, but per coding guidelines a comment explaining why any is necessary here would be helpful.

Suggested fix

-    const ts = (e as any).timestamp;
+    // `any` is needed because events are untyped rrweb payloads; the next line validates that ts is a finite number.
+    const ts = (e as any).timestamp;  // eslint-disable-line `@typescript-eslint/no-unsafe-member-access`

As per coding guidelines: "Whenever you need to use any, leave a comment explaining why you're using it, why the type system fails, and how you can be certain that errors would still be flagged."

---

61-70: Use a typed boolean schema for deduped instead of yupMixed().

The deduped field is always true or false, but yupMixed() allows any value. This weakens response validation.

Suggested fix

 response: yupObject({
     statusCode: yupNumber().oneOf([200]).defined(),
     bodyType: yupString().oneOf(["json"]).defined(),
     body: yupObject({
       session_recording_id: yupString().defined(),
       batch_id: yupString().defined(),
       s3_key: yupString().defined(),
-      deduped: yupMixed().defined(),
+      deduped: yupMixed<boolean>().defined(),
     }).defined(),
 }),

Or better, if a yupBoolean helper is available in the schema-fields:

-      deduped: yupMixed().defined(),
+      deduped: yupBoolean().defined(),

apps/backend/prisma/seed.ts (1)

1812-1829: Non-null assertion on array access.

Line 1819 uses ! on the array element access. While the length check on line 1804 guarantees a non-empty array, this still goes against the project's preference.

Suggested fix

-    const projectUserId = userIds[Math.floor(Math.random() * userIds.length)]!;
+    const projectUserId = userIds[Math.floor(Math.random() * userIds.length)] ?? throwErr("Expected non-empty userIds array");

As per coding guidelines: "Prefer ?? throwErr(...) over non-null assertions and fallback values, with good error messages explicitly stating assumptions."

docker/server/.env (1)

35-40: Static analysis: env var ordering.

The linter suggests STACK_S3_PRIVATE_BUCKET should come before STACK_S3_REGION for alphabetical consistency. The current placement logically groups it with other S3 vars which is reasonable, but reordering would satisfy the linter.

Suggested fix

 STACK_S3_ENDPOINT=
+STACK_S3_PRIVATE_BUCKET=
 STACK_S3_REGION=
 STACK_S3_ACCESS_KEY_ID=
 STACK_S3_SECRET_ACCESS_KEY=
 STACK_S3_BUCKET=
-STACK_S3_PRIVATE_BUCKET=

packages/template/src/providers/stack-provider-client.tsx (1)

27-27: Consider wrapping analytics in an error boundary.

If StackAnalyticsInternal throws during render (e.g., rrweb fails to initialize on an unsupported browser), the error will propagate up and could break the entire app since Suspense doesn't catch render errors. An error boundary around the analytics component would prevent session recording failures from taking down the host application.

apps/backend/src/app/api/latest/internal/session-recordings/[session_recording_id]/chunks/[chunk_id]/events/route.tsx (2)

63-68: Prefer unknown over any for parsed JSON.

Per coding guidelines, any should be avoided. Since you're doing runtime type checks on parsed immediately after (lines 70-81), using unknown would be more type-safe and still work with the typeof / Array.isArray checks.

Suggested fix

-  let parsed: any;
+  let parsed: unknown;
   try {
     parsed = JSON.parse(new TextDecoder().decode(unzipped));
   } catch (e) {
     throw new StackAssertionError("Failed to decode session recording chunk JSON", { cause: e });
   }

Note: you'll need to adjust the property accesses on lines 73 and 87 to use type narrowing or a cast after the object check:

   if (typeof parsed !== "object" || parsed === null) {
     throw new StackAssertionError("Decoded session recording chunk is not an object");
   }
-  if (parsed.session_recording_id !== sessionRecordingId) {
+  const parsedObj = parsed as Record<string, unknown>;
+  if (parsedObj.session_recording_id !== sessionRecordingId) {
     throw new StackAssertionError("Decoded session recording chunk session_recording_id mismatch", {
       expected: sessionRecordingId,
-      actual: parsed.session_recording_id,
+      actual: parsedObj.session_recording_id,
     });
   }
-  if (!Array.isArray(parsed.events)) {
+  if (!Array.isArray(parsedObj.events)) {
     throw new StackAssertionError("Decoded session recording chunk events is not an array");
   }

---

51-60: S3 download error handling uses untyped catch — acceptable but worth a comment.

Line 54 uses catch (e: any) to access e?.$metadata?.httpStatusCode, which is an AWS SDK convention without a public typed error class. This is a valid reason for any — consider adding a brief comment explaining why, per coding guidelines.

apps/backend/src/s3.tsx (1)

73-98: Add brief comments explaining the as any casts in readBodyToBytes.

The coding guidelines ask for a comment whenever any is used. The casts on lines 78-79 and 83-85 are justified because AWS SDK response body types vary by runtime (Node.js Readable, Web ReadableStream, SdkStreamMixin), but a short comment would help future readers understand why.

packages/template/src/components/stack-analytics.tsx (2)

196-200: JSON.stringify(event) on every rrweb event is expensive for byte estimation.

rrweb FullSnapshot events can be hundreds of KB. Stringifying every event just to measure length adds significant CPU work on the hot path of DOM mutation recording. Consider a cheaper heuristic—e.g., a rough constant per event type, or stringifying only once per N events to amortize the cost.

♻️ Suggested alternative

-          approxBytes += JSON.stringify(event).length;
-          if (events.length >= MAX_EVENTS_PER_BATCH || approxBytes >= MAX_APPROX_BYTES_PER_BATCH) {
+          // Rough byte estimate without serializing every event.
+          // FullSnapshot (type 2) and FullSnapshot-derived events are large;
+          // use a coarse multiplier for others.
+          const evtType = (event as { type?: number }).type;
+          approxBytes += evtType === 2 ? 200_000 : 600;
+          if (events.length >= MAX_EVENTS_PER_BATCH || approxBytes >= MAX_APPROX_BYTES_PER_BATCH) {

---

215-223: visibilitychange handler fires on every tab switch, not just page unload.

The onPageHide callback also triggers when the user simply switches browser tabs (visibility → hidden → visible). This causes extra flush calls on tab-in, which is harmless but slightly wasteful. Consider checking document.visibilityState === "hidden" inside the handler to only flush when the page is being hidden.

♻️ Suggested change

       const onPageHide = () => {
+        if (document.visibilityState !== "hidden") return;
         runAsynchronously(() => flush({ keepalive: true }), { noErrorLogging: true });
       };

apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/analytics/replays/page-client.tsx (4)

394-422: loadPage closes over the recordings state, leading to a stale reference on append.

On Line 408, cursor ? [...recordings, ...res.items] : res.items captures recordings from the closure at the time loadPage was created. If recordings changes between invocations (e.g., a React re-render updated the dep list), the callback gets recreated, but there's a subtle window: loadPage is a dep of the useEffect on Line 424-426 which will re-run loadPage(null), but that's guarded by hasFetchedInitialRef. The scroll-triggered calls at Line 435 use the current loadPage reference, so they're fine. However, using the functional updater form of setRecordings is safer and would let you remove recordings from the dependency array, stabilizing the callback.

♻️ Suggested change

-    const items = cursor ? [...recordings, ...res.items] : res.items;
-    setRecordings(items);
+    setRecordings(prev => cursor ? [...prev, ...res.items] : res.items);

Then remove recordings from the useCallback deps.

---

87-97: coerceRrwebEvents double-casts through unknown — consider a more descriptive comment.

Line 96's filtered as unknown as RrwebEventWithTime[] is a forced cast. The preceding validation only checks for a finite timestamp property but doesn't verify type, data, or other fields rrweb expects. This is acceptable since the data originates from rrweb recording, but a brief comment noting that the validation is intentionally minimal (trusting the source) would help.

---

721-991: ensureReplayerForTab is very large (~270 lines) — consider extracting the "finish" handler.

The "finish" event handler alone (Lines 854-982) is ~130 lines with multiple branching paths (premature finish, buffer-while-downloading, tab switch, gap-fast-forward, true finish). Extracting it into a named function (e.g., handleTabFinish) would improve readability and make individual branches easier to test.

---

225-253: RAF-driven setCurrentTime on every animation frame may cause excessive re-renders.

The Timeline component calls setCurrentTime(getCurrentTimeMs()) on every requestAnimationFrame tick (~60fps). Each call triggers a state update and re-render of the component. Consider throttling updates to ~10fps (every ~100ms) since the timeline scrubber doesn't need sub-frame visual precision, or use a ref + manual DOM update for the progress bar position.

apps/backend/src/app/api/latest/session-recordings/batch/route.tsx (3)

28-28: Unnecessary as any — TypeScript narrows sufficiently after the in check.

After the guards on lines 26–27 (typeof e !== "object", !("timestamp" in e)), TypeScript already narrows e to Record<"timestamp", unknown>, so you can access e.timestamp directly. Per coding guidelines, any casts require a justifying comment, but here the cast can simply be removed.

Suggested fix

-    const ts = (e as any).timestamp;
+    const ts = e.timestamp;

---

82-84: Body-size gate runs after the buffer is already in memory.

fullReq.bodyBuffer is populated by createSmartRouteHandler (via req.arrayBuffer()) before the handler is invoked, so a client can still force the server to allocate up to the framework's own limit before this check rejects it. This line is still useful as defense-in-depth validation, but consider also enforcing an upstream limit (e.g., reverse-proxy client_max_body_size or a streaming check) if you haven't already.

---

126-140: Consider moving the dedup check before the upsert.

The existingChunk lookup (line 143) is done after the upsert, so duplicate batches still perform an unnecessary DB write to update startedAt/lastEventAt. Since the timestamps are derived from the same events, the result is idempotent, but you can save a round-trip by checking for the existing chunk first and returning early before the upsert.

apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/analytics/replays/page-client.tsx (5)

83-93: as unknown as RrwebEventWithTime[] double-cast bypasses type safety entirely.

After filtering, the objects are only validated to have a number timestamp. The RrwebEventWithTime type likely requires additional fields (type, data, etc.). This cast silently permits structurally invalid events into downstream rrweb code that expects the full shape.

Given that these events come from an untrusted external source (chunk storage), this is a reasonable tradeoff for a "best-effort coerce" function — but it should at minimum have a comment explaining why the cast is safe (e.g., "rrweb is tolerant of extra/missing fields at runtime") so future readers don't assume full structural validation has occurred.

As per coding guidelines: "Try to avoid the any type. Whenever you need to use any, leave a comment explaining why you're using it."

---

441-442: catch (e: any) used in multiple places — prefer typed error access.

Lines 441, 657, and 956 all use catch (e: any) to access e?.message. Per coding guidelines, any should be avoided. Use a helper or inline narrowing instead:

Suggested pattern

-    } catch (e: any) {
-      setListError(e?.message ?? "Failed to load session recordings.");
+    } catch (e: unknown) {
+      setListError(e instanceof Error ? e.message : "Failed to load session recordings.");

Apply the same pattern at lines 657 and 956.

As per coding guidelines: "Try to avoid the any type."

---

1132-1136: !idx would incorrectly return "Tab" if a label index were 0.

Currently safe because tabLabelIndex values start at 1 (line 891: i + 1), but the guard if (!idx) is fragile — it conflates "not in map" (undefined) with a hypothetical 0 value. Using idx === undefined is more precise and resilient to future changes.

Suggested fix

   const getTabLabel = useCallback((tabKey: TabKey) => {
     const idx = ms.tabLabelIndex.get(tabKey);
-    if (!idx) return "Tab";
+    if (idx === undefined) return "Tab";
     return `Tab ${idx}`;
   }, [ms.tabLabelIndex]);

---

498-520: Pervasive silent catch { // ignore } blocks throughout the file.

There are 15+ empty catch blocks swallowing errors from rrweb replayer operations (pause, play, addEvent, etc.). While individual rrweb calls can throw during teardown or invalid state, silently ignoring all errors makes debugging very difficult when something actually goes wrong.

Consider at minimum using console.debug or a structured logger at a low level so failures are visible during development. Alternatively, create a small helper like safeReplayerCall(r, fn) that catches and optionally logs, reducing the boilerplate.

---

1304-1308: Remove unnecessary as any cast on gridRow.

React's CSSProperties["gridRow"] type accepts both string and number values. The cast is not needed — the string "1 / -1" is a valid grid row value that TypeScript already allows without any cast.

Proposed fix

-                              gridRow: isActive ? ("1 / -1" as any) : ((miniIndex ?? 0) + 1),
+                              gridRow: isActive ? "1 / -1" : ((miniIndex ?? 0) + 1),

apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/analytics/replays/session-replay-machine.test.ts (3)

73-81: Unused helper dispatchChain.

dispatchChain is defined but never called anywhere in this test file. Remove it or add tests that use it.

---

340-344: Avoid as any — narrow the discriminated union instead.

Lines 343, 1133-1134, 1172-1173 all use as any to access effect properties without narrowing the ReplayEffect union. Per coding guidelines, any should be avoided and needs a comment when used. You can narrow first:

const play = playEffects[0];
expect(play.type).toBe("play_replayer");
if (play.type === "play_replayer") {
  expect(play.localOffsetMs).toBe(2000);
}

Or define a small test helper:

function expectEffect<T extends ReplayEffect["type"]>(
  effects: ReplayEffect[],
  type: T,
): Extract<ReplayEffect, { type: T }> {
  const e = effects.find(e => e.type === type);
  expect(e).toBeDefined();
  return e as Extract<ReplayEffect, { type: T }>;
}

This applies to the same pattern at lines 1133-1134 and 1172-1173.

As per coding guidelines: "Try to avoid the any type. Whenever you need to use any, leave a comment explaining why you're using it."

---

1372-1385: Vacuous effect-tabKey validation in the fuzz test.

The inner if (tabKeys.size > 0) block contains only comments — it never actually asserts anything. Either implement the check or remove the dead code to avoid giving a false sense of coverage.

Suggested implementation

-          if (tabKeys.size > 0) {
-            // Note: effects reference tabs from pre-transition state which
-            // might not be in post-transition state after SELECT_RECORDING.
-            // This is ok — the effect executor checks generation.
-          }
+          // Effects may reference tabs from the pre-transition state (e.g.
+          // after SELECT_RECORDING clears streams).  Skip the check in that
+          // case since the effect executor validates the generation.

apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/analytics/replays/session-replay-machine.ts (1)

206-265: Duplicated binary search across findBestTabAtGlobalOffset and isTabInRangeAtGlobalOffset.

Lines 216-229 and 251-264 contain identical binary-search-over-chunk-ranges logic. Consider extracting a shared helper:

function isInChunkRanges(ranges: ChunkRange[], ts: number): boolean {
  let lo = 0;
  let hi = ranges.length - 1;
  while (lo <= hi) {
    const mid = (lo + hi) >> 1;
    const r = ranges[mid] ?? throwErr("binary search OOB");
    if (ts < r.startTs) hi = mid - 1;
    else if (ts > r.endTs) lo = mid + 1;
    else return true;
  }
  return false;
}

This also addresses the non-null assertions (ranges[mid]!) on lines 220 and 255 — the helper can use ?? throwErr(...) in one place. As per coding guidelines: "Prefer ?? throwErr(...) over non-null assertions."

packages/stack-shared/src/interface/crud/session-recordings.ts (1)

29-44: Chunk shape is duplicated and inconsistent between the two response types.

AdminListSessionRecordingChunksResponse items include browser_session_id (line 34) but AdminGetSessionRecordingAllEventsResponse.chunks (lines 51-59) omits it. If this is intentional, consider extracting the common fields into a shared base type to reduce duplication and make the difference explicit. If unintentional, add browser_session_id to the all-events response as well.

♻️ Proposed refactor: extract shared chunk type

+type SessionRecordingChunkBase = {
+  id: string,
+  batch_id: string,
+  tab_id: string | null,
+  event_count: number,
+  byte_length: number,
+  first_event_at_millis: number,
+  last_event_at_millis: number,
+  created_at_millis: number,
+};
+
 export type AdminListSessionRecordingChunksResponse = {
-  items: Array<{
-    id: string,
-    batch_id: string,
-    tab_id: string | null,
-    browser_session_id: string | null,
-    event_count: number,
-    byte_length: number,
-    first_event_at_millis: number,
-    last_event_at_millis: number,
-    created_at_millis: number,
-  }>,
+  items: Array<SessionRecordingChunkBase & {
+    browser_session_id: string | null,
+  }>,
   pagination: {
     next_cursor: string | null,
   },
 };

Also applies to: 50-65

apps/backend/src/app/api/latest/internal/session-recordings/[session_recording_id]/events/route.tsx (3)

81-84: Invalid offset/limit values silently fall back to defaults instead of failing.

Per coding guidelines, the code should fail early and loud on invalid input. Currently, a non-numeric or negative offset silently becomes 0, and an invalid limit silently becomes chunks.length. This can mask client bugs.

Proposed fix

-    const parsedOffset = query.offset != null ? Number.parseInt(query.offset, 10) : 0;
-    const offset = Number.isFinite(parsedOffset) && parsedOffset >= 0 ? parsedOffset : 0;
-    const parsedLimit = query.limit != null ? Number.parseInt(query.limit, 10) : chunks.length;
-    const limit = Number.isFinite(parsedLimit) && parsedLimit >= 1 ? parsedLimit : chunks.length;
+    const offset = query.offset != null ? Number.parseInt(query.offset, 10) : 0;
+    if (!Number.isFinite(offset) || offset < 0) {
+      throw new StackAssertionError("Invalid offset parameter", { offset: query.offset });
+    }
+    const limit = query.limit != null ? Number.parseInt(query.limit, 10) : chunks.length;
+    if (!Number.isFinite(limit) || limit < 1) {
+      throw new StackAssertionError("Invalid limit parameter", { limit: query.limit });
+    }

As per coding guidelines, "Fail early, fail loud. Fail fast with an error instead of silently continuing."

---

89-89: Multiple uses of any without explanatory comments.

The coding guidelines require a comment explaining why any is used, why the type system fails, and how errors would still be caught. This applies to:

• Line 89: any[] in chunkEvents type
• Line 100: catch (e: any)
• Line 109: let parsed: any
• Line 129: parsed.events as any[]

At minimum, add a brief comment at each site (e.g., "S3 payload shape is validated at runtime below" for the parsed data, and "AWS SDK error type is untyped" for the catch).

As per coding guidelines, "Try to avoid the any type. Whenever you need to use any, leave a comment explaining why you're using it, why the type system fails, and how you can be certain that errors would still be flagged."

Also applies to: 100-100, 109-109, 129-129

---

90-94: Shared mutable index in concurrent workers is correct but fragile.

The nextIndex++ pattern works because JS is single-threaded and no await sits between the while condition check and the index capture. However, this is a subtle invariant — any future refactor inserting an await before nextIndex++ would introduce a race.

Consider a safer pull-from-queue approach:

Suggested alternative

-    const chunkEvents: Array<{ chunk_id: string, events: any[] }> = new Array(chunksToDownload.length);
-    let nextIndex = 0;
-
-    async function worker() {
-      while (nextIndex < chunksToDownload.length) {
-        const idx = nextIndex++;
-        const chunk = chunksToDownload[idx];
+    const chunkEvents: Array<{ chunk_id: string, events: any[] }> = new Array(chunksToDownload.length);
+    const indexQueue = chunksToDownload.map((_, i) => i);
+
+    async function worker() {
+      while (indexQueue.length > 0) {
+        const idx = indexQueue.shift()!;
+        const chunk = chunksToDownload[idx];

apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/analytics/replays/page-client.tsx (4)

82-92: Add justification comments for the as casts, especially the double-cast on line 91.

The runtime checks on lines 84–88 validate shape incrementally, so the intermediate casts are defensible. However, line 91's as unknown as RrwebEventWithTime[] is a full type-system bypass — per guidelines, every as/any should include a comment explaining why the cast is needed, why the type system fails, and how you're sure errors would still surface (e.g. "rrweb tolerates extra fields; we validated timestamp which is the only field required for replay ordering").

---

383-384: Use catch (e: unknown) instead of catch (e: any).

Per coding guidelines, avoid the any type. Use unknown and narrow:

Proposed fix

-    } catch (e: any) {
-      setListError(e?.message ?? "Failed to load session recordings.");
+    } catch (e: unknown) {
+      setListError(e instanceof Error ? e.message : "Failed to load session recordings.");

The same pattern appears at lines 599 and 929 — apply the same fix there.

---

609-733: Pervasive silent catch { // ignore } blocks mask potential failures.

There are 10+ silent catch blocks in executeEffects alone (and more elsewhere). While rrweb player calls can fail during teardown, silently swallowing all errors violates the "never silently swallow errors" guideline and makes debugging near-impossible. Consider at minimum a console.debug or structured log in development so failures are observable:

Example for one block (apply pattern throughout)

           try {
             r.play(effect.localOffsetMs);
-          } catch {
-            // ignore
+          } catch (e) {
+            console.debug("[session-replay] play failed for tab", effect.tabKey, e);
           }

---

799-801: Use rrweb's EventType enum instead of magic number 2 with as any.

rrweb exports EventType.FullSnapshot (value 2). Importing and using it avoids both the as any cast and the magic number:

Proposed fix

+// At top of file, add to rrweb type imports:
+type RrwebEventType = typeof import("rrweb").EventType;
+// Or import dynamically alongside Replayer

 const hasFullSnapshot = !msRef.current.hasFullSnapshotByTab.has(tabKey)
-  // eslint-disable-next-line `@typescript-eslint/no-unsafe-member-access`
-  && events.some(e => (e as any).type === 2);
+  && events.some(e => "type" in e && e.type === 2); // EventType.FullSnapshot

At minimum, add a comment explaining the magic number. Based on learnings: "Do NOT use as/any/type casts to bypass the type system unless you specifically asked the user about it."

apps/e2e/tests/backend/endpoints/api/v1/session-recordings.test.ts (2)

27-46: Consider using .toMatchInlineSnapshot for response assertions.

The coding guidelines prefer .toMatchInlineSnapshot over other selectors where possible. Throughout this file, tests use broad range checks like toBeGreaterThanOrEqual(400) / toBeLessThan(500) and toMatchObject. Inline snapshots would capture the exact response shape and status, making regressions easier to detect. This applies to most tests in this file (e.g., lines 44–45, 73–79, 96–101, 121–122, etc.).

As per coding guidelines: "When writing tests, prefer .toMatchInlineSnapshot over other selectors, if possible."

---

312-315: Use encodeURIComponent() for path parameters in URL template literals.

Several URLs in this file interpolate recordingId and chunkId directly into paths without encoding (lines 312, 323, 434, 447, 456, 467, 503, 514). While UUIDs are URL-safe, the coding guidelines require consistent encoding. Some query-string usages already correctly use encodeURIComponent (e.g., line 372).

As per coding guidelines: "Use urlString`` or encodeURIComponent() instead of normal string interpolation for URLs, for consistency."