Skip to content

fix(replay): apply a saved filter picked from recent - #111091

Draft
phillram wants to merge 2 commits into
masterfrom
posthog/fix-recent-saved-replay-filters
Draft

phillram wants to merge 2 commits into
masterfrom
posthog/fix-recent-saved-replay-filters

Conversation

@phillram

@phillram phillram commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Problem

Picking a saved filter from the replay filter picker's Recent category does nothing. No filter is applied, and nothing says why.

A Recent row is a summary the picker keeps in local storage: it holds the short id the row is keyed by, and not the filters themselves, which are too heavy to store. The apply path required the filters to be on the picked row, so the guard failed and the listener returned silently. The same filter picked from the Saved filters list applies fine.

Ticket 76386.

Changes

  • A saved filter picked from Recent now applies. The picker already passes the short id, and the replay logic resolves it back to the filter.
  • A short id that resolves to nothing now shows "Could not apply that saved filter. It may have been deleted." instead of failing in silence.
  • requestApplySavedFilterByShortId on sessionRecordingSavedFiltersLogic owns the resolution. It reads the loaded saved filter list first, then falls back to one fetch by short id.
  • The loaded list alone is not enough. It holds one page, narrowed further by the search and created-by filters of the saved filters panel, so a filter the picker offers can be absent from it.

The rows already rendered correctly, so the only new visual is the error toast.

How did you test this code?

Test rationale: The regression is "a Recent row resolves to nothing, so the filter is silently not applied". The nearest existing coverage, the addGroupFilter tests in universalFiltersLogic.test.ts, only exercises property and quick-filter rows and never reaches the saved-filter branch.

  • sessionRecordingSavedFiltersLogic.test.ts: three cases on the resolution, one per outcome. A filter the loaded page holds applies with no request, a filter it does not hold is fetched, and an unresolvable short id applies nothing and reports.
  • universalFiltersLogic.test.ts: two cases on the wiring from the picker, which a test of the resolution alone does not reach. A list row passes the filter straight through, and a Recent row passes its short id.

Ran locally: both suites (39 tests) and the repo-wide TypeScript check, which reports no errors. The workspace packages @posthog/quill and @posthog/hogvm need a build first, or that check reports missing modules across unrelated files.

Not run: the rest of the frontend suite and anything backend. The change is two frontend logic files.

Release status

  • No feature flag controls this change

Automatic notifications

  • Publish to changelog?

Docs update

No doc covers the picker's Recent category, so no docs change.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Agent: PostHog Desktop, Claude Opus 5 (claude-opus-5)

  • Skills invoked: /modifying-taxonomic-filter, /writing-tests, /writing-user-facing-copy, /writing-code-comments, /writing-pr-descriptions.
  • Found by tracing which taxonomic filter consumers read fields off the picked row rather than off the value the picker passes. Most read only the value and are unaffected; this one and the feature flag picker in fix(flags): make recent feature flags show and link correctly #110227 were the exceptions.
  • A review of the first commit moved the resolution into the replay logic and added the fallback fetch. The first version read the loaded list only, which left the same silent failure for a filter outside that page.
  • Independent of fix(flags): make recent feature flags show and link correctly #110227 and branched from master. It resolves from the short id the picker already passes, which predates that PR's storage change, so the two can land in either order.
  • No duplicate: gh pr list --state open --search over saved filter and replay recent terms found nothing.
  • Public artifact: no customer material reached the diff. The saved filter names in the test are invented.

Created with PostHog Desktop

A row in the picker's Recent category is a summary kept in local storage: it holds the short id the row is keyed by, and not the filters, which are too heavy to store. The apply path required the filters to be on the picked row, so the guard failed and the listener returned without applying anything or reporting why.

Resolve the picked short id against the already-loaded saved filter list instead. `resolveSavedFilter` is pure, so the recent, list, missing and no-filters cases are testable without mounting the replay scene.

Generated-By: PostHog Desktop
Task-Id: 1b898a61-0065-4b4d-b7b2-84f843040805
@phillram phillram self-assigned this Oct 2, 2026
@trunk-io

