Skip to content

fix(admin): guard every HogQL query against PostHog's silent default LIMIT 100 (ports #10194)#10361

Merged
Git-on-my-level merged 2 commits into
BasedHardware:mainfrom
Git-on-my-level:port/admin-hogql-row-limit-guard
Jul 23, 2026
Merged

fix(admin): guard every HogQL query against PostHog's silent default LIMIT 100 (ports #10194)#10361
Git-on-my-level merged 2 commits into
BasedHardware:mainfrom
Git-on-my-level:port/admin-hogql-row-limit-guard

Conversation

@Git-on-my-level

@Git-on-my-level Git-on-my-level commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Summary

PostHog's query API fills LIMIT 100 into any HogQL (sub)query that carries none and truncates silently. This PR ports the fix from #10194 — a shared guard that closes that class across the web admin app — onto current main, carrying the original commits forward verbatim so their authors retain credit.

Ported from #10194 by @aryanorastar. Both commits cherry-pick cleanly onto main (none of the six touched files have changed since #10194's base), so the original author (Aryan <aryangupts05@gmail.com>) remains the commit author of both. No reconstruction was needed — this is a direct cherry-pick, not a rewrite.

Fixes #10190.

Root cause

PostHog silently injects LIMIT 100 into any HogQL query (or subquery) that lacks an explicit LIMIT. The cap truncates results with no error and no log, so affected panels read plausible-but-wrong numbers (often 0) for the rows that were dropped:

  • The releases route groups by version × event over 60 days with no LIMIT and bypasses the shared fetch helper — 5 events × ~40–80 desktop versions exceeds 100, so per-version crash/launch/feedback counts read 0 for most versions today.
  • Several other routes hand-fixed this with ad-hoc limits that overstate PostHog's real served max (LIMIT 100000 / LIMIT 500000, when the verified served maximum is 50 000).
  • The response-reliability charts were already fixed in fix(admin): response reliability charts dropped newest days (HogQL default LIMIT 100) #10191 by pinning explicit limits; the same class remained everywhere else.

Verified live against project 302298 in #10190: SELECT 1 UNION ALL SELECT 2 LIMIT 1 → 2 rows (a trailing LIMIT after UNION ALL binds the last arm only), and LIMIT 100000 returns exactly 50 000 rows.

