8000 test(max): Make evals reflect user flow by using root node by Twixes · Pull Request #32751 · PostHog/posthog · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

test(max): Make evals reflect user flow by using root node #32751

New issue 10000

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 4 commits into from
Jun 11, 2025
Merged

test(max): Make evals reflect user flow by using root node #32751

merged 4 commits into from
Jun 11, 2025

Conversation

Twixes
Copy link
Member
@Twixes Twixes commented May 28, 2025 •
edited
Loading

Problem

We have dozens of AI evaluation cases, but they don't test insight type selection right now, as they all feed questions into an already-selected insight type flow.

Changes

Let's make insight creation evals evaluate reality, by going through the root node. This will allow us to actually see the failure modes around wrong insight types being chosen. Insight-kind specific call_nodes fixtures are replaced by a common call_root_for_insight_generation.

We now also have a simple eval scorer for query kind selection, called… QueryKindSelection.

How did you test this code?

These are the tests.

greptile-apps[bot]
greptile-apps bot reviewed May 28, 2025
View reviewed changes
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.

PR Summary

This PR restructures the AI evaluation framework to test insight type selection by routing queries through the root node, better reflecting real user interaction flow.

  • Added call_root_for_insight_generation fixture in ee/hogai/eval/conftest.py to route all insight evaluations through root node
  • Introduced QueryKindSelection scorer in ee/hogai/eval/scorers.py to verify correct insight type selection
  • Added add_query_creation_flow method in ee/hogai/graph/graph.py to separate query creation from execution
  • Refactored all insight evaluations (trends, funnels, retention, SQL) to use root node routing instead of direct node access
  • Removed individual call_node fixtures from each insight evaluation file in favor of unified root approach

8 file(s) reviewed, 5 comment(s)
Edit PR Review Bot Settings | Greptile

@posthog-bot

This comment was marked as outdated.

Sign in to view
@posthog-bot

This comment was marked as outdated.

Sign in to view
@Twixes Twixes force-pushed the max-eval-unify branch 2 times, most recently from 18e1f68 to 821ded8 Compare June 6, 2025 12:59
@posthog-bot

This comment was marked as outdated.

Sign in to view
@Twixes Twixes requested a review from a team June 6, 2025 14:56
@posthog-bot

This comment was marked as outdated.

Sign in to view
kappa90
kappa90 reviewed Jun 6, 2025
View reviewed changes
Copy link
Contributor
@kappa90 kappa90 left a comment

Choose a reason for hiding this comment

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

Won't this become superseded by the new planning architecture you're working on?

@posthog-bot posthog-bot removed the stale label Jun 9, 2025
@posthog-bot

This comment was marked as outdated.

Sign in to view
@posthog-bot
Copy link
Contributor

🧠 AI eval results

Evaluated 6 experiments, comprising 19 metrics.

funnel

🆕 QueryKindSelection: 100.00%
🆕 plan_correctness: 92.45%
🆕 query_and_plan_alignment: 86.98%
🆕 time_range_relevancy: 95.88%

Avg. case performance: ⏱️ 91.16 s, 🔢 6338 tokens, 💵 $0.0166 in tokens

memory

🆕 ToolRelevance: 98.24%
🆕 memory_content_relevance: 90.00%

Avg. case performance: ⏱️ 6.43 s, 🔢 1213 tokens, 💵 $0.0033 in tokens

retention

🆕 QueryKindSelection: 100.00%
🆕 plan_correctness: 61.67%
🆕 query_and_plan_alignment: 57.50%
🆕 time_range_relevancy: 96.15%

Avg. case performance: ⏱️ 28.69 s, 🔢 5384 tokens, 💵 $0.0162 in tokens

root

🆕 ToolRelevance: 58.88%

Avg. case performance: ⏱️ 5.36 s, 🔢 0 tokens

sql

🆕 QueryKindSelection: 0.00%
🆕 plan_correctness: 83.33%
🆕 query_and_plan_alignment: 50.00%
🆕 time_range_relevancy: 100.00%

Avg. case performance: ⏱️ 13.82 s, 🔢 16194 tokens, 💵 $0.0415 in tokens

trends

🆕 QueryKindSelection: 100.00%
🆕 plan_correctness: 72.14%
🆕 query_and_plan_alignment: 79.47%
🆕 time_range_relevancy: 96.84%

Avg. case performance: ⏱️ 39.50 s, 🔢 11009 tokens, 💵 $0.0296 in tokens

Triggered by this commit.

@Twixes Twixes requested a review from kappa90 June 10, 2025 19:00
Twixes and others added 4 commits June 10, 2025 21:01
@Twixes
Copy link
Member Author
Twixes commented Jun 10, 2025

Had this on the back burner for a bit, but good to go now.
@kappa90 In fact this is the base for the planner rework PR, as this change allows us to prevent introducing regressions in query kind choice.
Braintrust comparison logic got borked, so annoyingly the eval runs here shows up as "new", even though we already have a baseline. I've already messaged them about this.

@posthog-bot
Copy link
Contributor

🧠 AI eval results

Evaluated 6 experiments, comprising 19 metrics.

funnel

🆕 QueryKindSelection: 100.00%
🆕 plan_correctness: 94.90%
🆕 query_and_plan_alignment: 87.04%
🆕 time_range_relevancy: 97.65%

Avg. case performance: ⏱️ 93.25 s, 🔢 6209 tokens, 💵 $0.0163 in tokens

memory

🆕 ToolRelevance: 99.16%
🆕 memory_content_relevance: 94.29%

Avg. case performance: ⏱️ 5.02 s, 🔢 1213 tokens, 💵 $0.0033 in tokens

retention

🆕 QueryKindSelection: 100.00%
🆕 plan_correctness: 51.67%
🆕 query_and_plan_alignment: 53.64%
🆕 time_range_relevancy: 97.73%

Avg. case performance: ⏱️ 31.34 s, 🔢 4557 tokens, 💵 $0.0162 in tokens

root

🆕 ToolRelevance: 58.88%

Avg. case performance: ⏱️ 5.78 s, 🔢 0 tokens

sql

🆕 QueryKindSelection: 0.00%
🆕 plan_correctness: 66.67%
🆕 query_and_plan_alignment: 50.00%
🆕 time_range_relevancy: 100.00%

Avg. case performance: ⏱️ 11.68 s, 🔢 12605 tokens, 💵 $0.0323 in tokens

trends

🆕 QueryKindSelection: 100.00%
🆕 plan_correctness: 76.90%
🆕 query_and_plan_alignment: 80.00%
🆕 time_range_relevancy: 97.75%

Avg. case performance: ⏱️ 43.00 s, 🔢 9889 tokens, 💵 $0.0267 in tokens

Triggered by this commit.

@Twixes Twixes merged commit 3ea84d4 into master Jun 11, 2025
97 checks passed
@Twixes Twixes deleted the max-eval-unify branch June 11, 2025 08:00
@Twixes Twixes mentioned this pull request Jun 11, 2025
Open
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@greptile-apps greptile-apps[bot] greptile-apps[bot] left review comments

@kappa90 kappa90 kappa90 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Twixes @posthog-bot @kappa90
0