chore: separate aifilter into subcomponents and make it nicer #28735

veryayskiy · 2025-02-14T16:41:34Z

Separate the AI filter component into subcomponents
Use taxonomy.py instead of hard coded values
Move the component into a button instead of collapses element
Add validation of filters before applying

Before:

After:

…stHog/posthog into chore/sr/chat-with-recordings

…bject is valid

veryayskiy · 2025-02-14T16:45:19Z

Some of the files I guess changed because of the "ruff check --fix"

greptile-apps

PR Summary

This PR refactors the AI filter component in the session recordings interface, improving its organization and user experience while adding validation for filters.

Moved AI filter from a collapse element to a button-based interface in /frontend/src/scenes/session-recordings/components/AiFilter/AiFilter.tsx
Added validation in sessionRecordingsPlaylistLogic.ts to ensure filters meet type requirements before being applied
Created new AiFilterIntro component with FilmCameraHog icon and consent popover for better UX
Replaced hardcoded property definitions with dynamic taxonomy data from posthog.taxonomy.taxonomy in ai_filter_prompts.py
Added preference rules for property type selection (event > session > person) when handling ambiguous cases

_{14 file(s) reviewed, 8 comment(s)}
_{Edit PR Review Bot Settings | Greptile}

greptile-apps · 2025-02-14T16:43:29Z

frontend/src/scenes/session-recordings/playlist/Playlist.tsx

+                            {' '}
+                            {typeof content === 'function' ? content({ activeItem }) : content}


syntax: unnecessary whitespace character before content rendering

Suggested change

{' '}

{typeof content === 'function' ? content({ activeItem }) : content}

{typeof content === 'function' ? content({ activeItem }) : content}

greptile-apps · 2025-02-14T16:43:30Z

frontend/src/scenes/session-recordings/playlist/Playlist.tsx

+                        >
+                            <DraggableToNotebook href={notebooksHref}>{filterActions}</DraggableToNotebook>
+
+                            <div className="flex flex-col relative w-full bg-bg-light overflow-hidden h-full Playlist__list">


style: duplicate class name 'Playlist__list' applied to nested div, may cause styling conflicts

greptile-apps · 2025-02-14T16:43:43Z

frontend/src/scenes/session-recordings/components/AiFilter/AiFilterIntro.tsx

