Conversation
…uth/stack-auth into analytics-event-tracking
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds server- and client-side session-replay filtering: new query parameters (user/team, duration, last-event window, click_count_min), ClickHouse-backed click-count prefiltering, composite cursor pagination, embedded project_user in rows, dashboard filter UI with a user-search picker, and extensive E2E tests. Changes
Sequence DiagramsequenceDiagram
participant User as Dashboard User
participant Client as Page Client
participant API as Backend API Route
participant DB as PostgreSQL
participant CH as ClickHouse
User->>Client: Open filters / Apply filters
Client->>Client: Build appliedFilters state
Client->>API: GET /internal/session-replays?user_ids...&click_count_min...&cursor...
rect rgba(100,150,200,0.5)
API->>API: Parse & validate query params (durations, dates, non-negatives)
alt click_count_min supplied
API->>CH: loadClickQualifiedReplayIds(click_count_min)
CH-->>API: [qualified_replay_ids]
API->>API: Short-circuit to empty if none
end
end
rect rgba(150,200,100,0.5)
API->>DB: Query session_replays with tenancy, user/team filters, lastEventAt window, duration bounds, qualified_replay_ids, join project_user + email
DB-->>API: [rows with embedded project_user + timestamps + counts]
end
rect rgba(200,100,150,0.5)
API->>API: Compute chunk/event counts, map rows to items, build composite nextCursor
end
API-->>Client: { items: [...], nextCursor }
Client->>Client: Update UI, show filter chips/badge
Client-->>User: Render filtered replays
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Contributor
Greptile SummaryAdded comprehensive filtering capabilities to session replay listing, enabling filtering by user, team, duration, last active time, and click count. The backend implementation optimizes query performance by pushing most filters to the database while handling duration filtering in memory with overfetch. Click count filtering queries ClickHouse analytics data and passes qualifying IDs to Postgres. The UI provides a clean filter dropdown with modal dialogs for each filter type and displays active filters as removable chips. Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant UI as Dashboard UI
participant API as Backend API
participant PG as Postgres
participant CH as ClickHouse
UI->>UI: User applies filters (user/team/duration/clicks)
UI->>API: GET /internal/session-replays?user_ids=...&click_count_min=...
API->>API: Parse & validate filters
alt Click count filter active
API->>CH: Query events table for replays with N+ clicks
CH-->>API: Return qualified replay IDs
end
API->>API: Build WHERE clause (user, team, time filters)
alt Duration filter active
API->>PG: Overfetch replays (5x limit)
else No duration filter
API->>PG: Fetch replays (limit + 1)
end
PG-->>API: Return session replays with user data
alt Duration filter active
API->>API: Filter by duration in memory
API->>API: Slice to requested limit
end
API->>PG: Fetch chunk aggregates for page
PG-->>API: Return chunk counts & event counts
API-->>UI: Return filtered replays with pagination
Last reviewed commit: d7cd237 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
apps/backend/src/app/api/latest/internal/session-replays/route.tsx
Outdated
Show resolved
Hide resolved
<!-- Make sure you've read the CONTRIBUTING.md guidelines: https://github.com/stack-auth/stack-auth/blob/dev/CONTRIBUTING.md -->
Base automatically changed from
analytics-anon-users
to
analytics-replays-event-markers
February 19, 2026 03:33
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (9)
apps/backend/src/app/api/latest/internal/session-replays/route.tsx (2)
13-16: Make CSV parsing use explicit empty-string checks.
Using!rawandfilter(Boolean)violates the explicit null/undefined guideline.🔧 Suggested change
- if (!raw) return []; - return raw.split(",").map(s => s.trim()).filter(Boolean); + if (raw == null || raw === "") return []; + return raw.split(",").map(s => s.trim()).filter((s) => s.length > 0);As per coding guidelines: Use explicit null/undefinedness checks instead of boolean checks, e.g.,
foo == nullinstead of!foo.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/backend/src/app/api/latest/internal/session-replays/route.tsx` around lines 13 - 16, The parseCsvIds function uses truthy checks and filter(Boolean); change to explicit null/undefined checks and explicit empty-string filtering: in parseCsvIds, replace the if (!raw) with if (raw == null) and replace .filter(Boolean) with .filter(s => s !== '') (or .filter(s => s.length > 0)) after trimming so only non-empty strings remain; keep the split and trim logic and return type the same.
126-141: Avoid truthy checks forclickCountMinandclickQualifiedIds.
Use explicit!== nullchecks to keep intent clear.🔧 Suggested change
- const clickQualifiedIds = clickCountMin && clickCountMin > 0 + const clickQualifiedIds = clickCountMin !== null && clickCountMin > 0 ? await loadClickQualifiedReplayIds({ projectId: auth.tenancy.project.id, branchId: auth.tenancy.branchId, clickCountMin, }) : null; - if (clickQualifiedIds && clickQualifiedIds.length === 0) { + if (clickQualifiedIds !== null && clickQualifiedIds.length === 0) {As per coding guidelines: Use explicit null/undefinedness checks instead of boolean checks, e.g.,
foo == nullinstead of!foo.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/backend/src/app/api/latest/internal/session-replays/route.tsx` around lines 126 - 141, The current code uses truthy checks for clickCountMin and clickQualifiedIds; change them to explicit null/undefined checks: call loadClickQualifiedReplayIds only when clickCountMin !== null && clickCountMin !== undefined && clickCountMin > 0 (or equivalently clickCountMin != null && clickCountMin > 0), and replace the if that short-circuits on clickQualifiedIds with an explicit check like clickQualifiedIds !== null && clickQualifiedIds.length === 0 (or clickQualifiedIds != null && clickQualifiedIds.length === 0); reference variables/functions: clickCountMin, clickQualifiedIds, loadClickQualifiedReplayIds, and auth.tenancy.project.id when updating these conditions.apps/e2e/tests/backend/endpoints/api/v1/session-replays.test.ts (2)
626-633: Avoid manual URL interpolation inlistReplays.
PreferurlString(if available) or aURL/URLSearchParamscomposition rather than interpolating query strings into the path.As per coding guidelines: Use
urlString``-template literals orencodeURIComponent()` instead of normal string interpolation for URLs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/e2e/tests/backend/endpoints/api/v1/session-replays.test.ts` around lines 626 - 633, The function listReplays currently builds the request URL by manually interpolating the query string into the path; change it to construct the full URL safely using a URL or urlString helper to avoid manual interpolation and ensure proper encoding: use the existing URLSearchParams (params) to build the query and then create a new URL with pathname "/api/v1/internal/session-replays" and set search to params.toString(), or use the project's urlString template helper if available, then pass that URL.toString() (or urlString result) into niceBackendFetch along with the same options; update references to listReplays and keep accessType "admin" unchanged.
635-964: Prefer inline snapshots for the new response assertions where feasible.
Inline snapshots would reduce repetitive expectations in the new filter tests.As per coding guidelines: When writing tests, prefer .toMatchInlineSnapshot over other selectors, if possible.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/e2e/tests/backend/endpoints/api/v1/session-replays.test.ts` around lines 635 - 964, Replace repetitive field-by-field assertions in the new filter tests (the it blocks titled "admin list session replays filters by user_ids", "team_ids", "duration range", "last_event_at time range", "click_count_min", and invalid filter params") with inline snapshots using Jest's toMatchInlineSnapshot; for each call-site that currently does expect(res.status).toBe(200) and multiple expect(res.body... ) checks (e.g. after listReplays, uploadBatch, uploadEventBatch and the retry loop using listReplays), replace the body assertions with a single expect(res.body).toMatchInlineSnapshot(...) or a snapshot that uses Jest asymmetric matchers (e.g. expect.any(String)/expect.any(Number)) to handle dynamic ids/timestamps (references: listReplays, uploadBatch, uploadEventBatch, the variables replayIdA/shortId/longId/userA/userB/teamId/segmentIdA), while keeping the status code asserts; capture snapshots by running the tests to generate the inline snapshots and ensure any unstable fields are normalized with asymmetric matchers in the snapshot.apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/analytics/replays/page-client.tsx (3)
1385-1386: Add a dashboard deep link for the updated page.
Please include a deep link (e.g.,http://localhost:<NEXT_PUBLIC_STACK_PORT_PREFIX>01/projects/-selector-/analytics/replays) in the PR description so reviewers can jump straight to the updated view.Based on learnings: When making changes in the dashboard, provide users with a deep link to the dashboard page that was changed, typically in the form of
http://localhost:<NEXT_PUBLIC_STACK_PORT_PREFIX>01/projects/-selector-/....🤖 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 1385 - 1386, Add a dashboard deep link to the PR description that points reviewers to the updated view for the Session Replays page; include a URL like http://localhost:<NEXT_PUBLIC_STACK_PORT_PREFIX>01/projects/-selector-/analytics/replays so reviewers can jump directly to the component rendered in apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/analytics/replays/page-client.tsx (look for PageLayout title="Session Replays" and the surrounding PanelGroup) and ensure the link uses the correct NEXT_PUBLIC_STACK_PORT_PREFIX placeholder for local testing.
501-515: Prefer aMapfor presets and make filter checks explicit.
This keeps the preset table aligned with the Map guideline and avoids truthy checks for filter values.🔧 Suggested change
- const presetMs: Record<string, number> = { "24h": 86_400_000, "7d": 604_800_000, "30d": 2_592_000_000 }; - const lastActiveFromMillis = appliedFilters.lastActivePreset && presetMs[appliedFilters.lastActivePreset] - ? Date.now() - presetMs[appliedFilters.lastActivePreset] - : undefined; + const presetMs = new Map<Exclude<ReplayFilters["lastActivePreset"], "">, number>([ + ["24h", 86_400_000], + ["7d", 604_800_000], + ["30d", 2_592_000_000], + ]); + const presetValue = appliedFilters.lastActivePreset === "" + ? undefined + : presetMs.get(appliedFilters.lastActivePreset); + const lastActiveFromMillis = presetValue == null ? undefined : Date.now() - presetValue; ... - userIds: appliedFilters.userId ? [appliedFilters.userId] : undefined, - teamIds: appliedFilters.teamId ? [appliedFilters.teamId] : undefined, - durationMsMin: appliedFilters.durationMinSeconds ? Number(appliedFilters.durationMinSeconds) * 1000 : undefined, - durationMsMax: appliedFilters.durationMaxSeconds ? Number(appliedFilters.durationMaxSeconds) * 1000 : undefined, - clickCountMin: appliedFilters.clickCountMin ? Number(appliedFilters.clickCountMin) : undefined, + userIds: appliedFilters.userId === "" ? undefined : [appliedFilters.userId], + teamIds: appliedFilters.teamId === "" ? undefined : [appliedFilters.teamId], + durationMsMin: appliedFilters.durationMinSeconds === "" ? undefined : Number(appliedFilters.durationMinSeconds) * 1000, + durationMsMax: appliedFilters.durationMaxSeconds === "" ? undefined : Number(appliedFilters.durationMaxSeconds) * 1000, + clickCountMin: appliedFilters.clickCountMin === "" ? undefined : Number(appliedFilters.clickCountMin),As per coding guidelines: Use ES6 Maps instead of records/objects wherever possible; use explicit null/undefinedness checks instead of boolean checks, e.g.,
foo == nullinstead 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 501 - 515, Replace the preset Record with an ES6 Map (e.g., presetMs Map) and compute lastActiveFromMillis by explicitly checking appliedFilters.lastActivePreset against null/undefined and using presetMs.get(appliedFilters.lastActivePreset) instead of truthy lookup; also change the filter boolean checks passed to adminApp.listSessionReplays to explicit null/undefined checks (e.g., appliedFilters.userId != null, appliedFilters.teamId != null, appliedFilters.durationMinSeconds != null, appliedFilters.durationMaxSeconds != null, appliedFilters.clickCountMin != null) so you only convert values to arrays/numbers when they are not null/undefined before calling listSessionReplays.
151-158: Use explicit empty-string checks infiltersActiveCount.
Make the empty-string handling explicit instead of relying on truthiness.🔧 Suggested change
- if (filters.userId) count += 1; - if (filters.teamId) count += 1; - if (filters.durationMinSeconds || filters.durationMaxSeconds) count += 1; - if (filters.lastActivePreset) count += 1; - if (filters.clickCountMin) count += 1; + if (filters.userId !== "") count += 1; + if (filters.teamId !== "") count += 1; + if (filters.durationMinSeconds !== "" || filters.durationMaxSeconds !== "") count += 1; + if (filters.lastActivePreset !== "") count += 1; + if (filters.clickCountMin !== "") count += 1;As per coding guidelines: Use explicit null/undefinedness checks instead of boolean checks, e.g.,
foo == nullinstead 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 151 - 158, The function filtersActiveCount relies on truthiness which misclassifies empty strings and zero; update it to use explicit null/undefined and empty-string checks: replace if (filters.userId) and if (filters.teamId) and if (filters.lastActivePreset) with checks like filters.userId != null && filters.userId !== '' (same for teamId/lastActivePreset), replace the duration check with (filters.durationMinSeconds != null || filters.durationMaxSeconds != null), and replace if (filters.clickCountMin) with filters.clickCountMin != null so zero values are counted correctly (refer to function filtersActiveCount and the properties filters.userId, filters.teamId, filters.durationMinSeconds, filters.durationMaxSeconds, filters.lastActivePreset, filters.clickCountMin).packages/stack-shared/src/interface/admin-interface.ts (1)
790-792: Use explicit null/undefined checks for optional ID arrays.
Boolean checks can blur intent; make the null/undefined handling explicit.🔧 Suggested change
- if (params?.user_ids && params.user_ids.length > 0) qs.set("user_ids", params.user_ids.join(",")); - if (params?.team_ids && params.team_ids.length > 0) qs.set("team_ids", params.team_ids.join(",")); + if (params?.user_ids != null && params.user_ids.length > 0) qs.set("user_ids", params.user_ids.join(",")); + if (params?.team_ids != null && params.team_ids.length > 0) qs.set("team_ids", params.team_ids.join(","));As per coding guidelines: Use explicit null/undefinedness checks instead of boolean checks, e.g.,
foo == nullinstead of!foo.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stack-shared/src/interface/admin-interface.ts` around lines 790 - 792, Replace the boolean presence checks for the optional ID arrays with explicit null/undefined checks: change the conditionals that reference params.user_ids and params.team_ids to use explicit checks like params.user_ids != null && params.user_ids.length > 0 (or params.user_ids == null for negation) before calling qs.set, leaving the duration_ms_min typeof check as-is; update the conditionals surrounding qs.set("user_ids", ...) and qs.set("team_ids", ...) accordingly.apps/dashboard/src/components/data-table/user-search-picker.tsx (1)
17-25: Use explicit empty-string checks for query fallback.
Relying on|| undefineduses a boolean check; make the empty-string handling explicit.🔧 Suggested change
- const [filters, setFilters] = useState<Parameters<typeof stackAdminApp.listUsers>[0]>({ - limit: PAGE_SIZE, - query: props.query || undefined, - }); + const [filters, setFilters] = useState<Parameters<typeof stackAdminApp.listUsers>[0]>({ + limit: PAGE_SIZE, + query: props.query === "" ? undefined : props.query, + }); ... - setFilters({ limit: PAGE_SIZE, query: props.query || undefined }); + setFilters({ limit: PAGE_SIZE, query: props.query === "" ? undefined : props.query }); ... - query: props.query || undefined, + query: props.query === "" ? undefined : props.query,As per coding guidelines: Use explicit null/undefinedness checks instead of boolean checks, e.g.,
foo == nullinstead of!foo.Also applies to: 56-60
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/components/data-table/user-search-picker.tsx` around lines 17 - 25, The current initialization and reset of filters uses the boolean fallback "props.query || undefined" which treats empty string like undefined; change these to explicit empty-string checks so empty query strings are preserved or converted deliberately—for example, when creating the initial state passed to useState (the filters variable typed from stackAdminApp.listUsers) and in the useEffect that calls setFilters, replace "props.query || undefined" with an explicit check such as "props.query === '' ? undefined : props.query" (apply the same change to the second occurrence around lines 56–60) so the logic is clear and doesn't rely on truthiness.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/dashboard/src/components/data-table/user-search-picker.tsx`:
- Around line 49-55: The onUpdate callback's parameter type currently uses `any`
for `globalFilters`; change its type to `unknown` to preserve type safety while
remaining pass-through. Update the signature in the `onUpdate` function (inside
the `useCallback`) so the options object declares `globalFilters: unknown`
instead of `any`, leaving `cursor`, `limit`, `sorting: SortingState`, and
`columnFilters: ColumnFiltersState` unchanged.
---
Nitpick comments:
In `@apps/backend/src/app/api/latest/internal/session-replays/route.tsx`:
- Around line 13-16: The parseCsvIds function uses truthy checks and
filter(Boolean); change to explicit null/undefined checks and explicit
empty-string filtering: in parseCsvIds, replace the if (!raw) with if (raw ==
null) and replace .filter(Boolean) with .filter(s => s !== '') (or .filter(s =>
s.length > 0)) after trimming so only non-empty strings remain; keep the split
and trim logic and return type the same.
- Around line 126-141: The current code uses truthy checks for clickCountMin and
clickQualifiedIds; change them to explicit null/undefined checks: call
loadClickQualifiedReplayIds only when clickCountMin !== null && clickCountMin
!== undefined && clickCountMin > 0 (or equivalently clickCountMin != null &&
clickCountMin > 0), and replace the if that short-circuits on clickQualifiedIds
with an explicit check like clickQualifiedIds !== null &&
clickQualifiedIds.length === 0 (or clickQualifiedIds != null &&
clickQualifiedIds.length === 0); reference variables/functions: clickCountMin,
clickQualifiedIds, loadClickQualifiedReplayIds, and auth.tenancy.project.id when
updating these conditions.
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/analytics/replays/page-client.tsx:
- Around line 1385-1386: Add a dashboard deep link to the PR description that
points reviewers to the updated view for the Session Replays page; include a URL
like
http://localhost:<NEXT_PUBLIC_STACK_PORT_PREFIX>01/projects/-selector-/analytics/replays
so reviewers can jump directly to the component rendered in
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/analytics/replays/page-client.tsx
(look for PageLayout title="Session Replays" and the surrounding PanelGroup) and
ensure the link uses the correct NEXT_PUBLIC_STACK_PORT_PREFIX placeholder for
local testing.
- Around line 501-515: Replace the preset Record with an ES6 Map (e.g., presetMs
Map) and compute lastActiveFromMillis by explicitly checking
appliedFilters.lastActivePreset against null/undefined and using
presetMs.get(appliedFilters.lastActivePreset) instead of truthy lookup; also
change the filter boolean checks passed to adminApp.listSessionReplays to
explicit null/undefined checks (e.g., appliedFilters.userId != null,
appliedFilters.teamId != null, appliedFilters.durationMinSeconds != null,
appliedFilters.durationMaxSeconds != null, appliedFilters.clickCountMin != null)
so you only convert values to arrays/numbers when they are not null/undefined
before calling listSessionReplays.
- Around line 151-158: The function filtersActiveCount relies on truthiness
which misclassifies empty strings and zero; update it to use explicit
null/undefined and empty-string checks: replace if (filters.userId) and if
(filters.teamId) and if (filters.lastActivePreset) with checks like
filters.userId != null && filters.userId !== '' (same for
teamId/lastActivePreset), replace the duration check with
(filters.durationMinSeconds != null || filters.durationMaxSeconds != null), and
replace if (filters.clickCountMin) with filters.clickCountMin != null so zero
values are counted correctly (refer to function filtersActiveCount and the
properties filters.userId, filters.teamId, filters.durationMinSeconds,
filters.durationMaxSeconds, filters.lastActivePreset, filters.clickCountMin).
In `@apps/dashboard/src/components/data-table/user-search-picker.tsx`:
- Around line 17-25: The current initialization and reset of filters uses the
boolean fallback "props.query || undefined" which treats empty string like
undefined; change these to explicit empty-string checks so empty query strings
are preserved or converted deliberately—for example, when creating the initial
state passed to useState (the filters variable typed from
stackAdminApp.listUsers) and in the useEffect that calls setFilters, replace
"props.query || undefined" with an explicit check such as "props.query === '' ?
undefined : props.query" (apply the same change to the second occurrence around
lines 56–60) so the logic is clear and doesn't rely on truthiness.
In `@apps/e2e/tests/backend/endpoints/api/v1/session-replays.test.ts`:
- Around line 626-633: The function listReplays currently builds the request URL
by manually interpolating the query string into the path; change it to construct
the full URL safely using a URL or urlString helper to avoid manual
interpolation and ensure proper encoding: use the existing URLSearchParams
(params) to build the query and then create a new URL with pathname
"/api/v1/internal/session-replays" and set search to params.toString(), or use
the project's urlString template helper if available, then pass that
URL.toString() (or urlString result) into niceBackendFetch along with the same
options; update references to listReplays and keep accessType "admin" unchanged.
- Around line 635-964: Replace repetitive field-by-field assertions in the new
filter tests (the it blocks titled "admin list session replays filters by
user_ids", "team_ids", "duration range", "last_event_at time range",
"click_count_min", and invalid filter params") with inline snapshots using
Jest's toMatchInlineSnapshot; for each call-site that currently does
expect(res.status).toBe(200) and multiple expect(res.body... ) checks (e.g.
after listReplays, uploadBatch, uploadEventBatch and the retry loop using
listReplays), replace the body assertions with a single
expect(res.body).toMatchInlineSnapshot(...) or a snapshot that uses Jest
asymmetric matchers (e.g. expect.any(String)/expect.any(Number)) to handle
dynamic ids/timestamps (references: listReplays, uploadBatch, uploadEventBatch,
the variables replayIdA/shortId/longId/userA/userB/teamId/segmentIdA), while
keeping the status code asserts; capture snapshots by running the tests to
generate the inline snapshots and ensure any unstable fields are normalized with
asymmetric matchers in the snapshot.
In `@packages/stack-shared/src/interface/admin-interface.ts`:
- Around line 790-792: Replace the boolean presence checks for the optional ID
arrays with explicit null/undefined checks: change the conditionals that
reference params.user_ids and params.team_ids to use explicit checks like
params.user_ids != null && params.user_ids.length > 0 (or params.user_ids ==
null for negation) before calling qs.set, leaving the duration_ms_min typeof
check as-is; update the conditionals surrounding qs.set("user_ids", ...) and
qs.set("team_ids", ...) accordingly.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
apps/backend/src/app/api/latest/internal/session-replays/route.tsx (1)
13-34: Several boolean checks on nullable values violate the explicit-null-check guidelineThe three helper functions each use
!rawto guard anundefinedparameter:// parseCsvIds, parseNonNegativeInt, parseMillis if (!raw) return []; // and null variants
rawis typedstring | undefined, so the guard should beraw == null. The!rawform would also suppress an empty string"", but forparseNonNegativeIntthat actually matters:Number("") === 0, which is a valid non-negative integer — silently returningnullfor?duration_ms_min=hides what is arguably a client error. Use the explicit form:- if (!raw) return []; + if (raw == null) return [];- if (!raw) return null; + if (raw == null) return null;The same pattern recurs for
Date | nullandnumber | nullvalues inside the handler at lines 126, 131, 139, and 151–155 (lastEventAtFrom && lastEventAtTo,clickCountMin && clickCountMin > 0,clickQualifiedIds && ...,lastEventAtFrom || lastEventAtTo). All should use!= null/!== nullcomparisons. As per coding guidelines: "Use explicit null/undefinedness checks instead of boolean checks, e.g.,foo == nullinstead of!foo."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/backend/src/app/api/latest/internal/session-replays/route.tsx` around lines 13 - 34, Change all boolean falsy guards on possibly empty strings/nullable values to explicit null/undefined checks: in parseCsvIds, parseNonNegativeInt, and parseMillis replace the if (!raw) guards with if (raw == null) (or raw === undefined/null) so empty string "" is not treated as absent; similarly, in the request handler replace all truthy checks like lastEventAtFrom && lastEventAtTo, clickCountMin && clickCountMin > 0, clickQualifiedIds && ..., and lastEventAtFrom || lastEventAtTo with explicit null/undefined comparisons (e.g., lastEventAtFrom != null && lastEventAtTo != null, clickCountMin != null && clickCountMin > 0, clickQualifiedIds != null, lastEventAtFrom != null || lastEventAtTo != null) to preserve empty-string/zero semantics and follow the explicit-null-check guideline.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/backend/src/app/api/latest/internal/session-replays/route.tsx`:
- Around line 36-66: The ClickHouse query in loadClickQualifiedReplayIds is
unbounded and can return very large ID lists which yield memory spikes and
oversized Prisma IN(...) clauses; add a pragmatic cap to the ClickHouse query
(e.g., LIMIT 10000) and detect when the cap is hit (compare rows.length to the
cap) so the calling API can return a 400/validation error or a descriptive "too
many qualifying replays — refine your filter" response instead of passing the
full list to Prisma (see where clickQualifiedIds is used as id: { in:
clickQualifiedIds }). Optionally document the cap or make it configurable via a
parameter to loadClickQualifiedReplayIds so callers can adjust behavior.
- Around line 230-232: The pagination logic incorrectly returns next_cursor null
when page is empty but the DB fetch was exhausted; update the nextCursor
computation in the route so that if hasDurationFilter is true and page.length
=== 0 and sessions.length === fetchSize, nextCursor falls back to the last DB
row id (sessions[sessions.length - 1].id) instead of null; keep existing hasMore
calculation but change the ternary for nextCursor to check for page.length > 0
first, otherwise if hasDurationFilter && sessions.length === fetchSize return
the last session id, else null.
- Line 64: Replace the prohibited TypeScript "as" cast on the JSON result:
instead of using "result.json() as Array<{ session_replay_id: string }>", call
the ClickHouse ResultSet generic form result.json<Array<{ session_replay_id:
string }>>() to get typed rows; update the variable assignment for rows
accordingly (and optionally add zod validation if you need runtime schema
checks).
---
Nitpick comments:
In `@apps/backend/src/app/api/latest/internal/session-replays/route.tsx`:
- Around line 13-34: Change all boolean falsy guards on possibly empty
strings/nullable values to explicit null/undefined checks: in parseCsvIds,
parseNonNegativeInt, and parseMillis replace the if (!raw) guards with if (raw
== null) (or raw === undefined/null) so empty string "" is not treated as
absent; similarly, in the request handler replace all truthy checks like
lastEventAtFrom && lastEventAtTo, clickCountMin && clickCountMin > 0,
clickQualifiedIds && ..., and lastEventAtFrom || lastEventAtTo with explicit
null/undefined comparisons (e.g., lastEventAtFrom != null && lastEventAtTo !=
null, clickCountMin != null && clickCountMin > 0, clickQualifiedIds != null,
lastEventAtFrom != null || lastEventAtTo != null) to preserve empty-string/zero
semantics and follow the explicit-null-check guideline.
apps/backend/src/app/api/latest/internal/session-replays/route.tsx
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/backend/src/app/api/latest/internal/session-replays/route.tsx (2)
215-215: Replace non-null assertion with?? throwErr(...)
limitis clamped to≥ 1andpageis only used here whenhasMoreis true (meaningrows.length > limit ≥ 1), so the assertion is logically safe — but the project guideline explicitly prefers?? throwErr(...)with a descriptive message over!.- const nextCursor = hasMore ? page[page.length - 1]!.id : null; + const nextCursor = hasMore + ? (page[page.length - 1] ?? throwErr("Expected page to be non-empty when hasMore is true")).id + : null;As per coding guidelines: "Prefer
?? throwErr(...)over non-null assertions with good error messages explicitly stating the assumption."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/backend/src/app/api/latest/internal/session-replays/route.tsx` at line 215, The line computing nextCursor uses a non-null assertion on page[page.length - 1] (const nextCursor = hasMore ? page[page.length - 1]!.id : null;); replace the `!` with a null-coalescing guard that calls throwErr(...) so we fail with a clear message if the assumption is broken (e.g., use page[page.length - 1]?.id ?? throwErr("expected last page item when hasMore is true in nextCursor")). Update the expression that sets nextCursor (referencing nextCursor, page and hasMore) to use this pattern and include a descriptive error message about the invariant.
129-129: Prefer explicit!== nullchecks over boolean coercion on nullable valuesThree separate spots use implicit boolean truthiness on
Date | nullornumber | nullvalues, against the project's explicit-null-check guideline:- if (lastEventAtFrom && lastEventAtTo && lastEventAtFrom.getTime() > lastEventAtTo.getTime()) { + if (lastEventAtFrom !== null && lastEventAtTo !== null && lastEventAtFrom.getTime() > lastEventAtTo.getTime()) {- const clickQualifiedIds = clickCountMin && clickCountMin > 0 + const clickQualifiedIds = clickCountMin !== null && clickCountMin > 0- if (clickQualifiedIds && clickQualifiedIds.length === 0) { + if (clickQualifiedIds !== null && clickQualifiedIds.length === 0) {As per coding guidelines: "Unless very clearly equivalent from types, prefer explicit null/undefinedness checks over boolean checks, e.g.,
foo == nullinstead of!foo."Also applies to: 134-134, 142-142
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/backend/src/app/api/latest/internal/session-replays/route.tsx` at line 129, The conditional checks currently rely on boolean coercion for nullable values (e.g., lastEventAtFrom and lastEventAtTo); update those conditions to use explicit null checks instead of truthiness. Replace expressions like "if (lastEventAtFrom && lastEventAtTo && ...)" with explicit checks such as "lastEventAtFrom !== null" and "lastEventAtTo !== null" (and similarly for the other two occurrences flagged around lines 134 and 142) so every nullable Date or number is compared explicitly to null (and to undefined where applicable) before using .getTime() or other operations.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/backend/src/app/api/latest/internal/session-replays/route.tsx`:
- Line 66: Replace the post-fetch type assertion with the fetch response
generic: instead of casting the result of result.json() to Array<{
session_replay_id: string }>, call result.json<Array<{ session_replay_id: string
}>>() so the type is inferred by the json<T>() overload; update the declaration
of rows (currently "const rows = await result.json() as Array<{
session_replay_id: string }>;") to await result.json<Array<{ session_replay_id:
string }>>() to remove the `as` cast and keep typesafe typing for the rows
variable.
- Around line 50-57: The query applies CLICK_FILTER_ID_CAP but doesn't indicate
when results were truncated; in the route handler that executes the query and
prepares the response (look for CLICK_FILTER_ID_CAP, rows, and the exported
GET/route function in route.tsx) detect if rows.length === CLICK_FILTER_ID_CAP
and surface that to the caller rather than silently truncating: either return a
400 with a clear message referencing the cap, or (preferred) add a field to the
JSON response such as capExceeded: true (and optionally include cap:
CLICK_FILTER_ID_CAP) so callers can detect overflow; update any response
Type/interface accordingly.
---
Nitpick comments:
In `@apps/backend/src/app/api/latest/internal/session-replays/route.tsx`:
- Line 215: The line computing nextCursor uses a non-null assertion on
page[page.length - 1] (const nextCursor = hasMore ? page[page.length - 1]!.id :
null;); replace the `!` with a null-coalescing guard that calls throwErr(...) so
we fail with a clear message if the assumption is broken (e.g., use
page[page.length - 1]?.id ?? throwErr("expected last page item when hasMore is
true in nextCursor")). Update the expression that sets nextCursor (referencing
nextCursor, page and hasMore) to use this pattern and include a descriptive
error message about the invariant.
- Line 129: The conditional checks currently rely on boolean coercion for
nullable values (e.g., lastEventAtFrom and lastEventAtTo); update those
conditions to use explicit null checks instead of truthiness. Replace
expressions like "if (lastEventAtFrom && lastEventAtTo && ...)" with explicit
checks such as "lastEventAtFrom !== null" and "lastEventAtTo !== null" (and
similarly for the other two occurrences flagged around lines 134 and 142) so
every nullable Date or number is compared explicitly to null (and to undefined
where applicable) before using .getTime() or other operations.
…uth/stack-auth into analytics-replay-filters
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/analytics/replays/page-client.tsx (1)
1473-1512: User filter dialog bypasses draft state and applies immediately — inconsistent UX patternThe team filter dialog (lines 1514–1553) follows the same pattern, but both differ from the Duration and Clicks dialogs that use
draftFilters+ an explicit "Apply" button. Selecting a user/team immediately commitsappliedFilters, with no way to cancel before the filter is applied.This is not a correctness bug, but the inconsistency may surprise users. If immediate-apply is intentional (a picker selection is final), consider at least adding a "Cancel" escape that also clears the selection (similar to the "Clear" button already present), or document the design intent with a comment.
🤖 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 1473 - 1512, The User/team picker dialogs currently call setAppliedFilters directly on selection (via UserSearchPicker and the onClick that calls setAppliedFilters and setActiveFilterDialog), causing immediate commits; change this to mirror the Duration/Clicks pattern by writing selections to draftFilters instead and only commit to appliedFilters when the dialog's explicit "Apply" button is pressed (keep the existing Clear behavior to reset draftFilters), or if immediate apply is intended make that explicit with a comment and add a "Cancel" action that closes the dialog without changing appliedFilters; update references to setAppliedFilters, setActiveFilterDialog, UserSearchPicker, appliedFilters and draftFilters accordingly.apps/backend/src/app/api/latest/internal/session-replays/route.tsx (1)
137-151: Prefer explicit!= nullchecks over boolean truthiness onDateandnumber | nullLines 140 and 145 use truthy/falsy checks on values where explicit null guards are clearer per coding guidelines.
new Date(0)is truthy, so the Date check works, but it's non-obvious.clickCountMinisnumber | null;clickCountMin && clickCountMin > 0conflatesnulland0via falsy collapse.♻️ Proposed fix
- if (lastEventAtFrom && lastEventAtTo && lastEventAtFrom.getTime() > lastEventAtTo.getTime()) { + if (lastEventAtFrom != null && lastEventAtTo != null && lastEventAtFrom.getTime() > lastEventAtTo.getTime()) { - const clickQualifiedIds = clickCountMin && clickCountMin > 0 + const clickQualifiedIds = clickCountMin != null && clickCountMin > 0As per coding guidelines: "Unless very clearly equivalent from types, prefer explicit null/undefinedness checks over boolean checks, e.g.,
foo == nullinstead of!foo."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/backend/src/app/api/latest/internal/session-replays/route.tsx` around lines 137 - 151, Replace truthy/falsy checks with explicit null checks: for the date comparison use explicit guards (check lastEventAtFrom != null && lastEventAtTo != null before calling getTime()), and for duration use durationMsMin != null && durationMsMax != null; for the click filter compute clickQualifiedIds only when clickCountMin != null && clickCountMin > 0 (instead of clickCountMin && clickCountMin > 0). Update the conditions around durationMsMin/durationMsMax, lastEventAtFrom/lastEventAtTo, and the clickQualifiedIds ternary that calls loadClickQualifiedReplayIds so they all use explicit != null checks and an explicit numeric > 0 check for clickCountMin.apps/e2e/tests/backend/endpoints/api/v1/session-replays.test.ts (1)
839-1384: Good coverage of all new filter parameters and error pathsThe new filter tests are well-structured: they cover all seven new query parameters, validate AND semantics, test boundary/invalid inputs with inline snapshots, and use
Project.createAndSwitchfor isolation. The ClickHouse retry loop (lines 1116–1124) correctly accounts for eventual consistency.One minor coverage gap: no test exercises cursor pagination while a filter is simultaneously active (e.g.,
user_ids+cursor). The SQL composes both correctly, but a combined test would catch future regressions. Consider adding a short test that uploads two sessions for the same user, paginates withlimit=1anduser_ids=<that user>, and verifies both pages contain only that user's sessions.Would you like me to draft that pagination+filter combination test?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/e2e/tests/backend/endpoints/api/v1/session-replays.test.ts` around lines 839 - 1384, Add a test that verifies cursor pagination works when a filter is active: sign in a user via Auth.Otp.signIn, upload two sessions for that user with uploadBatch (use randomUUID for ids and bumpEmailAddress between users if needed), call listReplays with { user_ids: <userId>, limit: "1" } to get the first page, capture first.body.pagination.next_cursor, then call listReplays again with { user_ids: <userId>, limit: "1", cursor: next_cursor } and assert both pages return items with project_user.id equal to the signed-in user's id and the two replay ids are distinct; also assert pagination behaves (next_cursor string for first page, eventually null when no more pages).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/analytics/replays/page-client.tsx:
- Around line 152-160: filtersActiveCount currently uses truthy checks and
counts string "0" as active; align it with loadPage by treating zero as a no-op.
Update filtersActiveCount (function filtersActiveCount) to only increment for
numeric filters when Number(value) > 0 (e.g., for clickCountMin,
durationMinSeconds, durationMaxSeconds) and keep existing checks for
userId/teamId/lastActivePreset; also update loadPage to mirror this behavior by
passing clickCountMin/durationMinSeconds/durationMaxSeconds as Number(value) > 0
? Number(value) : undefined so zero is not sent to the API.
- Around line 517-524: Remove the side-effect inside the setRecordings
functional updater: do not call setSelectedRecordingId or mutate
hasAutoSelectedRef from within the setRecordings((prev) => { ... }) callback;
instead just return the new items. Also delete the redundant hasAutoSelectedRef
mutation in the filter-reset effect and then remove the hasAutoSelectedRef ref
entirely if it has no other usages, since the dedicated useEffect that handles
auto-selection already covers initial load, filter-reset, and load-more cases.
---
Duplicate comments:
In `@apps/backend/src/app/api/latest/internal/session-replays/route.tsx`:
- Line 77: Replace the prohibited TypeScript cast on result.json() by using the
generic form: change the line assigning rows to call result.json with a type
parameter (e.g., const rows = await result.json<Array<{ session_replay_id:
string }>>(); or alternatively annotate rows: const rows: Array<{
session_replay_id: string }> = await result.json();), so the JSON parse uses the
generic type instead of an "as" cast in route.tsx where rows and result.json()
are used.
- Around line 47-79: In loadClickQualifiedReplayIds detect when the ClickHouse
result was truncated by checking if rows.length === CLICK_FILTER_ID_CAP
(referencing CLICK_FILTER_ID_CAP and loadClickQualifiedReplayIds) and surface
that to the caller instead of returning a silently truncated list: when cap is
hit throw a descriptive error (or a typed error the route handler maps to a 400)
indicating the query hit the ID cap so the client can return a 400 user-facing
response; ensure the calling code that currently short-circuits on zero matches
(lines around the caller) handles this thrown error and maps it to a 400
response.
---
Nitpick comments:
In `@apps/backend/src/app/api/latest/internal/session-replays/route.tsx`:
- Around line 137-151: Replace truthy/falsy checks with explicit null checks:
for the date comparison use explicit guards (check lastEventAtFrom != null &&
lastEventAtTo != null before calling getTime()), and for duration use
durationMsMin != null && durationMsMax != null; for the click filter compute
clickQualifiedIds only when clickCountMin != null && clickCountMin > 0 (instead
of clickCountMin && clickCountMin > 0). Update the conditions around
durationMsMin/durationMsMax, lastEventAtFrom/lastEventAtTo, and the
clickQualifiedIds ternary that calls loadClickQualifiedReplayIds so they all use
explicit != null checks and an explicit numeric > 0 check for clickCountMin.
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/analytics/replays/page-client.tsx:
- Around line 1473-1512: The User/team picker dialogs currently call
setAppliedFilters directly on selection (via UserSearchPicker and the onClick
that calls setAppliedFilters and setActiveFilterDialog), causing immediate
commits; change this to mirror the Duration/Clicks pattern by writing selections
to draftFilters instead and only commit to appliedFilters when the dialog's
explicit "Apply" button is pressed (keep the existing Clear behavior to reset
draftFilters), or if immediate apply is intended make that explicit with a
comment and add a "Cancel" action that closes the dialog without changing
appliedFilters; update references to setAppliedFilters, setActiveFilterDialog,
UserSearchPicker, appliedFilters and draftFilters accordingly.
In `@apps/e2e/tests/backend/endpoints/api/v1/session-replays.test.ts`:
- Around line 839-1384: Add a test that verifies cursor pagination works when a
filter is active: sign in a user via Auth.Otp.signIn, upload two sessions for
that user with uploadBatch (use randomUUID for ids and bumpEmailAddress between
users if needed), call listReplays with { user_ids: <userId>, limit: "1" } to
get the first page, capture first.body.pagination.next_cursor, then call
listReplays again with { user_ids: <userId>, limit: "1", cursor: next_cursor }
and assert both pages return items with project_user.id equal to the signed-in
user's id and the two replay ids are distinct; also assert pagination behaves
(next_cursor string for first page, eventually null when no more pages).
.../dashboard/src/app/(main)/(protected)/projects/[projectId]/analytics/replays/page-client.tsx
Show resolved
Hide resolved
.../dashboard/src/app/(main)/(protected)/projects/[projectId]/analytics/replays/page-client.tsx
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/backend/src/app/api/latest/internal/session-replays/route.tsx (1)
140-165: Prefer explicit!= null/== nullguards over boolean coercion on nullable typesFour places use truthiness checks on
Date | null,number | null, orstring[] | nullvalues, which violates the codebase guideline:
- Line 140:
if (lastEventAtFrom && lastEventAtTo && ...)— both areDate | null- Line 145:
clickCountMin && clickCountMin > 0—number | null;clickCountMin = 0is legitimately falsy but the intent is a null-check, not a zero-check- Line 153:
if (clickQualifiedIds && ...)—string[] | null- Line 164:
if (cursorId)—string | undefined♻️ Suggested changes
- if (lastEventAtFrom && lastEventAtTo && lastEventAtFrom.getTime() > lastEventAtTo.getTime()) { + if (lastEventAtFrom != null && lastEventAtTo != null && lastEventAtFrom.getTime() > lastEventAtTo.getTime()) { - const clickQualifiedIds = clickCountMin && clickCountMin > 0 + const clickQualifiedIds = clickCountMin != null && clickCountMin > 0 - if (clickQualifiedIds && clickQualifiedIds.length === 0) { + if (clickQualifiedIds != null && clickQualifiedIds.length === 0) { - if (cursorId) { + if (cursorId != null) {As per coding guidelines: "Unless very clearly equivalent from types, prefer explicit null/undefinedness checks over boolean checks, e.g.,
foo == nullinstead of!foo."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/backend/src/app/api/latest/internal/session-replays/route.tsx` around lines 140 - 165, Summary: Replace truthy/falsy checks with explicit null/undefined guards for nullable variables to follow codebase guidelines. Fix: change the condition at the start to use explicit null checks (if (lastEventAtFrom != null && lastEventAtTo != null && lastEventAtFrom.getTime() > lastEventAtTo.getTime()) ...), change the click filter check to (clickCountMin != null && clickCountMin > 0) before calling loadClickQualifiedReplayIds, change the empty-result guard to (clickQualifiedIds != null && clickQualifiedIds.length === 0) and change the cursor existence check to (cursorId != null) before using prisma.sessionReplay.findUnique to populate cursorPivot; keep references to loadClickQualifiedReplayIds, clickQualifiedIds, clickCountMin, lastEventAtFrom, lastEventAtTo, cursorId, cursorPivot and prisma.sessionReplay.findUnique so the edits are easy to locate.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/backend/src/app/api/latest/internal/session-replays/route.tsx`:
- Around line 216-219: The Prisma SQL fragment uses the separate variable
cursorId (type string | undefined) which is not type-safe when cursorPivot is
truthy; change the interpolation to use cursorPivot.id (which is string) inside
the Prisma.sql block so the comparison sr."id" < ${cursorPivot.id} is type-safe.
Update the conditional branch that builds the Prisma.sql snippet (the block
referencing cursorPivot and sr."lastEventAt") to replace cursorId with
cursorPivot.id and ensure cursorPivot is non-null before accessing .id.
- Line 226: The current use of a non-null assertion in nextCursor
(page[page.length - 1]!.id) should be replaced with a null-safe access combined
with the codebase's throwErr pattern; when hasMore is true, compute nextCursor
using a safe lookup (e.g., use page.at(-1) or page[page.length - 1] with
optional chaining) and append "?? throwErr(...)" with a clear message like
"Expected non-empty page when hasMore is true" so that nextCursor, hasMore, page
and throwErr are referenced and the assumption is enforced without using "!".
---
Duplicate comments:
In `@apps/backend/src/app/api/latest/internal/session-replays/route.tsx`:
- Around line 47-79: The query now uses CLICK_FILTER_ID_CAP but doesn't indicate
when results were truncated; update loadClickQualifiedReplayIds to detect a
cap-hit by checking whether rows.length === CLICK_FILTER_ID_CAP after parsing
result.json() and propagate that to callers (e.g., change the return shape from
Promise<string[]> to Promise<{ ids: string[]; capped: boolean }>, or throw a
specific error) so callers of loadClickQualifiedReplayIds can detect incomplete
results; reference the function loadClickQualifiedReplayIds, the
CLICK_FILTER_ID_CAP constant, the result.json() parsing and rows.map(...) when
implementing this change.
- Line 77: Replace the prohibited "as" cast on result.json() by using the
generic form of the method: call result.json with the type parameter for Array<{
session_replay_id: string }> so the returned value assigned to rows is properly
typed (replace the usage of result.json() as Array<{ session_replay_id: string
}> and update the declaration of rows accordingly); locate this in the
session-replays route where the variable rows is assigned from result.json() and
use the ResultSet's generic json<T>() method instead.
---
Nitpick comments:
In `@apps/backend/src/app/api/latest/internal/session-replays/route.tsx`:
- Around line 140-165: Summary: Replace truthy/falsy checks with explicit
null/undefined guards for nullable variables to follow codebase guidelines. Fix:
change the condition at the start to use explicit null checks (if
(lastEventAtFrom != null && lastEventAtTo != null && lastEventAtFrom.getTime() >
lastEventAtTo.getTime()) ...), change the click filter check to (clickCountMin
!= null && clickCountMin > 0) before calling loadClickQualifiedReplayIds, change
the empty-result guard to (clickQualifiedIds != null && clickQualifiedIds.length
=== 0) and change the cursor existence check to (cursorId != null) before using
prisma.sessionReplay.findUnique to populate cursorPivot; keep references to
loadClickQualifiedReplayIds, clickQualifiedIds, clickCountMin, lastEventAtFrom,
lastEventAtTo, cursorId, cursorPivot and prisma.sessionReplay.findUnique so the
edits are easy to locate.
N2D4
approved these changes
Feb 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://www.loom.com/share/5fe96d0d675b455391a6ade1377d2fa5
Summary by CodeRabbit
New Features
Tests