trunk-io Bot commented Oct 2, 2026

Copy link
Copy Markdown

Merging to master in this repository is managed by Trunk.

  • To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

@github-actions

github-actions Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

🤖 CI report

✅ Trunk lane — non-backend lane

This PR is assigned to the non-backend lane. It does not run backend Python tests and may merge in parallel with PRs in other lanes.

⚠️ Complexity (TypeScript) — 1 function above the limit (max 28)

Cyclomatic complexity above the limit in changed typescript files (10 for production files, 15 for test files). Warn only: worth simplifying when you next touch these functions.

Function Location Complexity Limit
addGroupFilter frontend/src/lib/components/UniversalFilters/universalFiltersLogic.ts:263 28 10
✅ Duplication (Python) — clean

New Python code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

✅ Duplication (TypeScript) — clean

New TypeScript code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

⚠️ Comment density — 4% of added code lines are comments (5 of 118)

This section warns when comments are more than 3% of the code lines a PR adds, and alerts above 6%. Before agent-assisted PRs, the typical share was about 2%. Only full-line comments count. Docstrings, generated files, snapshots, migrations, and workflow files are left out.

Comments that restate the code, record how the change came about, or narrate the next line add noise for the next reader. Keep the comments that explain a reason the code cannot show, and remove the rest. See .agents/skills/writing-code-comments/SKILL.md for the house rules.

Files with the most added comment lines:

File Comment lines Added lines
frontend/src/lib/components/UniversalFilters/universalFiltersLogic.ts 3 11
frontend/src/scenes/session-recordings/filters/sessionRecordingSavedFiltersLogic.ts 2 20

This check does not block merging. It updates on every push and clears when the share drops.

✅ Bundle size — 🟢 -5.5 KiB (-0.0%)

Uncompressed size of every built .js bundle, compared against the base branch.

Total: 69.79 MiB · 🟢 -5.5 KiB (-0.0%)

File Size Δ vs base
posthog-app/_parent/products/replay_vision/frontend/replay_scanners/components/ScannerScoutsTab.js 18.9 KiB 🟢 -8.5 KiB (-31.1%)
posthog-app/_parent/products/replay_vision/frontend/replay_scanners/ReplayScanner.js 53.7 KiB 🔺 +2.2 KiB (+4.3%)

Posted automatically by build-bundle-size-report · uncompressed bytes from dist-report

✅ Eager graph — within budget

How much code each root ships on the eager path — downloaded and parsed before the surface is interactive. Measured from the esbuild output chunks (post-tree-shake, static imports only); lazy import() / React.lazy chunks are not counted.

Root Eager (shipped) Δ vs base Budget
entry (logged-out pages, app bootstrap)
src/index.tsx
1.63 MiB · 22 files 🔺 +55 B (+0.0%) █████████░ 88.7% of 1.84 MiB
logged-out boot: index + App + bootApp (preloaded by every page, including /login)
src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
3.72 MiB · 661 files 🔺 +55 B (+0.0%) █████████░ 92.3% of 4.03 MiB
authenticated shell (every logged-in page)
src/scenes/AuthenticatedShell.tsx
7.59 MiB · 2,409 files 🔺 +550 B (+0.0%) █████████░ 91.0% of 8.34 MiB
dashboard scene
src/scenes/dashboard/Dashboard.tsx
9.67 MiB · 3,392 files 🔺 +657 B (+0.0%) ███████░░░ 71.8% of 13.48 MiB
today home path
src/scenes/AuthenticatedShell.tsx + src/scenes/project-homepage/ProjectHomepage.tsx + src/scenes/project-homepage/today/TodayHome.tsx
7.61 MiB · 2,417 files 🔺 +550 B (+0.0%) █████████░ 88.6% of 8.58 MiB
events scene
src/scenes/activity/explore/EventsScene.tsx
9.29 MiB · 3,244 files 🔺 +657 B (+0.0%) ███████░░░ 73.5% of 12.64 MiB
replay detail scene
src/scenes/session-recordings/detail/SessionRecordingDetail.tsx
12.12 MiB · 4,130 files 🔺 +673 B (+0.0%) ████████░░ 77.1% of 15.72 MiB

🟢 node_modules/monaco-editor/ stays out of src/index.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 node_modules/monaco-editor/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/layout/navigation-3000/navigationLogic.tsx stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/scenes/dashboard/dashboardLogic.tsx stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/lemon-ui/LemonMarkdown/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/components/RichContentEditor/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/components/CodeSnippet/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/taxonomy/core-filter-definitions-by-group.json stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 node_modules/monaco-editor/ stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/scenes/session-recordings/player/sessionRecordingPlayerLogic.ts stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/scenes/project-homepage/ai-first/AiFirstHomepage.tsx stays out of src/scenes/AuthenticatedShell.tsx + src/scenes/project-homepage/ProjectHomepage.tsx + src/scenes/project-homepage/today/TodayHome.tsx
🟢 src/scenes/project-homepage/today/TodayReportPage.tsx stays out of src/scenes/AuthenticatedShell.tsx + src/scenes/project-homepage/ProjectHomepage.tsx + src/scenes/project-homepage/today/TodayHome.tsx

Largest files eagerly shipped from src/index.tsx
Size File
126.8 KiB ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js
24.6 KiB ../node_modules/.pnpm/buffer@6.0.3/node_modules/buffer/index.js
6.3 KiB ../node_modules/.pnpm/react@18.3.1/node_modules/react/cjs/react.production.min.js
4.5 KiB ../node_modules/.pnpm/@jspm+core@2.1.0/node_modules/@jspm/core/nodelibs/browser/process.js
3.9 KiB ../node_modules/.pnpm/scheduler@0.23.2/node_modules/scheduler/cjs/scheduler.production.min.js
1.4 KiB ../node_modules/.pnpm/base64-js@1.5.1/node_modules/base64-js/index.js
1.3 KiB src/index.tsx
1.3 KiB src/RootErrorBoundary.tsx
912 B ../node_modules/.pnpm/ieee754@1.2.1/node_modules/ieee754/index.js
854 B src/scenes/ChunkLoadErrorBoundary.tsx
Largest files eagerly shipped from src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
Size File
306.2 KiB ../node_modules/.pnpm/posthog-js@1.435.5_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs
220.3 KiB ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js
126.8 KiB ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js
100.5 KiB src/lib/api.ts
92.7 KiB src/products.tsx
69.4 KiB src/lib/lemon-ui/icons/icons.tsx
40.1 KiB src/lib/utils/eventUsageLogic.ts
38.7 KiB ../node_modules/.pnpm/@dnd-kit+core@6.0.8_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@dnd-kit/core/dist/core.esm.js
33.9 KiB ../node_modules/.pnpm/kea@4.0.0-pre.6_patch_hash=139b8d1f1304f9d9da452a9a1244c94ea679dbcb85687d8999563146879fb6f5_react@18.3.1/node_modules/kea/lib/index.cjs.js
29.0 KiB ../node_modules/.pnpm/zod@4.3.6/node_modules/zod/v4/core/schemas.js
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
Size File
306.2 KiB ../node_modules/.pnpm/posthog-js@1.435.5_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs
279.9 KiB src/taxonomy/core-filter-definitions-by-group.json
220.3 KiB ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js
153.7 KiB ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js
126.8 KiB ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js
110.1 KiB ../packages/quill/packages/quill/dist/index.js
100.5 KiB src/lib/api.ts
93.3 KiB ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js
92.7 KiB src/products.tsx
90.6 KiB ../node_modules/.pnpm/@tiptap+core@3.20.6_@tiptap+pm@3.20.6/node_modules/@tiptap/core/dist/index.js
Largest files eagerly shipped from src/scenes/dashboard/Dashboard.tsx
Size File
306.2 KiB ../node_modules/.pnpm/posthog-js@1.435.5_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs
279.9 KiB src/taxonomy/core-filter-definitions-by-group.json
220.3 KiB ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js
181.8 KiB src/queries/validators.js
153.7 KiB ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js
126.8 KiB ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js
110.1 KiB ../packages/quill/packages/quill/dist/index.js
100.5 KiB src/lib/api.ts
93.3 KiB ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js
92.7 KiB src/products.tsx
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx + src/scenes/project-homepage/ProjectHomepage.tsx + src/scenes/project-homepage/today/TodayHome.tsx
Size File
306.2 KiB ../node_modules/.pnpm/posthog-js@1.435.5_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs
279.9 KiB src/taxonomy/core-filter-definitions-by-group.json
220.3 KiB ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js
153.7 KiB ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js
126.8 KiB ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js
110.1 KiB ../packages/quill/packages/quill/dist/index.js
100.5 KiB src/lib/api.ts
93.3 KiB ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js
92.7 KiB src/products.tsx
90.6 KiB ../node_modules/.pnpm/@tiptap+core@3.20.6_@tiptap+pm@3.20.6/node_modules/@tiptap/core/dist/index.js
Largest files eagerly shipped from src/scenes/activity/explore/EventsScene.tsx
Size File
306.2 KiB ../node_modules/.pnpm/posthog-js@1.435.5_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs
279.9 KiB src/taxonomy/core-filter-definitions-by-group.json
220.3 KiB ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js
181.8 KiB src/queries/validators.js
153.7 KiB ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js
126.8 KiB ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js
110.1 KiB ../packages/quill/packages/quill/dist/index.js
100.5 KiB src/lib/api.ts
93.3 KiB ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js
92.7 KiB src/products.tsx
Largest files eagerly shipped from src/scenes/session-recordings/detail/SessionRecordingDetail.tsx
Size File
315.5 KiB ../node_modules/.pnpm/posthog-js@1.435.5_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/rrweb.js
306.2 KiB ../node_modules/.pnpm/posthog-js@1.435.5_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs
279.9 KiB src/taxonomy/core-filter-definitions-by-group.json
220.3 KiB ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js
181.8 KiB src/queries/validators.js
153.7 KiB ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js
126.8 KiB ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js
110.1 KiB ../packages/quill/packages/quill/dist/index.js
100.5 KiB src/lib/api.ts
93.3 KiB ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js

