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