Durable guard

  • New withRowLimit(query, max) in web/admin/lib/posthog.ts — binds one explicit outer LIMIT at PostHog's served max (50 000) via a subquery wrap.
  • Applied at posthogFetch, the low-level chokepoint that both posthogResults and cachedPosthogFetch route through — so every shared-helper caller is guarded automatically.
  • Applied directly in the three routes with their own fetch that bypass the chokepoint: releases, stats/profitability (incl. fetchDesktopActivePerDay's previously LIMIT-less GROUP BY), and stats/onboarding/posthog.
  • No-bypass ratchet (lib/__tests__/posthog-no-bypass.test.ts): any file sending a raw kind: "HogQLQuery" body must also apply withRowLimit, so a future own-fetch route can't silently reintroduce the truncation. Fails on exactly the pre-fix state.

Correctness properties of the wrap:

  • UNION safety: wrapping in a subquery makes the single outer LIMIT bind the whole result (not just the last arm).
  • Ceiling-only: wrapping never widens an intentionally small query — a caller's tighter inner LIMIT 10 sits inside the subquery and still wins; the outer 50 000 is just a cap.
  • posthogResults' truncation observer moves from the fragile exactly-100 heuristic to the real signal now that an explicit limit is guaranteed.

Attribution treatment

Both source commits from #10194 are cherry-picked verbatim onto current main (their tree-diff is byte-identical to #10194's — verified with git diff). Original commit author Aryan <aryangupts05@gmail.com> (@aryanorastar) is preserved as author on both; only the committer changed (expected for a cherry-pick). This is a direct port, not a reconstruction; credit is fully retained.

Validation (run independently on current main, head 9b7ffc7)

From web/admin:

  • npm run typecheck (tsc --noEmit) → clean.
  • npx vitest run36 passed (5 files), incl. the new posthog-row-limit.test.ts (plain query gets an explicit outer LIMIT; a UNION is wrapped so the LIMIT binds the whole result; ceiling-only inner-limit preservation; custom max) and posthog-no-bypass.test.ts (the bypass ratchet).
  • npm run lint0 errors (23 pre-existing <img> / react-hooks/exhaustive-deps warnings in unrelated components, unchanged by this diff).
  • scripts/pr-preflight --base origin/main --head HEAD --lane local (with this body) → PASS; product invariants: none; failure-class: none.

Limitations of live PostHog verification

Not exercised against a live PostHog project: no admin PostHog key is available in this environment. The truncation mechanism, the UNION binding behavior, and the 50 000 served max are documented and verified live in #10190; this PR bounds every query path so the default-100 cap can no longer apply, and the unit tests assert the wrap shape directly. A live run against project 302298 would be the maintainer's call during review.

Out of scope (per #10194)

  • The writeCache >900 KB Firestore-field skip — a separate caching-completeness concern, not query truncation.
  • Full migration of the releases route onto posthogResults (for Firestore caching) — a larger, separable refactor.
  • A hard upper bound on the unclamped days param for notifications — now truncation-safe via the chokepoint guard; a resource-shape concern, not correctness.

Failure-Class: none

Review in cubic

aryanorastar and others added 2 commits July 22, 2026 19:34
…LIMIT 100

PostHog fills LIMIT 100 into any HogQL (sub)query without one and truncates
silently. BasedHardware#10191 fixed the response-reliability charts by pinning explicit
limits; the same class remained on the releases panel — 5 events x ~40-80
desktop versions exceeds 100, so per-version crash/launch/feedback counts read
as 0 for most versions — and on any other LIMIT-less query.

Adds withRowLimit(query, max) to lib/posthog: one explicit outer LIMIT bound via
a subquery wrap. The wrap is required for UNION correctness — a trailing LIMIT
after UNION ALL binds the last arm only (SELECT 1 UNION ALL SELECT 2 LIMIT 1
returns 2 rows). Applied at posthogFetch, the chokepoint both posthogResults and
cachedPosthogFetch route through, so every shared-helper caller is guarded; and
directly in the releases route, whose own fetch bypasses that chokepoint.
Wrapping only ever adds a ceiling — a caller's tighter inner LIMIT still wins, so
it never widens an intentionally small query.

posthogResults' truncation observer moves from the fragile exactly-100 heuristic
to the real signal now that an explicit limit is guaranteed: results at the
served max (50k, PostHog's verified maximum).

Test: withRowLimit unit coverage (plain query; UNION binds the whole result not
the last arm; ceiling-only inner-limit preservation; custom max). typecheck +
all 35 web/admin tests + lint (0 errors) green.

Failure-Class: none

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…o-bypass ratchet

Follow-up to review on BasedHardware#10190: the row-limit guard must cover every admin
HogQL request, but two routes have their own fetch that bypasses
lib/posthog's posthogFetch chokepoint and were still sending unwrapped
queries — profitability (fetchDesktopActivePerDay's GROUP BY day has no
LIMIT; the other query's ad-hoc LIMIT 500000 overstates PostHog's 50k
served max) and onboarding. Both now apply withRowLimit directly, matching
the releases route.

Adds a regression ratchet (posthog-no-bypass.test.ts): any file sending a
raw 'kind: HogQLQuery' body must also apply withRowLimit, so a future route
with its own fetch can't silently reintroduce the default-100 truncation.
The test fails on exactly the state this commit fixes.

Kept the edits minimal and style-matched rather than prettier-reformatting
the whole files — web/admin has no prettier CI check (only web/frontend
does), and these files were already non-compliant on main, so a wholesale
reformat would be hundreds of lines of unrelated churn.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@Git-on-my-level
Git-on-my-level merged commit 019e073 into BasedHardware:main Jul 23, 2026
28 of 31 checks passed
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.

admin: HogQL queries without LIMIT are silently truncated to 100 rows (releases panel likely affected); shared guard needed

2 participants