Posted automatically by check-eager-graph · sizes are eager output bytes (shipped, post-tree-shake) from the esbuild metafile · part of #32479

✅ Toolbar bundle — eager 2.20 MiB within budget

What the toolbar ships to customer pages, measured from the esbuild output (minified, post-tree-shake). The eager set is the entry plus everything statically imported from it — fetched before any feature runs; deferred chunks load lazily. The eager guardrail is 5.72 MiB. Each output file must also stay below 10 MB, where CloudFront stops compressing it. The module boundary is enforced separately by check-toolbar-graph.

Metric Size Δ vs base Budget
Eager (shipped)
entry + static imports
2.20 MiB · 19 files 🔺 +55 B (+0.0%) ████░░░░░░ 38.4% of 5.72 MiB
Deferred (lazy) 2.11 MiB · 44 files no change n/a — loads on demand
Loader dist/toolbar.js 1.2 KiB no change █░░░░░░░░░ 6.0% of 19.5 KiB
Largest eagerly-shipped chunks
Size File
835.6 KiB dist/toolbar/toolbar-app-IDXKA4WE.css
657.5 KiB dist/toolbar/chunk-chunk-BALLB66W.js
259.4 KiB dist/toolbar/chunk-chunk-7JWMBALG.js
138.2 KiB dist/toolbar/chunk-chunk-RXJG6CZC.js
131.8 KiB dist/toolbar/chunk-chunk-FDH2IBXT.js
75.2 KiB dist/toolbar/toolbar-app-YIYWW6XJ.js
69.0 KiB dist/toolbar/chunk-chunk-TSAL54PB.js
35.6 KiB dist/toolbar/chunk-chunk-MM7MZI2L.js
21.0 KiB dist/toolbar/chunk-chunk-EZFR5QGQ.js
6.8 KiB dist/toolbar/chunk-chunk-DV7IWQNF.js

Posted automatically by check-toolbar-size · sizes are toolbar output bytes (shipped, post-tree-shake) from the esbuild metafile

✅ Dist folder size — 🔺 +94.4 KiB (+0.0%)

Total size of the built frontend/dist folder (all assets), compared against the base branch.

Total: 960.44 MiB · 🔺 +94.4 KiB (+0.0%)

@greptile-apps

greptile-apps Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Retrigger

[Medium risk] Fixes filter application from recent saved filters list.

The PR is not ready to merge because valid Recent selections can still silently fail, and the test fixtures must meet the repository requirement.

Reviews (1) · Last reviewed commit: "fix(replay): apply a saved filter picked..."

Comment thread frontend/src/lib/components/UniversalFilters/universalFiltersLogic.ts Outdated
Comment thread frontend/src/lib/components/UniversalFilters/universalFiltersLogic.test.ts Outdated
@coderabbitai

coderabbitai Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

📝 Walkthrough

Walkthrough

The change adds an action that applies a saved filter by short ID. It checks loaded results first and fetches the playlist when needed. It applies filters when available and shows an error toast when fetching fails or returns no filters. The ReplaySavedFilters branch uses direct application for applicable items and uses the short ID for other items when propertyKey is present. Tests cover these paths.

Priority: ⬇️ Low

Merge Risk: 🔵 Low · up to 0c001

Quickly selecting two saved filters can apply the earlier choice instead of the latest one. Guard the pending lookup before merging, or explicitly accept this bounded interaction risk.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 0c001

The change reuses existing playlist access checks. However, an older lookup can replace a newer filter selection and change the target of a later explicit save. Behavior across project changes and remounts remains unverified; no authorization bypass was established.

Retained concerns

  • Low · reliability · inferred: An older asynchronous Recent lookup can replace a newer direct selection or another Recent result. The consumer also replaces the active saved-filter identity used by a later explicit save, so application ordering and subsequent resource ownership follow response completion rather than the user's latest selection. No automatic persistence or authorization bypass was established.
Security review details

Security Blast Radius

  • inferred — The demonstrated consequence affects the mounted replay filter state and the identity of a playlist used for a later user-initiated save. The lookup itself performs a read, and no automatic persistent update is present in its application consumer. Cross-project stale-state exposure remains unresolved.

Trust Boundaries and Controls

  • observed — The Recent path stringifies the stored identifier without runtime format validation, and the playlist URL builder appends it as an unencoded component. The same API already accepts a URL-supplied savedFilterId through redirect handling, so this boundary condition predates the new caller. Final transport handling was not fully traced, and no introduced routing exploit was established.

Resilience and Maintainability Implications

  • observed — Failed resolution does not dispatch a new filter application. Subsequent persistence requires an explicit save action, and the interface displays the currently applied filter and save-target name. These controls limit, but do not prevent, stale lookup results from changing resource-selection state.

Hardening Proposals

  • proposed — Bind application requests to a shared selection generation and originating project or owner context. Invalidate that generation for both direct and Recent selections, resets, and lifecycle changes, and reject stale results before publishing application state. Cancellation limited to repeated short-ID requests would not protect a newer direct selection.
🚥 Pre-merge checks | ✅ 1
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The description is complete and follows the repository template. It explains the user impact, implementation, fallback behavior, error handling, tests, release status, documentation decision, and agen…
✨ Finishing Touches 💡 1
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
frontend/src/lib/components/UniversalFilters/universalFiltersLogic.test.ts (1)

381-405: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Exercise the ReplaySavedFilters caller path.

The new tests call resolveSavedFilter directly. They do not call addGroupFilter or assert that requestApplySavedFilter receives the resolved playlist. Add one focused logic test for a Recent item through addGroupFilter; it would catch caller-wiring regressions without duplicating the helper tests.


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: PostHog/posthog/.coderabbit.yaml

Review profile: QUIET

Plan: Enterprise

Run ID: 951d3e3a-a83c-46ba-9a56-6d8b1cd500f7

📥 Commits

Reviewing files that changed from the base of the PR and between 1b7ad38 and f5b5a7f.

📒 Files selected for processing (2)
  • frontend/src/lib/components/UniversalFilters/universalFiltersLogic.test.ts
  • frontend/src/lib/components/UniversalFilters/universalFiltersLogic.ts

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 10 remain after this review.

