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

[aryanorastar]

Summary

PostHog's query API fills LIMIT 100 into any HogQL (sub)query that carries none and truncates silently. #10191 fixed the response-reliability charts by pinning explicit limits; this closes the same class everywhere else with a shared guard.

• New withRowLimit(query, max) in web/admin/lib/posthog.ts — binds one explicit outer LIMIT at PostHog's served max (50k) via a subquery wrap.
• Applied at posthogFetch, the low-level chokepoint that both posthogResults and cachedPosthogFetch route through — so every shared-helper caller (response-reliability, notifications, …) is guarded automatically.
• Applied directly in the three routes with their own fetch that bypass the chokepoint: releases (version×event, 5 events × ~40–80 versions > 100, so per-version counts read 0 for most versions), stats/profitability (fetchDesktopActivePerDay's LIMIT-less GROUP BY; and an ad-hoc LIMIT 500000 that overstated the 50k served max), 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.

Fixes #10190.

Why a shared primitive (not per-route limits)

Per AGENTS.md, 2+ fixes sharing a cause warrant a reusable guard in the shared layer. This is that guard: one function, applied once at the fetch chokepoint, so no existing or future route can silently truncate. The two spots that don't reach the chokepoint (releases' own fetch) call the same primitive directly, so there's a single definition of "how we bound a HogQL result."

Correctness details

• UNION safety: a trailing LIMIT after UNION ALL binds the last arm only (SELECT 1 UNION ALL SELECT 2 LIMIT 1 → 2 rows, verified in the issue). Wrapping in a subquery makes the single outer LIMIT bind the whole result. This is the same technique fix(admin): response reliability charts dropped newest days (HogQL default LIMIT 100) #10191 used for the voice union.
• 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 50k is just a cap.
• Served max: 50k is PostHog's verified maximum (LIMIT 100000 returns exactly 50000). posthogResults' truncation observer moves from the fragile exactly-100 heuristic to the real signal now that an explicit limit is guaranteed: a result at the served max.

Failure class

Failure-Class: none

No registered class covers a provider's silent default-limit truncation of analytics reads; single instance, fixed with a shared primitive at the chokepoint rather than a per-caller guard.

Product invariants affected

none (verified with scripts/pr-preflight --suggest)

Validation

• New hermetic unit test (web/admin/lib/__tests__/posthog-row-limit.test.ts, vitest): plain query gets an explicit outer LIMIT; a UNION is wrapped so the LIMIT binds the whole result (asserts the LIMIT follows the closing paren, not the last arm); ceiling-only inner-LIMIT preservation; custom max.
• npm run typecheck (tsc --noEmit) → clean.
• npx vitest run → 35 passed (4 new + 31 existing; the response-reliability query-builder tests are unaffected — they test the builders, which still pin their own limits inside the now-guaranteed outer cap).
• npm run lint → 0 errors (23 pre-existing <img> warnings in unrelated components).
• Prettier applied to the three changed files.

Not exercised live: a real query against PostHog project 302298 (no admin PostHog key here). The truncation mechanism and the 50k served max are documented and verified live in the issue; this PR bounds every query so the default-100 cap can't apply, driven directly by the unit test.

Out of scope, named

• The writeCache >900KB Firestore-field skip (noted in the issue) — a separate caching-completeness concern, not a query-truncation one; left for its own change.
• Full migration of the releases route onto posthogResults (to also gain Firestore caching) — this PR only closes the truncation bug there; the caching migration is a larger, separable refactor.
• The unclamped days param on notifications is now truncation-safe via the chokepoint guard; a hard upper bound on days is a resource-shape concern, not correctness, and left out.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[Git-on-my-level]

Thanks for chasing this down — the shared withRowLimit primitive and the regression tests for the helper look good, and the targeted vitest/typecheck pass locally.

One blocker before this can land: the PR does not yet guard every admin HogQL request that can hit PostHog's silent default. web/admin/app/api/omi/stats/profitability/route.ts still has direct fetch(.../query/) calls that bypass posthogFetch; in particular fetchDesktopActivePerDay() builds a GROUP BY day query with no explicit LIMIT and sends it unwrapped. That means this PR still leaves a limit-less HogQL query outside the shared guard, despite the intended repo-wide fix.

Please either route those remaining direct PostHog HogQL calls through posthogFetch/posthogResults (or at least withRowLimit where direct fetch is still needed), and ideally add a small regression check that direct admin PostHog query helpers cannot accidentally bypass the row-limit guard again.

Validation I ran locally with secrets stripped:

• npm test -- lib/__tests__/posthog-row-limit.test.ts ✅
• npm run typecheck ✅

---

by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with need human response.

[aryanorastar]

Addressed — thanks for catching the bypass. You were right that profitability/route.ts (and a second one, onboarding/posthog/route.ts) have their own fetch outside the posthogFetch chokepoint and were still sending unwrapped queries. Both now apply withRowLimit directly, matching the releases route:

• profitability: fetchDesktopActivePerDay's GROUP BY day (no LIMIT) and the other query's ad-hoc LIMIT 500000 (overstates the 50k served max) are both wrapped.
• onboarding/posthog: its entrant-actors query wrapped.

And added the regression check you asked for — 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 default-100 truncation. It fails on exactly the state before this fix.

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

Validation: npx vitest run → 36 passed (incl. the new ratchet), npm run typecheck clean, npm run lint 0 errors.

[Git-on-my-level]

Thanks for the quick follow-up. I re-reviewed the current head (69dc5ddf) and the blocker from my previous review looks resolved: the remaining direct admin PostHog query fetches in profitability/route.ts are now wrapped, onboarding/posthog/route.ts is covered too, and the new no-bypass ratchet catches files that send raw kind: "HogQLQuery" without withRowLimit.

Validation I ran with secrets stripped:

• npm test -- lib/__tests__/posthog-row-limit.test.ts lib/__tests__/posthog-no-bypass.test.ts ✅ (5 tests)
• npm run typecheck ✅
• npm test ✅ (36 tests)

I’m not formally approving because this automation run is gated from approval while the PR/check state has approval-disallowed reasons in the monitor context, but from the code review side this is a positive signal. I’m dismissing my stale changes-requested review because its specific blocker is addressed.

---

by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with need human response.

Dismissed by automation: the current head wraps the previously unguarded direct PostHog HogQL fetches and adds a no-bypass regression test, so this review’s specific blocker is resolved.

[aryanorastar]

@Git-on-my-level need human response — thanks for the re-review and for dismissing the stale block. The bypass is fixed (profitability + onboarding wrapped, no-bypass ratchet added), all checks pass. Ready for a maintainer's approving review on #10190.