+    return (
+        <>
+            <div className="flex">
+                <AIConsentPopoverWrapper placement="right-end" middleware={[offset(-12)]} showArrow>


style: offset(-12) seems like a magic number. Consider defining this as a constant with a descriptive name to explain its purpose.

greptile-apps · 2025-02-14T16:44:34Z

posthog/api/decide.py

@@ -264,8 +264,7 @@ def get_decide(request: HttpRequest):
                request,
                generate_exception_response(
                    "decide",
-                    f"Team with ID {team.id} cannot access the /decide endpoint."
-                    f"Please contact us at [email protected]",
+                    f"Team with ID {team.id} cannot access the /decide endpoint.Please contact us at [email protected]",


style: Missing space after period in error message which may affect readability

greptile-apps · 2025-02-14T16:44:49Z

frontend/src/scenes/session-recordings/playlist/sessionRecordingsPlaylistLogic.ts

+    if ('duration' in filters) {
+        if (!Array.isArray(filters.duration)) {
+            return false
+        }
+        if (
+            filters.duration.length > 0 &&
+            (!filters.duration[0]?.type || !filters.duration[0]?.key || !filters.duration[0]?.operator)
+        ) {
+            return false
+        }
+    }


logic: Duration validation could be more robust - currently only checks first element's properties but array could contain invalid elements after index 0

greptile-apps · 2025-02-14T16:44:49Z

frontend/src/scenes/session-recordings/playlist/sessionRecordingsPlaylistLogic.ts

+    if ('filter_group' in filters) {
+        const group = filters.filter_group
+        if (!group || typeof group !== 'object') {
+            return false
+        }
+        if (!('type' in group) || !('values' in group) || !Array.isArray(group.values)) {
+            return false
+        }
+    }


logic: filter_group validation should also check that 'type' is a valid FilterLogicalOperator value

greptile-apps · 2025-02-14T16:44:50Z

frontend/src/scenes/session-recordings/playlist/sessionRecordingsPlaylistLogic.ts

+                    if (!isValidRecordingFilters(filters)) {
+                        console.error('Invalid filters provided:', filters)
+                        return state
+                    }


style: Consider throwing an error or showing a user-facing notification instead of just logging to console when invalid filters are provided

greptile-apps · 2025-02-14T16:45:41Z

posthog/session_recordings/ai_data/ai_filter_prompts.py

+    Full list of available properties and their definitions:
+    {json.dumps(CORE_FILTER_DEFINITIONS_BY_GROUP, indent=2)}


style: Using f-strings with json.dumps() could potentially cause issues if the JSON contains curly braces. Consider using a different string formatting approach or escaping the braces

we should probably prompt "when there is a choice prefer event properties over session properties and session properties over person properties"

event properties are faster and generally better match people's expected results better

github-actions · 2025-02-14T16:55:26Z

Size Change: 0 B

Total Size: 1.21 MB

ℹ️ View Unchanged

Filename	Size
`frontend/dist/toolbar.js`	1.21 MB

_{compressed-size-action}

posthog-bot · 2025-02-14T17:03:35Z

📸 UI snapshots have been updated

16 snapshot changes in total. 0 added, 16 modified, 0 deleted:

chromium: 0 added, 16 modified, 0 deleted (diff for shard 1, diff for shard 2)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

pauldambra · 2025-02-17T11:02:07Z

frontend/__snapshots__/components-playlist--default--dark.png

is the pop up here expected?

…stHog/posthog into chore/sr/chat-with-recordings

pauldambra

quick pass

pauldambra · 2025-02-17T11:10:45Z

frontend/__snapshots__/replay-player-failure--recent-recordings-404--dark.png

same popping in here

frontend/src/scenes/session-recordings/components/AiFilter/AiFilterInput.tsx

pauldambra · 2025-02-17T11:13:07Z

frontend/src/scenes/session-recordings/playlist/Playlist.tsx

@@ -83,6 +88,8 @@ export function Playlist({
        750: 'medium',
    })

+    const [isExpanded, setIsExpanded] = useState(false)


really this should be on a kea logic but I'll pretend not to notice 😉

pauldambra · 2025-02-17T11:14:01Z

frontend/src/scenes/session-recordings/playlist/Playlist.tsx

-                className={clsx('Playlist h-full min-h-96 w-full min-w-96 lg:min-w-[560px] order-first xl:order-none', {
-                    'Playlist--wide': size !== 'small',
-                    'Playlist--embedded': embedded,
+                className={clsx('flex flex-col w-full gap-2 h-full', {


are we sure we're not using the wide and embedded classes?
they might be necessary when shared or full screen?

pauldambra · 2025-02-17T11:14:17Z

frontend/src/scenes/session-recordings/playlist/Playlist.tsx

+                        ref={playlistRef}
+                        data-attr={dataAttr}
+                        className={clsx('Playlist w-full min-w-60 min-h-96', {
+                            'Playlist--wide': size !== 'small',


ah, they're down here 🤣

pauldambra · 2025-02-17T11:15:20Z

frontend/src/scenes/session-recordings/playlist/sessionRecordingsPlaylistLogic.ts

+ * @param filters - The filters to check.
+ * @returns True if the filters are valid, false otherwise.
+ */
+export function isValidRecordingFilters(filters: Partial<RecordingUniversalFilters>): boolean {


Suggested change

export function isValidRecordingFilters(filters: Partial<RecordingUniversalFilters>): boolean {

export function isValidRecordingFilters(filters: Partial<RecordingUniversalFilters>): x is Partial<RecordingUniversalFilters> {

can do this so TS can do type narrowing

pauldambra · 2025-02-17T11:17:14Z

posthog/session_recordings/ai_data/ai_filter_prompts.py

+    Full list of available properties and their definitions:
+    {json.dumps(CORE_FILTER_DEFINITIONS_BY_GROUP, indent=2)}


we should probably prompt "when there is a choice prefer event properties over session properties and session properties over person properties"

event properties are faster and generally better match people's expected results better

…ilterInput.tsx Co-authored-by: Paul D'Ambra <[email protected]>

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Peter Kirkham <[email protected]> Co-authored-by: Paul D'Ambra <[email protected]>

veryayskiy and others added 12 commits February 12, 2025 14:19

chore: separate aifilter into subcomponents and make it nicer

2aa27a2

Update UI snapshots for chromium (2)

f1c0fe4

fix: typo

363015a

Merge branch 'chore/sr/chat-with-recordings' of https://github.com/Po…

d4eac97

…stHog/posthog into chore/sr/chat-with-recordings

Merge branch 'master' into chore/sr/chat-with-recordings

799bb7e

chore:bump posthoganalytics version

b1ec89e

Merge branch 'chore/sr/chat-with-recordings' of https://github.com/Po…

520f668

…stHog/posthog into chore/sr/chat-with-recordings

fix: update default model

9f16dc7

chore: optimisation

dd9f4d4

chore: use bindlogic instead of passing it down

b01272b

chore: optimisation of promts and filter component + check that the o…

e7798e8

…bject is valid

Merge branch 'master' into chore/sr/chat-with-recordings

3cbb01c

greptile-apps bot reviewed Feb 14, 2025

View reviewed changes

github-actions bot added 2 commits February 14, 2025 16:59

Update UI snapshots for chromium (1)

af7bb8f

Update UI snapshots for chromium (2)

16d1ce3

veryayskiy and others added 7 commits February 14, 2025 19:46

fix: return that ruff check thing

0ca4715

fix: return that ruff check thing 2

9156e11

return that ruff check thing 3

3e9208b

fix: return

4d432e1

Merge branch 'master' into chore/sr/chat-with-recordings

81cf8cf

Merge branch 'master' into chore/sr/chat-with-recordings

b6b50c7

Merge branch 'master' into chore/sr/chat-with-recordings

54da2b0

veryayskiy requested a review from pauldambra February 17, 2025 11:01

pauldambra reviewed Feb 17, 2025

View reviewed changes

frontend/__snapshots__/components-playlist--default--dark.png Outdated

Copy link

Member

pauldambra Feb 17, 2025

Choose a reason for hiding this comment

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

is the pop up here expected?

veryayskiy added 3 commits February 17, 2025 12:04

chore: add posthog actions

cc15244

chore: more posthog actions

99783c7

Merge branch 'chore/sr/chat-with-recordings' of https://github.com/Po…

2458157

…stHog/posthog into chore/sr/chat-with-recordings

Merge branch 'master' into chore/sr/chat-with-recordings

6fc9ad2

pauldambra approved these changes Feb 17, 2025

View reviewed changes

veryayskiy and others added 3 commits February 17, 2025 12:25

Update frontend/src/scenes/session-recordings/components/AiFilter/AiF…

432bc4a

…ilterInput.tsx Co-authored-by: Paul D'Ambra <[email protected]>

Merge branch 'master' into chore/sr/chat-with-recordings

de31d4b

fix: prettier

f8fb89f

veryayskiy merged commit 53f5d58 into master Feb 17, 2025
104 checks passed

veryayskiy deleted the chore/sr/chat-with-recordings branch February 17, 2025 13:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: separate aifilter into subcomponents and make it nicer #28735

chore: separate aifilter into subcomponents and make it nicer #28735

veryayskiy commented Feb 14, 2025

veryayskiy commented Feb 14, 2025

greptile-apps bot left a comment

greptile-apps bot Feb 14, 2025

greptile-apps bot Feb 14, 2025

greptile-apps bot Feb 14, 2025

greptile-apps bot Feb 14, 2025

greptile-apps bot Feb 14, 2025

greptile-apps bot Feb 14, 2025

greptile-apps bot Feb 14, 2025

greptile-apps bot Feb 14, 2025

pauldambra Feb 17, 2025

github-actions bot commented Feb 14, 2025 •

edited

Loading

posthog-bot commented Feb 14, 2025

pauldambra Feb 17, 2025

pauldambra left a comment

pauldambra Feb 17, 2025

pauldambra Feb 17, 2025

pauldambra Feb 17, 2025

pauldambra Feb 17, 2025

pauldambra Feb 17, 2025

pauldambra Feb 17, 2025

		{' '}
		{typeof content === 'function' ? content({ activeItem }) : content}

		Full list of available properties and their definitions:
		{json.dumps(CORE_FILTER_DEFINITIONS_BY_GROUP, indent=2)}

	export function isValidRecordingFilters(filters: Partial<RecordingUniversalFilters>): boolean {
	export function isValidRecordingFilters(filters: Partial<RecordingUniversalFilters>): x is Partial<RecordingUniversalFilters> {

chore: separate aifilter into subcomponents and make it nicer #28735

chore: separate aifilter into subcomponents and make it nicer #28735

Conversation

veryayskiy commented Feb 14, 2025

veryayskiy commented Feb 14, 2025

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

greptile-apps bot Feb 14, 2025

Choose a reason for hiding this comment

greptile-apps bot Feb 14, 2025

Choose a reason for hiding this comment

greptile-apps bot Feb 14, 2025

Choose a reason for hiding this comment

greptile-apps bot Feb 14, 2025

Choose a reason for hiding this comment

greptile-apps bot Feb 14, 2025

Choose a reason for hiding this comment

greptile-apps bot Feb 14, 2025

Choose a reason for hiding this comment

greptile-apps bot Feb 14, 2025

Choose a reason for hiding this comment

greptile-apps bot Feb 14, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Feb 14, 2025 • edited Loading

posthog-bot commented Feb 14, 2025

📸 UI snapshots have been updated

Choose a reason for hiding this comment

pauldambra left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Feb 14, 2025 •

edited

Loading