Resolving the picked short id against `savedFilters.results` alone was too
narrow. That list holds one page of 30, and the saved filters panel narrows it
further with its search and created-by filters, so a filter the picker offers
can be absent from it. The pick then applied nothing and said nothing.

The replay logic now owns the resolution through
`requestApplySavedFilterByShortId`. It uses the loaded list first, falls back to
one fetch by short id, and reports an unresolvable pick with a toast instead of
returning in silence.

Tests: the resolution cases move to the replay logic (loaded hit with no
request, a miss that fetches, and an unresolvable id), and the universal filters
tests now cover the wiring from the picker, which the pure helper tests did not
reach.

Generated-By: PostHog Desktop
Task-Id: f48638a7-ba40-4124-bb01-b1c3e42b429a

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Note

Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.

🟡 Other comments (1)
frontend/src/scenes/session-recordings/filters/sessionRecordingSavedFiltersLogic.ts-283-297 (1)

283-297: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Guard the shared application against stale short-ID results.

breakpoint() in requestApplySavedFilterByShortId only cancels a superseded short-ID listener. A direct selection dispatches requestApplySavedFilter separately, so it does not cancel the pending fetch. The stale fetch can still replace the newer selection in pendingFilterApplication.

Track a request generation for both actions. Apply the fetched filter only when its generation is still current.

Suggested fix
@@
         pendingFilterApplication: [
             null as SessionRecordingPlaylistType | null,
             {
                 requestApplySavedFilter: (_, { filter }) => filter,
                 clearPendingFilterApplication: () => null,
             },
         ],
+        savedFilterRequestGeneration: [
+            0,
+            {
+                requestApplySavedFilter: (state) => state + 1,
+                requestApplySavedFilterByShortId: (state) => state + 1,
+            },
+        ],
@@
-        requestApplySavedFilterByShortId: async ({ shortId }) => {
+        requestApplySavedFilterByShortId: async ({ shortId }, breakpoint) => {
+            const requestGeneration = values.savedFilterRequestGeneration
             const loadedFilter = values.savedFilters.results.find((savedFilter) => savedFilter.short_id === shortId)
             if (loadedFilter?.filters) {
                 actions.requestApplySavedFilter(loadedFilter)
                 return
             }
@@
             // filter of the saved filters panel, so a filter the picker offers can be absent from it.
             const fetchedFilter = await api.recordings.getPlaylist(shortId).catch(() => null)
+            breakpoint()
+            if (values.savedFilterRequestGeneration !== requestGeneration) {
+                return
+            }
             if (fetchedFilter?.filters) {
                 actions.requestApplySavedFilter(fetchedFilter)
                 return

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: PostHog/posthog/.coderabbit.yaml

Review profile: QUIET

Plan: Enterprise

Run ID: 03751bcb-dd8d-4672-b0a9-4fc8af7debce

📥 Commits

Reviewing files that changed from the base of the PR and between f5b5a7f and 0c001e0.

📒 Files selected for processing (4)
  • frontend/src/lib/components/UniversalFilters/universalFiltersLogic.test.ts
  • frontend/src/lib/components/UniversalFilters/universalFiltersLogic.ts
  • frontend/src/scenes/session-recordings/filters/sessionRecordingSavedFiltersLogic.test.ts
  • frontend/src/scenes/session-recordings/filters/sessionRecordingSavedFiltersLogic.ts

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 9 remain after this review.

@trunk-io

trunk-io Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Static Badge   Static Badge   Static Badge

Failed Test Failure Summary Logs
Scenes/Code review Default play-test The test failed because a logic component was not mounted when accessed, and there were unhandled network requests intercepted by the mock service... Logs ↗︎
compareTopLevelSections() reports a modifiers change when the current query overrides the team default A TypeError occurred because the code attempted to access the 'add' property of an undefined object. Logs ↗︎

View Full Report ↗︎ ⋅ Docs

@phillram phillram added the reviewhog ($$$) Reviews pull requests before humans do label Oct 2, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

reviewhog ($$$) Reviews pull requests before humans do

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant