Skip to content

Conversation

@sortafreel
Copy link
Contributor

Problem

  • If the user provides incorrect session ids (manually, or sessions without replay recordings) - we still start workflows and fail

Changes

  • Confirm that the session has Replay data

How did you test this code?

👉 Stay up-to-date with PostHog coding conventions for a smoother review.

Changelog: (features only) Is this feature complete?

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Additional Comments (1)

  1. ee/hogai/chat_agent/session_summaries/test/test_nodes.py, line 654-697 (link)

    logic: This test may fail after this PR. The test provides specific_session_ids_to_summarize with a fake session ID that doesn't exist in ClickHouse, but the new validation code will now verify the session exists before proceeding. Since TestSessionSummarizationNode doesn't extend ClickhouseTestMixin, the validation query will return no results, causing the test to receive "No sessions were found" instead of proceeding to summarization.

    The test needs to either:

    1. Mock _validate_specific_session_ids to return the session IDs
    2. Mock database_sync_to_async like other tests in this class do

2 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Copy link
Member

@Twixes Twixes left a comment

Choose a reason for hiding this comment

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

:nice:

Twixes added a commit that referenced this pull request Nov 28, 2025
@sortafreel sortafreel enabled auto-merge (squash) December 2, 2025 10:53
@sortafreel sortafreel merged commit 3b74cd0 into master Dec 2, 2025
175 checks passed
@sortafreel sortafreel deleted the signals/validate-specific_session_ids_to_summarize branch December 2, 2025 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants