Skip to content

fix(subscriptions): freeze post-fix HogQL in the AI query plan - #69637

Merged
vdekrijger merged 3 commits into
masterfrom
posthog-code/ai-report-freeze-postfix-hogql
Jul 9, 2026
Merged

fix(subscriptions): freeze post-fix HogQL in the AI query plan#69637
vdekrijger merged 3 commits into
masterfrom
posthog-code/ai-report-freeze-postfix-hogql

Conversation

@vdekrijger

@vdekrijger vdekrijger commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Problem

AI subscription reports freeze a query plan (Subscription.ai_query_plan) on the first delivery so later runs skip the planner LLM and produce consistent results. But the frozen plan stored the planner's original per-step HogQL — and a step can succeed only after the fix LLM (_arequest_hogql_fix) rewrites its query mid-run.

Freezing the pre-fix form means every reused delivery replays the known-broken query, fails, and re-bills the fix LLM for the same repair — the exact repetition the frozen plan exists to avoid. Determinism is also only partial: the fixer re-rolls each run and may not produce the same rewrite twice.

Found during review of the (now closed) #66824.

Changes

  • run_step writes each succeeding step's final window-agnostic HogQL (post any fix-LLM rewrite) back onto its plan step, so _plan_to_freeze freezes what actually ran. A failed step never reaches the write-back and keeps the planner's original — a broken rewrite is never kept. Synthesis reads only the plan's intent, so the in-place update has no other readers. (An earlier revision threaded this through widened return tuples; a review pass simplified it to the write-back.)
  • The existing fail-safe now applies to the post-fix text: if the fixer stripped the {{date_range}}-style window placeholder, nothing is frozen (freezing it would cement an unbounded scan) and the next run re-plans — same degradation semantics as today.
flowchart LR
    P[planner HogQL] --> E{executes?}
    E -- yes --> F[freeze as-is]
    E -- no --> X[fix LLM rewrite] --> R{rerun succeeds?}
    R -- yes --> G[freeze POST-fix HogQL]
    R -- no --> O[keep original, step degrades]
    G --> H{placeholder still present?}
    H -- no --> N[do not freeze - re-plan next run]
Loading

How did you test this code?

I'm an agent (Claude Code via PostHog Code). Automated tests only — no manual testing claimed.

  • New parameterized test test_freeze_carries_post_fix_hogql (through the public generate_ai_report): (a) a step fixed by the fix LLM freezes the post-fix HogQL, not the original — catches the regression where reused plans replay broken queries; (b) a fixer output without the window placeholder freezes nothing — pins the fail-safe to the post-fix text.
  • Extended test_run_steps_retries_then_succeeds to assert the freezable form is the fixed query.
  • Full touched suite green locally: test_report_pipeline.py 30/30. Existing tests updated only mechanically for the widened _run_steps/_execute_plan return tuple; none encoded the old pre-fix freezing behavior.
  • ruff check / ruff format clean. (Local DB-fixture errors in unrelated test files reproduce identically on clean master — pre-existing environment staleness, not this change.)

Automatic notifications

  • Publish to changelog?
  • Alert Sales and Marketing teams?

Docs update

No user-facing docs describe the freeze internals; behavior change is cost/consistency only.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

  • Built with Claude Code (PostHog Code) in an isolated worktree, directed by @vdekrijger.
  • Origin: review finding on feat(subscriptions): edit and regenerate the frozen AI query plan #66824 (closed in favor of this upstream fix — improving planner/frozen-plan quality beats maintaining owner-facing plan-editing escape hatches).
  • Key decision: failed steps keep the planner's original HogQL in the frozen plan rather than a possibly-broken rewrite; the placeholder fail-safe intentionally moved to the post-fix text.

Created with PostHog Code

The frozen plan stored the planner's original per-step HogQL, but a step
can succeed only after the fix LLM rewrites its query. Freezing the
pre-fix form meant every reused delivery replayed the broken query and
re-billed the fix LLM for the same repair - the exact repetition the
frozen plan exists to avoid.

_run_steps now threads each step's final window-agnostic HogQL (post any
fix rewrites; the original for failed steps) out to _plan_to_freeze,
which freezes a copy of the plan carrying those queries. The existing
fail-safe still applies to the post-fix text: if the fixer stripped the
window placeholder, nothing is frozen and the next run re-plans.

Generated-By: PostHog Code
Task-Id: 2ac31fed-9d1f-4e72-adcd-2ed69d049cba
…threading

Review-swarm simplify pass: instead of threading each step's final HogQL
through a widened 4-tuple across run_step -> _run_steps -> _execute_plan
-> _plan_to_freeze and deep-copying the plan, run_step writes the
succeeding query back onto its own step. Failed steps keep the planner's
original by not reaching the write; synthesis reads only the plan intent,
so the in-place update has no other readers. Same behavior, 40 fewer
lines, no signature changes.

Generated-By: PostHog Code
Task-Id: 2ac31fed-9d1f-4e72-adcd-2ed69d049cba
@vdekrijger
vdekrijger marked this pull request as ready for review July 9, 2026 12:24
@vdekrijger vdekrijger added the stamphog Request AI approval (no full review) label Jul 9, 2026
@pr-assigner-resolver-posthog
pr-assigner-resolver-posthog Bot requested a review from a team July 9, 2026 12:24
@greptile-apps

greptile-apps Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Reviews (1): Last reviewed commit: "refactor(subscriptions): write post-fix ..." | Re-trigger Greptile

@github-actions github-actions 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.

Contained fix to internal AI subscription plan-freezing logic with added parameterized tests covering both the fix-path and fail-safe; author is on the owning team with strong familiarity, and Greptile reviewed with no concerns. No risky territory (no schema/API/auth/billing/CI/deps touched).

  • Author wrote 100% of the modified lines and has 16 merged PRs in these paths (familiarity STRONG).
  • 👍 on the PR from greptile-apps[bot], hex-security-app[bot].
Gate mechanics and policy version
Gate Result
prerequisites all clear
deny-list no deny categories matched
size 5L, 1F substantive, 56L/2F incl. docs/generated/snapshots — within ceiling
tier T1-agent / T1b-small (56L, 2F, single-area, fix)
stamphog 2.0.0b3 .stamphog/policy.yml @ 1c274d3 · reviewed head 96b169f

@trunk-io

trunk-io Bot commented Jul 9, 2026

Copy link
Copy Markdown

Static BadgeStatic BadgeStatic BadgeStatic Badge

View Full Report ↗︎Docs

Extends test_freeze_carries_post_fix_hogql past the freeze assertion:
feed the frozen envelope back into generate_ai_report as ai_query_plan
(through the real build_frozen_prompt validation) with an executor that
only accepts the fixed query. Proves the payoff the freeze exists for -
the reused plan runs the fixed query first try and never re-invokes the
fix LLM. The prior coverage only asserted the frozen dict's shape; a
regression to pre-fix freezing would have re-billed the fixer on every
delivery without any test tripping on the reuse side.

Generated-By: PostHog Code
Task-Id: 2ac31fed-9d1f-4e72-adcd-2ed69d049cba
@vdekrijger
vdekrijger enabled auto-merge (squash) July 9, 2026 12:46
@stamphog

stamphog Bot commented Jul 9, 2026

Copy link
Copy Markdown

Retaining stamphog approval — delta since last review classified as trivial_paths.

@vdekrijger
vdekrijger merged commit 16a388c into master Jul 9, 2026
306 of 341 checks passed
@vdekrijger
vdekrijger deleted the posthog-code/ai-report-freeze-postfix-hogql branch July 9, 2026 14:51
@deployment-status-posthog

deployment-status-posthog Bot commented Jul 9, 2026

Copy link
Copy Markdown

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-07-09 15:35 UTC Run
prod-us ✅ Deployed 2026-07-09 16:06 UTC Run
prod-eu ✅ Deployed 2026-07-09 16:06 UTC Run

Gilbert09 added a commit that referenced this pull request Jul 17, 2026
test_plan_to_freeze_requires_no_failures called _plan_to_freeze without the
relevant_events argument that became required, breaking repo-wide mypy on master
(a semantic merge conflict between #69637 and #71837). Add the argument and the
matching key to the expected frozen envelope.

Generated-By: PostHog Code
Task-Id: b7c4d4cd-03f3-4055-af2d-4147a1b35747
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stamphog Request AI approval (no full review)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants